[RFC] lin-lwp.c prelim changes for new thread model

[RFC] lin-lwp.c prelim changes for new thread model

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

Hi folks, 

The up and coming kernel (2.4.20, I believe?) and the next glibc (2.3.1)
both bring some drastic changes to linux threads.  The current gdb thread
debugging code will not handle them as is.

This is a smallish change that I propose as a preliminary step;
it'll get things partly working in the new world, without breaking
them in the old.

Here's the rationalle.

In the old/current model, when one thread gets a signal (such as TRAP), 
we (gdb) have to call kill (SIGSTOP, pid) for every other thread
(excepting the event thread), and then do a waitpid on each of them.

In the new model, when one thread gets a signal, we only have to 
send kill(SIGSTOP, pid) to _one_ thread, and the kernel will then
propagate the signal to all of them (_including_ the one that has
already stopped with eg. SIGTRAP).  We must still do a waitpid on
each and every thread -- however, that now _includes_ the one that
stopped in the first place (and which we've already done one waitpid on).

I know, you're thinking "wasn't this supposed to get simpler?"

The minimal change I propose below is as follows:
When we send kill(SIGSTOP) to all the threads, we now include
the event thread, where previously we had made him a special case.
That way, whether in the new model or the old one, we can now do
a waitpid on every thread including the event thread.

What do you think?

Michael

[-- Attachment #2: kill.diff --]
[-- Type: text/plain, Size: 2371 bytes --]

2003-01-06  Michael Snyder  <msnyder@redhat.com>

	* lin-lwp.c (lin_lwp_wait): Allow the event thread to receive
	kill (SIGSTOP) along with everyone else, and do a corresponding
	waitpid on the event thread.  This will help us adapt gdb to 
	work with the new kernel and its different thread model.

Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.40
diff -p -r1.40 lin-lwp.c
*** lin-lwp.c	6 Jan 2003 23:12:29 -0000	1.40
--- lin-lwp.c	6 Jan 2003 23:14:44 -0000
*************** lin_lwp_wait (ptid_t ptid, struct target
*** 1358,1377 ****
  	}
      }
  
!   /* This LWP is stopped now.  */
!   lp->stopped = 1;
  
!   if (debug_lin_lwp)
!     fprintf_unfiltered (gdb_stdlog, "LLW: Candidate event %s in %s.\n",
! 			status_to_str (status), 
! 			target_pid_to_str (lp->ptid));
  
!   /* Now stop all other LWP's ...  */
!   iterate_over_lwps (stop_callback, NULL);
  
!   /* ... and wait until all of them have reported back that they're no
!      longer running.  */
!   iterate_over_lwps (stop_wait_callback, &flush_mask);
  
    /* If we're not waiting for a specific LWP, choose an event LWP from
       among those that have had events.  Giving equal priority to all
--- 1358,1388 ----
  	}
      }
  
!   /* Stop all other LWPs, if any.  */
!   if (!WIFEXITED (status))	/* Can't stop it if it's exited...  */
!     {
!       if (debug_lin_lwp)
! 	fprintf_unfiltered (gdb_stdlog, "LLW: Candidate event %s in %s.\n",
! 			    status_to_str (status), 
! 			    target_pid_to_str (lp->ptid));
  
!       /* Now stop all other LWP's ...  */
!       iterate_over_lwps (stop_callback, NULL);
  
!       /* The event thread must now be continued, before it can be
! 	 waited again.  */
!       errno = 0;
!       ptrace (PTRACE_CONT, GET_LWP (lp->ptid), 0, 0);
!       if (debug_lin_lwp)
! 	fprintf_unfiltered (gdb_stdlog, 
! 			    "LLW: PTRACE_CONT %s, 0, 0 (%s)\n",
! 			    target_pid_to_str (lp->ptid),
! 			    errno ? safe_strerror (errno) : "OK");
  
!       /* ... and wait until all of them have reported back that they're no
! 	 longer running.  */
!       iterate_over_lwps (stop_wait_callback, &flush_mask);
!     }
  
    /* If we're not waiting for a specific LWP, choose an event LWP from
       among those that have had events.  Giving equal priority to all

Re: [RFC] lin-lwp.c prelim changes for new thread model

[Daniel Jacobowitz]

On Mon, Jan 06, 2003 at 03:44:05PM -0800, Michael Snyder wrote:
> Hi folks, 
> 
> The up and coming kernel (2.4.20, I believe?) and the next glibc (2.3.1)
> both bring some drastic changes to linux threads.  The current gdb thread
> debugging code will not handle them as is.
> 
> This is a smallish change that I propose as a preliminary step;
> it'll get things partly working in the new world, without breaking
> them in the old.
> 
> Here's the rationalle.
> 
> In the old/current model, when one thread gets a signal (such as TRAP), 
> we (gdb) have to call kill (SIGSTOP, pid) for every other thread
> (excepting the event thread), and then do a waitpid on each of them.
> 
> In the new model, when one thread gets a signal, we only have to 
> send kill(SIGSTOP, pid) to _one_ thread, and the kernel will then
> propagate the signal to all of them (_including_ the one that has
> already stopped with eg. SIGTRAP).  We must still do a waitpid on
> each and every thread -- however, that now _includes_ the one that
> stopped in the first place (and which we've already done one waitpid on).
> 
> I know, you're thinking "wasn't this supposed to get simpler?"
> 
> The minimal change I propose below is as follows:
> When we send kill(SIGSTOP) to all the threads, we now include
> the event thread, where previously we had made him a special case.
> That way, whether in the new model or the old one, we can now do
> a waitpid on every thread including the event thread.
> 
> What do you think?

To be honest, I don't like this very much.  You're hurting performance
in the current case (whose performance is already quite bad enough,
thank you!).  I don't think that the additional complexity/waiting is
worthwhile.

If we detect CLONE_THREAD (how do we detect CLONE_THREAD?) we can mark
the new LWP as having a pending stop based on that.  Or, Roland has a
kernel patch that's stewing in my mailbox which provides a better way
to handle this entire thing than sending SIGSTOP.  I don't think he
ever tested it, and I know I haven't had time, but let me know if you
want a copy.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] lin-lwp.c prelim changes for new thread model

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> On Mon, Jan 06, 2003 at 03:44:05PM -0800, Michael Snyder wrote:
> > Hi folks,
> >
> > The up and coming kernel (2.4.20, I believe?) and the next glibc (2.3.1)
> > both bring some drastic changes to linux threads.  The current gdb thread
> > debugging code will not handle them as is.
> >
> > This is a smallish change that I propose as a preliminary step;
> > it'll get things partly working in the new world, without breaking
> > them in the old.
> >
> > Here's the rationalle.
> >
> > In the old/current model, when one thread gets a signal (such as TRAP),
> > we (gdb) have to call kill (SIGSTOP, pid) for every other thread
> > (excepting the event thread), and then do a waitpid on each of them.
> >
> > In the new model, when one thread gets a signal, we only have to
> > send kill(SIGSTOP, pid) to _one_ thread, and the kernel will then
> > propagate the signal to all of them (_including_ the one that has
> > already stopped with eg. SIGTRAP).  We must still do a waitpid on
> > each and every thread -- however, that now _includes_ the one that
> > stopped in the first place (and which we've already done one waitpid on).
> >
> > I know, you're thinking "wasn't this supposed to get simpler?"
> >
> > The minimal change I propose below is as follows:
> > When we send kill(SIGSTOP) to all the threads, we now include
> > the event thread, where previously we had made him a special case.
> > That way, whether in the new model or the old one, we can now do
> > a waitpid on every thread including the event thread.
> >
> > What do you think?
> 
> To be honest, I don't like this very much.  You're hurting performance
> in the current case (whose performance is already quite bad enough,
> thank you!).  I don't think that the additional complexity/waiting is
> worthwhile.

Well, your feedback is high on the priority list.
 
> If we detect CLONE_THREAD (how do we detect CLONE_THREAD?) 

That I don't know.  Because I don't know, and because CLONE_THREAD
represents an entirely new approach, I was looking for something 
that I could get working quickly on both models, using mostly
the existing code and approach.

> we can mark
> the new LWP as having a pending stop based on that.  Or, Roland has a
> kernel patch that's stewing in my mailbox which provides a better way
> to handle this entire thing than sending SIGSTOP.  I don't think he
> ever tested it, and I know I haven't had time, but let me know if you
> want a copy.

I'm not as kernel-hackerish as you.  I'll discuss with Roland 
whether he thinks he can get that patch into the kernel in the
near term (and how it will help).

Re: [RFC] lin-lwp.c prelim changes for new thread model

[Andrew Cagney]

>> The minimal change I propose below is as follows:
>> When we send kill(SIGSTOP) to all the threads, we now include
>> the event thread, where previously we had made him a special case.
>> That way, whether in the new model or the old one, we can now do
>> a waitpid on every thread including the event thread.
>> 
>> What do you think?
> 
> 
> To be honest, I don't like this very much.  You're hurting performance
> in the current case (whose performance is already quite bad enough,
> thank you!).  I don't think that the additional complexity/waiting is
> worthwhile.

Just an aside,

At present GDB does ~60 system calls per multi-threaded single step (see 
previous discussion related to ptrace), these extra two calls may easily 
prove to be in the noise.  Given the thread has already stopped, they 
should be close to kernel no-ops.

When commenting on performance, the thing that is always needed is hard 
data either supporting or dismissing any claim.

Andrew