Re: [PATCH] stop_machine: call rcu_momentary_dyntick_idle() from irq disable path

From: Paul E. McKenney
Date: Tue Apr 09 2024 - 14:16:54 EST

Next message: Konrad Dybcio: "Re: [PATCH v3 03/19] media: venus: pm_helpers: Add kerneldoc to venus_clks_get()"
Previous message: Dmitry Baryshkov: "Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value"
In reply to: Mukesh Ojha: "[PATCH] stop_machine: call rcu_momentary_dyntick_idle() from irq disable path"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Apr 09, 2024 at 01:32:16PM +0530, Mukesh Ojha wrote:
> rcu_momentary_dyntick_idle() is getting called from all the irq
> disable path(also documented in rcu_momentary_dyntick_idle())
> however, in multi_cpu_stop() it is not.
>
> "
>
> * Let the RCU core know that this CPU has gone through the scheduler,
> * which is a quiescent state. This is called when the need for a
> * quiescent state is urgent, so we burn an atomic operation and full
> * memory barriers to let the RCU core know about it, regardless of what
> * this CPU might (or might not) do in the near future.
> *
> * We inform the RCU core by emulating a zero-duration dyntick-idle period.
> *
> * The caller must have disabled interrupts and must not be idle.
>
> "
>
>
> Let's fix this as it is possible that during stop_machine() call for
> a kprobe registration[1] can get stuck when one cpu(cpu4) calls
> rcu_momentary_dyntick_idle() after acking MULTI_STOP_PREPARE state[2]
> and other cpus have acked MULTI_STOP_DISABLE_IRQ and waiting for earlier
> cpu to reach to next state [3].

Good eyes!

Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

