Re: [PATCH rcu 3/3] rcu: Allow expedited RCU grace periods on incoming CPUs

[Mukesh Ojha <quic_mojha@quicinc.com>]

On 2/15/2022 11:09 PM, Paul E. McKenney wrote:
> On Tue, Feb 15, 2022 at 07:53:10PM +0530, Mukesh Ojha wrote:
>> On 2/14/2022 10:14 PM, Paul E. McKenney wrote:
>>> On Thu, Feb 10, 2022 at 12:38:11AM +0100, Frederic Weisbecker wrote:
>>>> On Fri, Feb 04, 2022 at 02:55:07PM -0800, Paul E. McKenney wrote:
>>>>> Although it is usually safe to invoke synchronize_rcu_expedited() from a
>>>>> preemption-enabled CPU-hotplug notifier, if it is invoked from a notifier
>>>>> between CPUHP_AP_RCUTREE_ONLINE and CPUHP_AP_ACTIVE, its attempts to
>>>>> invoke a workqueue handler will hang due to RCU waiting on a CPU that
>>>>> the scheduler is not paying attention to.  This commit therefore expands
>>>>> use of the existing workqueue-independent synchronize_rcu_expedited()
>>>>> from early boot to also include CPUs that are being hotplugged.
>>>>>
>>>>> Link:https://lore.kernel.org/lkml/7359f994-8aaf-3cea-f5cf-c0d3929689d6@quicinc.com/
>>>>> Reported-by: Mukesh Ojha<quic_mojha@quicinc.com>
>>>>> Cc: Tejun Heo<tj@kernel.org>
>>>>> Signed-off-by: Paul E. McKenney<paulmck@kernel.org>
>>>> I'm surprised by this scheduler behaviour.
>>>>
>>>> Since sched_cpu_activate() hasn't been called yet,
>>>> rq->balance_callback = balance_push_callback. As a result, balance_push() should
>>>> be called at the end of schedule() when the workqueue is picked as the next task.
>>>> Then eventually the workqueue should be immediately preempted by the stop task to
>>>> be migrated elsewhere.
>>>>
>>>> So I must be missing something. For the fun, I booted the following and it
>>>> didn't produce any issue:
>>>>
>>>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>>>> index 80faf2273ce9..b1e74a508881 100644
>>>> --- a/kernel/rcu/tree.c
>>>> +++ b/kernel/rcu/tree.c
>>>> @@ -4234,6 +4234,8 @@ int rcutree_online_cpu(unsigned int cpu)
>>>>    	// Stop-machine done, so allow nohz_full to disable tick.
>>>>    	tick_dep_clear(TICK_DEP_BIT_RCU);
>>>> +	if (cpu != 0)
>>>> +		synchronize_rcu_expedited();
>>>>    	return 0;
>>>>    }
>>> That does seem compelling.  And others have argued that the workqueue
>>> system's handling of offline CPUs should deal with this.
>>>
>>> Mukesh, was this a theoretical bug, or did you actually make it happen?
>>> If you made it happen, as seems to have been the case given your original
>>> email [1], could you please post your reproducer?
>> No, it was not theoretical one. We saw this issue only once in our testing
>> and i don't think it is easy to reproduce otherwise
>> it would been fixed by now.
>>
>> When one of thread calling synchronize_expedite_rcu with timer of 20s but it
>> did not get the exp funnel
>> lock for 20s and there we crash it with panic() on timeout.
>>
>> The other thread cpuhp which was having the lock got stuck at the point
>> mentioned at the below link.
>>
>> https://lore.kernel.org/lkml/7359f994-8aaf-3cea-f5cf-c0d3929689d6@quicinc.com/
> OK.  Are you able to create an in-kernel reproducer, perhaps similar to
> Frederic's change above?
>
> I am worried that the patch that I am carrying might be fixing some
> other bug by accident...

Just for information, we are running on 5.10 kernel and after numerous 
attempt, i was not able to reproduce the issue:-)

Thanks,
-Mukesh

>
> 							Thanx, Paul
>
>> e.g Below sample test in combination of many other test in parallel
>>
>> :loop
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu0/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu0/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu1/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu1/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu2/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu2/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu3/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu3/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu4/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu4/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu5/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu5/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu6/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu6/online"
>>
>> adb shell "echo 0 > /sys/devices/system/cpu/cpu7/online"
>>
>> adb shell "echo 1 > /sys/devices/system/cpu/cpu7/online"
>>
>> goto loop
>>
>>
>>
>> Thanks, Mukesh
>>
>>> 							Thanx, Paul
>>>
>>> [1]https://lore.kernel.org/lkml/7359f994-8aaf-3cea-f5cf-c0d3929689d6@quicinc.com/