RFC: patch to lin-lwp.c

RFC: patch to lin-lwp.c

[J. Johnston]

The attached patch is created to deal with the problem as discussed on:

   http://sources.redhat.com/ml/gdb/2003-08/msg00043.html

The fix does two things:

   1. it sets the resumed flag for threads we stopped when the main thread
      exited and then resumed
   2. changes the running_callback to include threads that have events
      pending on them that will be caught on the next wait

Ok to commit?

-- Jeff J.

2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>

	* lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
	for any lwp we resume.
	(running_callback): Add lwps that have pending status events
	against them to be considered running.

Re: RFC: patch to lin-lwp.c

[J. Johnston]

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

Sorry, forgot to attach patch.

J. Johnston wrote:
> The attached patch is created to deal with the problem as discussed on:
> 
>   http://sources.redhat.com/ml/gdb/2003-08/msg00043.html
> 
> The fix does two things:
> 
>   1. it sets the resumed flag for threads we stopped when the main thread
>      exited and then resumed
>   2. changes the running_callback to include threads that have events
>      pending on them that will be caught on the next wait
> 
> Ok to commit?
> 
> -- Jeff J.
> 
> 2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>
> 
>     * lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
>     for any lwp we resume.
>     (running_callback): Add lwps that have pending status events
>     against them to be considered running.
> 
> 
> 


[-- Attachment #2: lin-lwp.patch --]
[-- Type: text/plain, Size: 769 bytes --]

Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.47
diff -u -r1.47 lin-lwp.c
--- lin-lwp.c	19 Jun 2003 22:52:03 -0000	1.47
+++ lin-lwp.c	5 Aug 2003 23:09:55 -0000
@@ -839,7 +839,7 @@
 static int
 running_callback (struct lwp_info *lp, void *data)
 {
-  return (lp->stopped == 0);
+  return (lp->stopped == 0 || (lp->status != 0 && lp->resumed));
 }
 
 /* Count the LWP's that have had events.  */
@@ -1087,7 +1087,10 @@
       /* Resume if the lwp still exists.  */
       for (ptr = lwp_list; ptr; ptr = ptr->next)
 	if (lp == ptr)
-	  resume_callback (lp, NULL);
+	  {
+	    resume_callback (lp, NULL);
+	    resume_set_callback (lp, NULL);
+	  }
     }
   return 0;
 }

Re: RFC: patch to lin-lwp.c

[Daniel Jacobowitz]

This patch was never reviewed as far as I can see.  I just got a bug
report that I think it would fix...

Mark, you've said that you don't feel comfortable maintaining the
Linux-specific threading layer, is that recollection right?  Michael,
do you?  If not, I'd like to clearly indicate who can approve patches
to lin-lwp.c.

On Wed, Aug 06, 2003 at 01:11:09PM -0400, J. Johnston wrote:
> Sorry, forgot to attach patch.
> 
> J. Johnston wrote:
> >The attached patch is created to deal with the problem as discussed on:
> >
> >  http://sources.redhat.com/ml/gdb/2003-08/msg00043.html
> >
> >The fix does two things:
> >
> >  1. it sets the resumed flag for threads we stopped when the main thread
> >     exited and then resumed
> >  2. changes the running_callback to include threads that have events
> >     pending on them that will be caught on the next wait
> >
> >Ok to commit?
> >
> >-- Jeff J.
> >
> >2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>
> >
> >    * lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
> >    for any lwp we resume.
> >    (running_callback): Add lwps that have pending status events
> >    against them to be considered running.
> >
> >
> >
> 

> Index: lin-lwp.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/lin-lwp.c,v
> retrieving revision 1.47
> diff -u -r1.47 lin-lwp.c
> --- lin-lwp.c	19 Jun 2003 22:52:03 -0000	1.47
> +++ lin-lwp.c	5 Aug 2003 23:09:55 -0000
> @@ -839,7 +839,7 @@
>  static int
>  running_callback (struct lwp_info *lp, void *data)
>  {
> -  return (lp->stopped == 0);
> +  return (lp->stopped == 0 || (lp->status != 0 && lp->resumed));
>  }
>  
>  /* Count the LWP's that have had events.  */
> @@ -1087,7 +1087,10 @@
>        /* Resume if the lwp still exists.  */
>        for (ptr = lwp_list; ptr; ptr = ptr->next)
>  	if (lp == ptr)
> -	  resume_callback (lp, NULL);
> +	  {
> +	    resume_callback (lp, NULL);
> +	    resume_set_callback (lp, NULL);
> +	  }
>      }
>    return 0;
>  }


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: patch to lin-lwp.c

[Michael Snyder]

Daniel Jacobowitz wrote:
> This patch was never reviewed as far as I can see.  I just got a bug
> report that I think it would fix...
> 
> Mark, you've said that you don't feel comfortable maintaining the
> Linux-specific threading layer, is that recollection right?  Michael,
> do you? 

Barely.  I felt about 50% able to grok it when I last worked on it,
and it's changed a lot since then.

 > If not, I'd like to clearly indicate who can approve patches
> to lin-lwp.c.

It's more who ought to than who can.  I think any of you, me,
Mark, or Jim Blandy (and maybe we ought to add Jeff Johnston)
are qualified to approve them, but I don't know which one or
two persons understand it fully enough to be called on as a
maintainer.  Jim and I, for instance, worked on it a long time
ago.

