[rfc] Preferred thread event reporting: Linux native target

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hello,

this is another patch we've been using internally for a while.

The problem addressed by this patch is related to the way the Linux native
target handled simultaneous events in multi-threaded programs.  After the
OS has signalled an event in some thread, GDB will stop all other threads
(unless in non-stop mode).  During this process, some other thread might
*also* signal an event, so once everything is stopped, there might be
multiple threads with pending events.

However, the Linux native target has to select just one of them to report
up to the infrun logic.  It currently makes that decision randomly.

This may be unexpected to a user who is currently working on one specific
thread (say, single-stepping).  If the single-step event in the thread the
user is working on arrives at the same time as some other event in some 
other thread, it would be nice if that single-step event were reported
first, so as to not interrupt the user's work flow.

And indeed, the Linux native target does contain special code that prefers
reporting an event in a thread that is currently single-stepped, bypassing
the randomization logic that would otherwise be used.

*However*, this unfortunately works only on platforms that use *hardware*
single-stepping, because the native target is not even aware that anything
special is going on when the target uses software single-stepping.

It seems to me that this implementation detail should not determine 
user-visible behaviour in the way described above.  Therefore, I'd suggest
to try to *always* prefer reporting events in the thread the user currently
"cares about" -- this is actually simply the currently selected thread
(i.e. the current value of inferior_ptid).  

The patch below implements this by adding a new member "preferred" to
"struct lwp_info", setting it according to the value of inferior_ptid
in linux_nat_resume, and using it (instead of the single-step flag) to
decide whether to prefer reporting events in this thread.

Tested on powerpc-linux and powerpc64-linux.

Any comments on this approach welcome ...  Am I missing some reason why
this might not be a good idea?

Bye,
Ulrich


ChangeLog:

	* linux-nat.h (struct lwp_info): New member "preferred".
	* linux-nat.c (resume_callback): Reset lp->preferred.
	(linux_nat_resume): Set lp->preferred.
	(select_singlestep_lwp_callback): Rename to ...
	(select_preferred_lwp_callback): ... this.  Check for lp->preferred
	instead of lp->step.
	(select_event_lwp): Update call to select_singlestep_lwp_callback.


diff -urNp gdb-orig/gdb/linux-nat.c gdb-head/gdb/linux-nat.c
--- gdb-orig/gdb/linux-nat.c	2008-08-08 20:14:08.000000000 +0200
+++ gdb-head/gdb/linux-nat.c	2008-08-14 19:28:07.334541534 +0200
@@ -1600,6 +1600,7 @@ resume_callback (struct lwp_info *lp, vo
 			    "RC:  PTRACE_CONT %s, 0, 0 (resume sibling)\n",
 			    target_pid_to_str (lp->ptid));
       lp->stopped = 0;
+      lp->preferred = 0;
       lp->step = 0;
       memset (&lp->siginfo, 0, sizeof (lp->siginfo));
     }
@@ -1671,6 +1672,9 @@ linux_nat_resume (ptid_t ptid, int step,
   /* Convert to something the lower layer understands.  */
   ptid = pid_to_ptid (GET_LWP (lp->ptid));
 
+  /* Mark this thread as the preferred thread.  */
+  lp->preferred = 1;
+
   /* Remember if we're stepping.  */
   lp->step = step;
 
@@ -2293,12 +2297,12 @@ count_events_callback (struct lwp_info *
   return 0;
 }
 
-/* Select the LWP (if any) that is currently being single-stepped.  */
+/* Select the LWP the user is currently interested in.  */
 
 static int
-select_singlestep_lwp_callback (struct lwp_info *lp, void *data)
+select_preferred_lwp_callback (struct lwp_info *lp, void *data)
 {
-  if (lp->step && lp->status != 0)
+  if (lp->preferred && lp->status != 0)
     return 1;
   else
     return 0;
@@ -2396,8 +2400,8 @@ select_event_lwp (struct lwp_info **orig
   /* Record the wait status for the original LWP.  */
   (*orig_lp)->status = *status;
 
-  /* Give preference to any LWP that is being single-stepped.  */
-  event_lp = iterate_over_lwps (select_singlestep_lwp_callback, NULL);
+  /* Give preference to the LWP the user is currently interested in.  */
+  event_lp = iterate_over_lwps (select_preferred_lwp_callback, NULL);
   if (event_lp != NULL)
     {
       if (debug_linux_nat)
@@ -2407,8 +2411,8 @@ select_event_lwp (struct lwp_info **orig
     }
   else
     {
-      /* No single-stepping LWP.  Select one at random, out of those
-         which have had SIGTRAP events.  */
+      /* No preferred LWP.  Select one at random, out of those which
+	 have had SIGTRAP events.  */
 
       /* First see how many SIGTRAP events we have.  */
       iterate_over_lwps (count_events_callback, &num_events);
diff -urNp gdb-orig/gdb/linux-nat.h gdb-head/gdb/linux-nat.h
--- gdb-orig/gdb/linux-nat.h	2008-08-05 22:16:25.000000000 +0200
+++ gdb-head/gdb/linux-nat.h	2008-08-14 19:20:07.523558223 +0200
@@ -65,6 +65,9 @@ struct lwp_info
   /* Non-zero if we expect a duplicated SIGINT.  */
   int ignore_sigint;
 
+  /* Non-zero if this is the thread preferred for event reporting.  */
+  int preferred;
+
   /* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus
      for this LWP's last event.  This may correspond to STATUS above,
      or to a local variable in lin_lwp_wait.  */
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com