Re: [PATCH] pi-futex: set PF_EXITING without taking ->pi_lock

[Paul Mackerras <paulus@samba.org>]

Ingo Molnar writes:

> 
> * Oleg Nesterov <oleg@tv-sign.ru> wrote:
> 
> > 	static inline void ccids_read_lock(void)
> > 	{
> > 		atomic_inc(&ccids_lockct);
> > 		spin_unlock_wait(&ccids_lock);
> > 	}
> > 
> > This looks racy, in theory atomic_inc() and spin_unlock_wait() could 
> > be re-ordered. However, in this particular case we have an "optimized" 
> > smp_mb_after_atomic_inc(), perhaps it is good that the caller can 
> > choose the "right" barrier by hand.
> 
> _all_ default locking and atomic APIs should be barrier-safe i believe. 
> (and that includes atomic_inc() too) Most people dont have barriers on 
> their mind when their code. _If_ someone is barrier-conscious then we 
> should have barrier-less APIs too for that purpose of squeezing the last 
> half cycle out of the code, but it should be a non-default choice. The 
> reason: nobody notices an unnecessary barrier, but a missing barrier can 
> be nasty.

The approach we have taken on powerpc is that the atomic_*_test and
atomic_*_return functions have a barrier, but the straight atomic_inc
etc. don't.

As for putting barriers in, it's not a half cycle, it's more like 50
to 100 on some processors.  Added to that, what I think you are
actually advocating is *two* full barriers - one before the increment
and one after.  That seems like an enormous penalty to pay just
because some people want to roll their own lock primitives instead of
using the standard ones.

Why is ccids_read_lock trying to implement a rwlock without using an
rwlock?  Could it be converted to an ordinary rwlock?  Or an rwsem?

Paul.