Re: [PATCH v3 01/11] gdbstub: Support disablement in a multi-threaded process

[Ilya Leoshkevich]

On Mon, 2024-02-19 at 14:05 +0100, Ilya Leoshkevich wrote:
> On Sat, 2024-02-17 at 10:21 -1000, Richard Henderson wrote:
> > On 2/16/24 03:05, Ilya Leoshkevich wrote:
> > > The upcoming follow-fork-mode child support will require
> > > disabling
> > > gdbstub in the parent process, which may have multiple threads
> > > (which
> > > are represented as CPUs).
> > > 
> > > Loop over all CPUs in order to remove breakpoints and disable
> > > single-step. Move the respective code into a separate function.
> > > 
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > >   gdbstub/user.c | 19 +++++++++++++++----
> > >   1 file changed, 15 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/gdbstub/user.c b/gdbstub/user.c
> > > index 14918d1a217..e17f7ece908 100644
> > > --- a/gdbstub/user.c
> > > +++ b/gdbstub/user.c
> > > @@ -356,16 +356,27 @@ int gdbserver_start(const char
> > > *port_or_path)
> > >       return -1;
> > >   }
> > >   
> > > +static void disable_gdbstub(void)
> > > +{
> > > +    CPUState *cpu;
> > > +
> > > +    close(gdbserver_user_state.fd);
> > > +    gdbserver_user_state.fd = -1;
> > > +    CPU_FOREACH(cpu) {
> > > +        cpu_breakpoint_remove_all(cpu, BP_GDB);
> > > +        /* no cpu_watchpoint_remove_all for user-mode */
> > > +        cpu_single_step(cpu, 0);
> > > +        tb_flush(cpu);
> > 
> > You only need to flush once.  The cpu argument is used to determine
> > if we can perform the 
> > flush immediately or need to queue it.
> 
> I thought we needed to flush jump caches on all CPUs, but I see now
> that do_tb_flush() already does this, so this loop is unnecessarily
> quadratic.
> 
> Btw, shouldn't do_tb_flush() have cpu as a local variable, and not as
> a parameter?

Never mind, the dummy parameter is needed for usage with
async_safe_run_on_cpu().


[...]