Re: [PATCH] Fix rwlock stall with PREFER_WRITER_NONRECURSIVE_NP (bug 23861)

[Torvald Riegel <triegel at redhat dot com>]

On Thu, 2018-11-08 at 14:36 -0500, Carlos O'Donell wrote:
> On 11/8/18 9:54 AM, Andreas Schwab wrote:
> > 	[BZ #23861]
> > 	* nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full):
> > 	Update expected value for __readers while waiting on
> > 	PTHREAD_RWLOCK_RWAITING.
> > 	* nptl/tst-rwlock-pwn.c: New file.
> > 	* nptl/Makefile (tests): Add tst-rwlock-pwn.
> 
> Is this at all related to this bug?
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=23844

No that's a different bug.  At least the bug in the code is different, not
sure whether it could result in the same symptoms.

> >  
> >  tests-internal := tst-rwlock19 tst-rwlock20 \
> >  		  tst-sem11 tst-sem12 tst-sem13 \
> > diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> > index a290d08332..e95cbe4033 100644
> > --- a/nptl/pthread_rwlock_common.c
> > +++ b/nptl/pthread_rwlock_common.c
> > @@ -314,12 +314,12 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock,
> >  		 harmless because the flag is just about the state of
> >  		 __readers, and all threads set the flag under the same
> >  		 conditions.  */

I'd add this to the comment:
"Update r so that the futex call in the loop uses the correct value."

> > -	      while ((atomic_load_relaxed (&rwlock->__data.__readers)
> > -		  & PTHREAD_RWLOCK_RWAITING) != 0)
> > +	      while (((r = atomic_load_relaxed (&rwlock->__data.__readers))
> > +		      & PTHREAD_RWLOCK_RWAITING) != 0)
> >  		{
> >  		  int private = __pthread_rwlock_get_private (rwlock);
> >  		  int err = futex_abstimed_wait (&rwlock->__data.__readers,
> > -		      r, abstime, private);
> > +						 r, abstime, private);
> 
> Why is this change correct?

The fix is fine (and the formatting too, I guess).  I think the comment
above should explain the fix.  The futex call needs to wait on the based on
the value that lead to calling futex_wait in the first place.