Re: [PATCH 2/2] percpu-rw-semaphores: use rcu_read_lock_sched

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Oct 24, 2012 at 08:43:11PM +0200, Oleg Nesterov wrote:
> On 10/24, Paul E. McKenney wrote:
> >
> > On Wed, Oct 24, 2012 at 07:18:55PM +0200, Oleg Nesterov wrote:
> > > On 10/24, Paul E. McKenney wrote:
> > > >
> > > > static inline void percpu_up_read(struct percpu_rw_semaphore *p)
> > > > {
> > > > 	/*
> > > > 	 * Decrement our count, but protected by RCU-sched so that
> > > > 	 * the writer can force proper serialization.
> > > > 	 */
> > > > 	rcu_read_lock_sched();
> > > > 	this_cpu_dec(*p->counters);
> > > > 	rcu_read_unlock_sched();
> > > > }
> > >
> > > Yes, the explicit lock/unlock makes the new assumptions about
> > > synchronize_sched && barriers unnecessary. And iiuc this could
> > > even written as
> > >
> > > 	rcu_read_lock_sched();
> > > 	rcu_read_unlock_sched();
> > >
> > > 	this_cpu_dec(*p->counters);
> >
> > But this would lose the memory barrier that is inserted by
> > synchronize_sched() after the CPU's last RCU-sched read-side critical
> > section.
> 
> How? Afaics there is no need to synchronize with this_cpu_dec(), its
> result was already seen before the 2nd synchronize_sched() was called
> in percpu_down_write().
> 
> IOW, this memory barrier is only needed to synchronize with memory
> changes inside down_read/up_read.
> 
> To clarify, of course I do not suggest to write is this way. I am just
> trying to check my understanding.

You are quite correct -- once the writer has seen the change in the
counter, it knows that the reader's empty RCU-sched read must have
at least started, and thus can rely on the following memory barrier
to guarantee that it sees the reader's critical section.

But that code really does look strange, I will grant you that!  ;-)

							Thanx, Paul