On 02/16/2015 11:20 AM, Paul E. McKenney wrote:
> On Mon, Feb 16, 2015 at 08:49:12AM +0530, Preeti U Murthy wrote:
>> 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 <pre...@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 <pre...@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 <pre...@linux.vnet.ibm.com>
>>>
>>> Reviewed-by: Paul E. McKenney <paul...@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 ?
>
> And the other advantage of doing it the way you did (and I -did- give
> you a Reviewed-by!) is that you are incurring the extra overhead from
> the idle loop, where that extra overhead is less likely to be holding
> something else up. So, yes, I do agree with your patch.
Ok. I was just unsure which approach you meant by 'this' in the above
statement 'The advantage of this approach is less RCU-notification
overhead..'. Hence wanted to make sure that I had understood you right.
Alright, I'll send out this patch to tglx with your Reviewed-by then.
Thanks!
Regards
Preeti U Murthy
> Thanx, Paul
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