Part of the problem, I think, is that nobody really wants it.
It's complicated, difficult, and fragile.


> 
> On Wed, Aug 06, 2003 at 01:11:09PM -0400, J. Johnston wrote:
> 
>>Sorry, forgot to attach patch.
>>
>>J. Johnston wrote:
>>
>>>The attached patch is created to deal with the problem as discussed on:
>>>
>>> http://sources.redhat.com/ml/gdb/2003-08/msg00043.html
>>>
>>>The fix does two things:
>>>
>>> 1. it sets the resumed flag for threads we stopped when the main thread
>>>    exited and then resumed
>>> 2. changes the running_callback to include threads that have events
>>>    pending on them that will be caught on the next wait
>>>
>>>Ok to commit?
>>>
>>>-- Jeff J.
>>>
>>>2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>
>>>
>>>   * lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
>>>   for any lwp we resume.
>>>   (running_callback): Add lwps that have pending status events
>>>   against them to be considered running.
>>>
>>>
>>>
>>
> 
>>Index: lin-lwp.c
>>===================================================================
>>RCS file: /cvs/src/src/gdb/lin-lwp.c,v
>>retrieving revision 1.47
>>diff -u -r1.47 lin-lwp.c
>>--- lin-lwp.c	19 Jun 2003 22:52:03 -0000	1.47
>>+++ lin-lwp.c	5 Aug 2003 23:09:55 -0000
>>@@ -839,7 +839,7 @@
>> static int
>> running_callback (struct lwp_info *lp, void *data)
>> {
>>-  return (lp->stopped == 0);
>>+  return (lp->stopped == 0 || (lp->status != 0 && lp->resumed));
>> }
>> 
>> /* Count the LWP's that have had events.  */
>>@@ -1087,7 +1087,10 @@
>>       /* Resume if the lwp still exists.  */
>>       for (ptr = lwp_list; ptr; ptr = ptr->next)
>> 	if (lp == ptr)
>>-	  resume_callback (lp, NULL);
>>+	  {
>>+	    resume_callback (lp, NULL);
>>+	    resume_set_callback (lp, NULL);
>>+	  }
>>     }
>>   return 0;
>> }
> 
> 
> 

Re: RFC: patch to lin-lwp.c

[Andrew Cagney]

>> If not, I'd like to clearly indicate who can approve patches
> to lin-lwp.c.
> 
> It's more who ought to than who can.  I think any of you, me,
> Mark, or Jim Blandy (and maybe we ought to add Jeff Johnston)
> are qualified to approve them, but I don't know which one or
> two persons understand it fully enough to be called on as a
> maintainer.  Jim and I, for instance, worked on it a long time
> ago.
> 
> Part of the problem, I think, is that nobody really wants it.
> It's complicated, difficult, and fragile.

The general trend for such code to not have an explicit maintainer. 
Instead changes get approved using peer-review that involves at least 
one core maintainer.  Anything architectural being pulled out and 
discssed separatly.

I should also red flag that changing the target vector means changing 
the thread code so "it could be fun times".

enjoy,
Andrew

Re: RFC: patch to lin-lwp.c

[Daniel Jacobowitz]

On Wed, Aug 06, 2003 at 01:11:09PM -0400, J. Johnston wrote:
> Sorry, forgot to attach patch.
> 
> J. Johnston wrote:
> >The attached patch is created to deal with the problem as discussed on:
> >
> >  http://sources.redhat.com/ml/gdb/2003-08/msg00043.html
> >
> >The fix does two things:
> >
> >  1. it sets the resumed flag for threads we stopped when the main thread
> >     exited and then resumed
> >  2. changes the running_callback to include threads that have events
> >     pending on them that will be caught on the next wait
> >
> >Ok to commit?
> >
> >-- Jeff J.
> >
> >2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>
> >
> >    * lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
> >    for any lwp we resume.
> >    (running_callback): Add lwps that have pending status events
> >    against them to be considered running.
> >
> >
> >

This patch is OK.  Thanks!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFC: patch to lin-lwp.c

[J. Johnston]

Ok, patch has been checked into mainline.  If it is supposed to also be
in gdb 6.0, let me know.

I fixed the typo in the ChangeLog entry with regards to the file name being changed.

-- Jeff J.

Daniel Jacobowitz wrote:
> On Wed, Aug 06, 2003 at 01:11:09PM -0400, J. Johnston wrote:
> 
>>Sorry, forgot to attach patch.
>>
>>J. Johnston wrote:
>>
>>>The attached patch is created to deal with the problem as discussed on:
>>>
>>> http://sources.redhat.com/ml/gdb/2003-08/msg00043.html
>>>
>>>The fix does two things:
>>>
>>> 1. it sets the resumed flag for threads we stopped when the main thread
>>>    exited and then resumed
>>> 2. changes the running_callback to include threads that have events
>>>    pending on them that will be caught on the next wait
>>>
>>>Ok to commit?
>>>
>>>-- Jeff J.
>>>
>>>2003-08-05  Jeff Johnston  <jjohnstn@redhat.com>
>>>
>>>   * lin-lwp-wait.c (stop_and_resume_callback): Set the resumed flag
>>>   for any lwp we resume.
>>>   (running_callback): Add lwps that have pending status events
>>>   against them to be considered running.
>>>
>>>
>>>
> 
> 
> This patch is OK.  Thanks!
>