Re: [PATCH tip/core/rcu 30/30] rcu: Make rcu_barrier() account for offline no-CBs CPUs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Paul,

On Fri, Feb 14, 2020 at 03:56:07PM -0800, paulmck@xxxxxxxxxx wrote:
> From: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> 
> Currently, rcu_barrier() ignores offline CPUs,  However, it is possible
> for an offline no-CBs CPU to have callbacks queued, and rcu_barrier()
> must wait for those callbacks.  This commit therefore makes rcu_barrier()
> directly invoke the rcu_barrier_func() with interrupts disabled for such
> CPUs.  This requires passing the CPU number into this function so that
> it can entrain the rcu_barrier() callback onto the correct CPU's callback
> list, given that the code must instead execute on the current CPU.
> 
> While in the area, this commit fixes a bug where the first CPU's callback
> might have been invoked before rcu_segcblist_entrain() returned, which
> would also result in an early wakeup.
> 
> Fixes: 5d6742b37727 ("rcu/nocb: Use rcu_segcblist for no-CBs CPUs")
> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.5.x
> ---
>  include/trace/events/rcu.h |  1 +
>  kernel/rcu/tree.c          | 32 ++++++++++++++++++++------------
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
> index 5e49b06..d56d54c 100644
> --- a/include/trace/events/rcu.h
> +++ b/include/trace/events/rcu.h
> @@ -712,6 +712,7 @@ TRACE_EVENT_RCU(rcu_torture_read,
>   *	"Begin": rcu_barrier() started.
>   *	"EarlyExit": rcu_barrier() piggybacked, thus early exit.
>   *	"Inc1": rcu_barrier() piggyback check counter incremented.
> + *	"OfflineNoCBQ": rcu_barrier() found offline no-CBs CPU with callbacks.
>   *	"OnlineQ": rcu_barrier() found online CPU with callbacks.
>   *	"OnlineNQ": rcu_barrier() found online CPU, no callbacks.
>   *	"IRQ": An rcu_barrier_callback() callback posted on remote CPU.
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index d15041f..160643e 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3098,9 +3098,10 @@ static void rcu_barrier_callback(struct rcu_head *rhp)
>  /*
>   * Called with preemption disabled, and from cross-cpu IRQ context.
>   */
> -static void rcu_barrier_func(void *unused)
> +static void rcu_barrier_func(void *cpu_in)
>  {
> -	struct rcu_data *rdp = raw_cpu_ptr(&rcu_data);
> +	uintptr_t cpu = (uintptr_t)cpu_in;
> +	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
>  
>  	rcu_barrier_trace(TPS("IRQ"), -1, rcu_state.barrier_sequence);
>  	rdp->barrier_head.func = rcu_barrier_callback;
> @@ -3127,7 +3128,7 @@ static void rcu_barrier_func(void *unused)
>   */
>  void rcu_barrier(void)
>  {
> -	int cpu;
> +	uintptr_t cpu;
>  	struct rcu_data *rdp;
>  	unsigned long s = rcu_seq_snap(&rcu_state.barrier_sequence);
>  
> @@ -3150,13 +3151,14 @@ void rcu_barrier(void)
>  	rcu_barrier_trace(TPS("Inc1"), -1, rcu_state.barrier_sequence);
>  
>  	/*
> -	 * Initialize the count to one rather than to zero in order to
> -	 * avoid a too-soon return to zero in case of a short grace period
> -	 * (or preemption of this task).  Exclude CPU-hotplug operations
> -	 * to ensure that no offline CPU has callbacks queued.
> +	 * Initialize the count to two rather than to zero in order
> +	 * to avoid a too-soon return to zero in case of an immediate
> +	 * invocation of the just-enqueued callback (or preemption of
> +	 * this task).  Exclude CPU-hotplug operations to ensure that no
> +	 * offline non-offloaded CPU has callbacks queued.
>  	 */
>  	init_completion(&rcu_state.barrier_completion);
> -	atomic_set(&rcu_state.barrier_cpu_count, 1);
> +	atomic_set(&rcu_state.barrier_cpu_count, 2);
>  	get_online_cpus();
>  
>  	/*
> @@ -3166,13 +3168,19 @@ void rcu_barrier(void)
>  	 */
>  	for_each_possible_cpu(cpu) {
>  		rdp = per_cpu_ptr(&rcu_data, cpu);
> -		if (!cpu_online(cpu) &&
> +		if (cpu_is_offline(cpu) &&
>  		    !rcu_segcblist_is_offloaded(&rdp->cblist))
>  			continue;
> -		if (rcu_segcblist_n_cbs(&rdp->cblist)) {
> +		if (rcu_segcblist_n_cbs(&rdp->cblist) && cpu_online(cpu)) {
>  			rcu_barrier_trace(TPS("OnlineQ"), cpu,
>  					  rcu_state.barrier_sequence);
> -			smp_call_function_single(cpu, rcu_barrier_func, NULL, 1);
> +			smp_call_function_single(cpu, rcu_barrier_func, (void *)cpu, 1);
> +		} else if (cpu_is_offline(cpu)) {

I wonder whether this should be:

		  else if (rcu_segcblist_n_cbs(&rdp->cblist) && cpu_is_offline(cpu))

? Because I think we only want to queue the barrier call back if there
are callbacks for a particular CPU. Am I missing something subtle?

Regards,
Boqun

> +			rcu_barrier_trace(TPS("OfflineNoCBQ"), cpu,
> +					  rcu_state.barrier_sequence);
> +			local_irq_disable();
> +			rcu_barrier_func((void *)cpu);
> +			local_irq_enable();
>  		} else {
>  			rcu_barrier_trace(TPS("OnlineNQ"), cpu,
>  					  rcu_state.barrier_sequence);
> @@ -3184,7 +3192,7 @@ void rcu_barrier(void)
>  	 * Now that we have an rcu_barrier_callback() callback on each
>  	 * CPU, and thus each counted, remove the initial count.
>  	 */
> -	if (atomic_dec_and_test(&rcu_state.barrier_cpu_count))
> +	if (atomic_sub_and_test(2, &rcu_state.barrier_cpu_count))
>  		complete(&rcu_state.barrier_completion);
>  
>  	/* Wait for all rcu_barrier_callback() callbacks to be invoked. */
> -- 
> 2.9.5
> 



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux