Re: suspicious RCU usage clockevents_lock, tick_broadcast_lock, hrtimer_bases.lock

[Preeti U Murthy <preeti@linux.vnet.ibm.com>]

On 02/13/2015 07:56 PM, Paul E. McKenney wrote:
> On Fri, Feb 13, 2015 at 12:52:45PM +0530, Preeti U Murthy wrote:
>> On 02/13/2015 10:57 AM, Preeti U Murthy wrote:
>>> On 02/13/2015 06:27 AM, Sam Bobroff wrote:
>>>> Hello,
>>>>
>>>> I'm receiving this while booting a vanilla 3.19 kernel on a Power 8 machine:
>>>
>>> Does the below patch fix the issue ?
>>>
>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>>
>>> [PATCH] tick/hrtimer-broadcast: Fix a suspicious RCU usage in the tick broadcast path
>>>
>>> ---
>>>  kernel/time/tick-broadcast-hrtimer.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c
>>> index eb682d5..57b8e32 100644
>>> --- a/kernel/time/tick-broadcast-hrtimer.c
>>> +++ b/kernel/time/tick-broadcast-hrtimer.c
>>> @@ -62,7 +62,7 @@ static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
>>>          * HRTIMER_RESTART.
>>>          */
>>>         if (hrtimer_try_to_cancel(&bctimer) >= 0) {
>>> -               hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
>>> +               RCU_NONIDLE(hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED));
>>>                 /* Bind the "device" to the cpu */
>>>                 bc->bound_on = smp_processor_id();
>>>         } else if (bc->bound_on == smp_processor_id()) {
>>>
>> Actually the below patch is the complete fix. Paul can you please
>> review this ?  As an alternate solution I checked to see if its
>> possible to move rcu_idle_enter()/exit() closer to the
>> cpuidle_enter() call, but that won't work as you may have already
>> tried earlier.
>>
>> -----------------------------------------------------------------
>>
>> tick/broadcast-hrtimer : Fix suspicious RCU usage in idle loop
>>
>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>
>> The hrtimer mode of broadcast queues hrtimers in the idle entry
>> path so as to wakeup cpus in deep idle states. hrtimer_{start/cancel}
>> functions call into tracing which uses RCU. But it is not legal to call
>> into RCU in cpuidle because it is one of the quiescent states. Hence
>> protect this region with RCU_NONIDLE which informs RCU that the cpu
>> is momentarily non-idle.
>>
>> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
> 
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Another alternative would be to change the hrtimer_{start/cancel}()
> functions' tracepoints to the _rcuidle form.  The advantage of this
> approach is less RCU-notification overhead when tracing is enabled.

But since the hrtimer_{start/cancel} functions' tracepoints are more
often called from paths which are in the non-quiescent states, wouldn't
we be doing an rcu_irq_enter/exit() redundantly far too often in that case ?

Regards
Preeti U Murthy