Re: [PATCH v2] rcu: Remove redundant rcu_is_cpu_rrupt_from_idle() from tiny RCU

[Paul E. McKenney]

On Fri, Oct 31, 2014 at 02:55:05PM +0000, Alexander Gordeev wrote:
> Let's start assuming that something in the idle loop posts a callback,
> and scheduling-clock interrupt occurs:
> 
> 1. The system is idle and stays that way, no runnable tasks.
> 
> 2. Scheduling-clock interrupt occurs, rcu_check_callbacks() is called
>    as result, which in turn calls rcu_is_cpu_rrupt_from_idle().
> 
> 3. rcu_is_cpu_rrupt_from_idle() reports the CPU was interrupted from
>    idle, which results in rcu_sched_qs() call, which does a
>    raise_softirq(RCU_SOFTIRQ).
> 
> 4. Upon return from interrupt, rcu_irq_exit() is invoked, which calls
>    rcu_idle_enter_common(), which in turn calls rcu_sched_qs() again,
>    which does another raise_softirq(RCU_SOFTIRQ).
> 
> 5. The softirq happens shortly and invokes rcu_process_callbacks(),
>    which invokes __rcu_process_callbacks().
> 
> 6. So now callbacks can be invoked. At least they can be if
>    ->donetail has been updated. Which it will have been because
>    rcu_sched_qs() invokes rcu_qsctr_help().
> 
> In the described scenario rcu_sched_qs() and raise_softirq(RCU_SOFTIRQ)
> get called twice in steps 3 and 4. This redundancy could be eliminated
> by removing rcu_is_cpu_rrupt_from_idle() function.
> 
> Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

Queued for 3.20, thank you!

							Thanx, Paul

> ---
>  kernel/rcu/tiny.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index c0623fc..0bfb3b9 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -186,16 +186,6 @@ EXPORT_SYMBOL(__rcu_is_watching);
>  #endif /* defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_RCU_TRACE) */
> 
>  /*
> - * Test whether the current CPU was interrupted from idle.  Nested
> - * interrupts don't count, we must be running at the first interrupt
> - * level.
> - */
> -static int rcu_is_cpu_rrupt_from_idle(void)
> -{
> -	return rcu_dynticks_nesting <= 1;
> -}
> -
> -/*
>   * Helper function for rcu_sched_qs() and rcu_bh_qs().
>   * Also irqs are disabled to avoid confusion due to interrupt handlers
>   * invoking call_rcu().
> @@ -250,7 +240,7 @@ void rcu_bh_qs(void)
>  void rcu_check_callbacks(int user)
>  {
>  	RCU_TRACE(check_cpu_stalls());
> -	if (user || rcu_is_cpu_rrupt_from_idle())
> +	if (user)
>  		rcu_sched_qs();
>  	else if (!in_softirq())
>  		rcu_bh_qs();
> -- 
> 1.8.3.1
> 
> -- 
> Regards,
> Alexander Gordeev
> agordeev@xxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/