> [1]
>
> [<ffffffdb78c75c54>] __switch_to+0x1e8
> [<ffffffdb78c76520>] __schedule+0x6bc
> [<ffffffdb78c76af0>] preempt_schedule_common+0xb4
> [<ffffffdb78c75e60>] preempt_schedule[jt]+0x20
> [<ffffffdb77dd7bd0>] queue_stop_cpus_work[jt]+0xdc
> [<ffffffdb77dd781c>] stop_machine_cpuslocked+0xdc
> [<ffffffdb78c816f8>] aarch64_insn_patch_text[jt]+0x4c
> [<ffffffdb78c81938>] arch_arm_kprobe+0x40
> [<ffffffdb77de88c0>] arm_kprobe+0x3c
> [<ffffffdb77de84f4>] register_kprobe+0x3fc
>
>
> [2]
> 000|check_preemption_disabled(what1 = 0xFFFFFFDB79153D0B, what2 = 0xFFFFFFDB7912A911)
> | what1 = 0xFFFFFFDB79153D0B
> | what2 = 0xFFFFFFDB7912A911
> -001|__this_cpu_preempt_check(op = ?)
> -002|rcu_preempt_need_deferred_qs(inline)
> | t = 0xFFFFFF8806879F80
> | pscr_ret__ = 0
> -002|rcu_preempt_deferred_qs(inline)
> | t = 0xFFFFFF8806879F80
> | flags = 0
> -002|rcu_momentary_dyntick_idle()
> -003|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_PREPARE ==> This seems to have acked MULTI_STOP_PREPARE
> state and calls rcu_momentary_dyntick_idle()
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -004|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6D812F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -005|smpboot_thread_fn(:data = 0xFFFFFF8805C063C0)
> | data = 0xFFFFFF8805C063C0
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -006|kthread(_create = 0xFFFFFF8806B7EC00)
> | _create = 0xFFFFFF8806B7EC00
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C063C0
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806402F00
> | done = 0xFFFFFFC08005BA98
> -007|ret_from_fork(asm)
>
> [3]
> 000|touch_softlockup_watchdog()
> -001|touch_nmi_watchdog(inline)
> -001|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | newstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -002|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6D04EF28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -003|smpboot_thread_fn(:data = 0xFFFFFF8805C06230)
> | data = 0xFFFFFF8805C06230
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -004|kthread(_create = 0xFFFFFF8806469D80)
> | _create = 0xFFFFFF8806469D80
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C06230
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806401500
> | done = 0xFFFFFFC08005B888
> -005|ret_from_fork(asm)
> ---|end of frame
>
> cpu1:
>
> -000|touch_softlockup_watchdog()
> -001|touch_nmi_watchdog(inline)
> -001|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | newstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -002|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6D23FF28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -003|smpboot_thread_fn(:data = 0xFFFFFF8805C06270)
> | data = 0xFFFFFF8805C06270
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -004|kthread(_create = 0xFFFFFF88066119C0)
> | _create = 0xFFFFFF88066119C0
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C06270
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806401000
> | done = 0xFFFFFFC08005BA98
> -005|ret_from_fork(asm)
> ---|end of frame
>
> cpu2:
>
> -000|touch_softlockup_watchdog()
> -001|touch_nmi_watchdog(inline)
> -001|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | newstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -002|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6D430F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -003|smpboot_thread_fn(:data = 0xFFFFFF8805C062E0)
> | data = 0xFFFFFF8805C062E0
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -004|kthread(_create = 0xFFFFFF88067D5B80)
> | _create = 0xFFFFFF88067D5B80
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C062E0
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806403F00
> | done = 0xFFFFFFC08005BA98
> -005|ret_from_fork(asm)
> ---|end of frame
>
> cpu3:
>
> -000|touch_softlockup_watchdog_sched(inline)
> | __ptr = 18446743916840366040
> -000|touch_softlockup_watchdog()
> -001|touch_nmi_watchdog(inline)
> -001|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | newstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -002|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6D621F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -003|smpboot_thread_fn(:data = 0xFFFFFF8805C06350)
> | data = 0xFFFFFF8805C06350
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -004|kthread(_create = 0xFFFFFF88069E7300)
> | _create = 0xFFFFFF88069E7300
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C06350
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806403300
> | done = 0xFFFFFFC08005BA98
> -005|ret_from_fork(asm)
> ---|end of frame
>
>
> Cpu5:
>
> -000|check_preemption_disabled(what1 = ?, what2 = ?)
> | what1 = ?
> | what2 = ?
> -001|debug_smp_processor_id()
> -002|ct_state_inc(inline)
> | incby = 8
> | __ptr = 0
> -002|rcu_momentary_dyntick_idle()
> | seq = 0
> -003|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -004|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6DA03F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -005|smpboot_thread_fn(:data = 0xFFFFFF8805C06430)
> | data = 0xFFFFFF8805C06430
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -006|kthread(_create = 0xFFFFFF8806D58180)
> | _create = 0xFFFFFF8806D58180
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C06430
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806402400
> | done = 0xFFFFFFC08005BA98
> -007|ret_from_fork(asm)
> ---|end of frame
>
> Cpu6:
>
> -000|check_preemption_disabled(what1 = ?, what2 = ?)
> | what1 = ?
> | what2 = ?
> -001|__this_cpu_preempt_check(op = ?)
> -002|rcu_preempt_need_deferred_qs(inline)
> | t = 0xFFFFFF8806E03F00
> | pscr_ret__ = 0
> -002|rcu_preempt_deferred_qs(inline)
> | t = 0xFFFFFF8806E03F00
> | flags = 0
> -002|rcu_momentary_dyntick_idle()
> -003|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -004|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6DBF4F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -005|smpboot_thread_fn(:data = 0xFFFFFF8805C064A0)
> | data = 0xFFFFFF8805C064A0
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -006|kthread(_create = 0xFFFFFF8806F67D00)
> | _create = 0xFFFFFF8806F67D00
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C064A0
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806402600
> | done = 0xFFFFFFC08005BA98
> -007|ret_from_fork(asm)
> ---|end of frame
>
> Cpu7:
>
> -000|check_preemption_disabled(what1 = ?, what2 = ?)
> | what1 = ?
> | what2 = ?
> -001|__this_cpu_preempt_check(op = ?)
> -002|rcu_preempt_need_deferred_qs(inline)
> | t = 0xFFFFFF8806E1DE80
> | pscr_ret__ = 0
> -002|rcu_preempt_deferred_qs(inline)
> | t = 0xFFFFFF8806E1DE80
> | flags = 0
> -002|rcu_momentary_dyntick_idle()
> -003|multi_cpu_stop(:data = 0xFFFFFFC084343478)
> | data = 0xFFFFFFC084343478
> | msdata = 0x0
> | curstate = MULTI_STOP_DISABLE_IRQ
> | flags = 0
> | is_active = TRUE
> | cpumask = 0xFFFFFFDB79E37718
> | err = 0
> -004|cpu_stopper_thread(cpu = ?)
> | cpu = ?
> | __already_done = FALSE
> | stopper = 0xFFFFFF8B6DDE5F28
> | fn = 0xFFFFFFDB77DD74C4
> | arg = 0xFFFFFFC084343478
> | done = 0xFFFFFFC0843434A0
> | ret = ???
> -005|smpboot_thread_fn(:data = 0xFFFFFF8805C06510)
> | data = 0xFFFFFF8805C06510
> | td = 0x0
> | ht = 0xFFFFFFDB79EE8B40
> -006|kthread(_create = 0xFFFFFF88070FF300)
> | _create = 0xFFFFFF88070FF300
> | param = (sched_priority = 0)
> | create = 0x0
> | data = 0xFFFFFF8805C06510
> | threadfn = 0x0
> | ret = ???
> | self = 0xFFFFFF8806407D00
> | done = 0xFFFFFFC08005BA98
> -007|ret_from_fork(asm)
> ---|end of frame
>
>
> Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
> ---
> kernel/stop_machine.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
> index cedb17ba158a..f93e6deb8150 100644
> --- a/kernel/stop_machine.c
> +++ b/kernel/stop_machine.c
> @@ -250,8 +250,8 @@ static int multi_cpu_stop(void *data)
> * be detected and reported on their side.
> */
> touch_nmi_watchdog();
> + rcu_momentary_dyntick_idle();
> }
> - rcu_momentary_dyntick_idle();
> } while (curstate != MULTI_STOP_EXIT);
>
> local_irq_restore(flags);
> --
> 2.7.4
>

Next message: Konrad Dybcio: "Re: [PATCH v3 03/19] media: venus: pm_helpers: Add kerneldoc to venus_clks_get()"
Previous message: Dmitry Baryshkov: "Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value"
In reply to: Mukesh Ojha: "[PATCH] stop_machine: call rcu_momentary_dyntick_idle() from irq disable path"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]