Re: [PATCH tip/core/rcu 4/5] sys_membarrier: Add expedited option

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

On Thu, Jul 27, 2017 at 09:55:51PM +0800, Boqun Feng wrote:
> Hi Paul,
> 
> I have a side question out of curiosity:
> 
> How does synchronize_sched() work properly for sys_membarrier()?
> 
> sys_membarrier() requires every other CPU does a smp_mb() before it
> returns, and I know synchronize_sched() will wait until all CPUs running
> a kernel thread do a context-switch, which has a smp_mb(). However, I
> believe sched flavor RCU treat CPU running a user thread as a quiesent
> state, so synchronize_sched() could return without that CPU does a
> context switch. 
> 
> So why could we use synchronize_sched() for sys_membarrier()?
> 
> In particular, could the following happens?
> 
> 	CPU 0:				CPU 1:
> 	=========================	==========================
> 	<in user space>			<in user space>
> 					{read Y}(reordered) <------------------------------+
> 	store Y;			                                                   |
> 					read X; --------------------------------------+    |
> 	sys_membarrier():		<timer interrupt>                             |    |
> 	  synchronize_sched();		  update_process_times(user): //user == true  |    |
> 	  				    rcu_check_callbacks(usr):                 |    |
> 					      if (user || ..) {                       |    |
> 					        rcu_sched_qs()                        |    |
> 						...                                   |    |
> 						<report quesient state in softirq>    |    |

The reporting of the quiescent state will acquire the leaf rcu_node
structure's lock, with an smp_mb__after_unlock_lock(), which will
one way or another be a full memory barrier.  So the reorderings
cannot happen.

Unless I am missing something subtle.  ;-)

						Thanx, Paul

> 					<return to user space>                        |    |
> 					read Y; --------------------------------------+----+
> 	store X;			                                              |
> 					{read X}(reordered) <-------------------------+
> 
> I assume the timer interrupt handler, which interrupts a user space and
> reports a quiesent state for sched flavor RCU, may not have a smp_mb()
> in some code path.
> 
> I may miss something subtle, but it just not very obvious how
> synchronize_sched() will guarantee a remote CPU running in userspace to
> do a smp_mb() before it returns, this is at least not in RCU
> requirements, right?
> 
> Regards,
> Boqun