Re: [PATCH 6/8] linux-nat.c: better starvation avoidance, handle non-stop mode too

[Yao Qi <yao at codesourcery dot com>]

Pedro Alves <palves@redhat.com> writes:

> 3) If multiple threads hit a breakpoint, we report one of those, and
> "cancel" the others.  Cancelling means decrementing the PC, and
> discarding the event.  If the next time the LWP is resumed the
> breakpoint is still installed, the LWP should hit it again, and we'll
> report the hit then.  The problem I found is that this delays threads
> from advancing too much, with the kernel potentially ending up
> scheduling the same threads over and over, and others not advancing.

Sorry, I don't fully understand what is the problem.  Can you elaborate?
Is it something like GDB gets an event of thread foo, and events from
other threads aren't interesting.  GDB cancels them, but when GDB
resumes them, kernel always schedules thread foo.

> So the patch switches away from cancelling the breakpoints, and
> instead remembering that the LWP had stopped for a breakpoint.  If on
> resume the breakpoint is still installed, we report it.  If it's no
> longer installed, we discard the pending event then.  This is actually

Does "breakpoint is installed" indicate "target is still running"? then,
we report the event.  Otherwise, "target is stopped", we discard the
pending event.  Is that correct?

> @@ -2453,7 +2482,9 @@ stop_wait_callback (struct lwp_info *lp, void *data)
>    return 0;
>  }
>  
> -/* Return non-zero if LP has a wait status pending.  */
> +/* Return non-zero if LP has a wait status pending.  Discard the
> +   pending event and resume the LWP if the event that originally
> +   caused the stop became uninteresting.  */
>  
>  static int
>  status_callback (struct lwp_info *lp, void *data)

Probably, we should rename status_callback to something more meaningful.

> @@ -2523,16 +2601,19 @@ select_event_lwp_callback (struct lwp_info *lp, void *data)
>  
>    gdb_assert (selector != NULL);
>  
> -  /* Select only resumed LWPs that have a SIGTRAP event pending.  */
> -  if (lp->resumed && linux_nat_lp_status_is_event (lp))
> +  /* Select only resumed LWPs that have an event pending.  */
> +  if (lp->resumed && lwp_status_pending_p (lp))
>      if ((*selector)-- == 0)
>        return 1;

Do we need to do the same change in count_events_callback?  IMO, the
condition in both count_events_callback and select_event_lwp_callback
should be consistent.

-- 
Yao (éå)