* [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep()
@ 2025-01-08 4:28 Koichiro Den
2025-01-09 5:16 ` Anshuman Khandual
2025-01-10 20:14 ` Charalampos Mitrodimas
0 siblings, 2 replies; 3+ messages in thread
From: Koichiro Den @ 2025-01-08 4:28 UTC (permalink / raw)
To: linux-mm; +Cc: akpm, lorenzo.stoakes, chenhuacai, linux-kernel
The upstream commit adcfb264c3ed ("vmstat: disable vmstat_work on
vmstat_cpu_down_prep()") introduced another warning during the boot phase
so was soon reverted on upstream by commit cd6313beaeae ("Revert "vmstat:
disable vmstat_work on vmstat_cpu_down_prep()""). This commit resolves it
and reattempts the original fix.
Even after mm/vmstat:online teardown, shepherd may still queue work for
the dying cpu until the cpu is removed from online mask. While it's quite
rare, this means that after unbind_workers() unbinds a per-cpu kworker, it
potentially runs vmstat_update for the dying CPU on an irrelevant cpu
before entering atomic AP states. When CONFIG_DEBUG_PREEMPT=y, it results
in the following error with the backtrace.
BUG: using smp_processor_id() in preemptible [00000000] code: \
kworker/7:3/1702
caller is refresh_cpu_vm_stats+0x235/0x5f0
CPU: 0 UID: 0 PID: 1702 Comm: kworker/7:3 Tainted: G
Tainted: [N]=TEST
Workqueue: mm_percpu_wq vmstat_update
Call Trace:
<TASK>
dump_stack_lvl+0x8d/0xb0
check_preemption_disabled+0xce/0xe0
refresh_cpu_vm_stats+0x235/0x5f0
vmstat_update+0x17/0xa0
process_one_work+0x869/0x1aa0
worker_thread+0x5e5/0x1100
kthread+0x29e/0x380
ret_from_fork+0x2d/0x70
ret_from_fork_asm+0x1a/0x30
</TASK>
So, for mm/vmstat:online, disable vmstat_work reliably on teardown and
symmetrically enable it on startup.
For secondary CPUs during CPU hotplug scenarios, ensure the delayed work
is disabled immediately after the initialization. These CPUs are not yet
online when start_shepherd_timer() runs on boot CPU. vmstat_cpu_online()
will enable the work for them.
Suggested-by: Huacai Chen <chenhuacai@kernel.org>
Signed-off-by: Huacai Chen <chenhuacai@kernel.org>
Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
---
v2: https://lore.kernel.org/all/20241221033321.4154409-1-koichiro.den@canonical.com/
v1: https://lore.kernel.org/all/20241220134234.3809621-1-koichiro.den@canonical.com/
---
mm/vmstat.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4d016314a56c..16bfe1c694dd 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -2122,10 +2122,20 @@ static void __init start_shepherd_timer(void)
{
int cpu;
- for_each_possible_cpu(cpu)
+ for_each_possible_cpu(cpu) {
INIT_DEFERRABLE_WORK(per_cpu_ptr(&vmstat_work, cpu),
vmstat_update);
+ /*
+ * For secondary CPUs during CPU hotplug scenarios,
+ * vmstat_cpu_online() will enable the work.
+ * mm/vmstat:online enables and disables vmstat_work
+ * symmetrically during CPU hotplug events.
+ */
+ if (!cpu_online(cpu))
+ disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
+ }
+
schedule_delayed_work(&shepherd,
round_jiffies_relative(sysctl_stat_interval));
}
@@ -2148,13 +2158,14 @@ static int vmstat_cpu_online(unsigned int cpu)
if (!node_state(cpu_to_node(cpu), N_CPU)) {
node_set_state(cpu_to_node(cpu), N_CPU);
}
+ enable_delayed_work(&per_cpu(vmstat_work, cpu));
return 0;
}
static int vmstat_cpu_down_prep(unsigned int cpu)
{
- cancel_delayed_work_sync(&per_cpu(vmstat_work, cpu));
+ disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep()
2025-01-08 4:28 [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep() Koichiro Den
@ 2025-01-09 5:16 ` Anshuman Khandual
2025-01-10 20:14 ` Charalampos Mitrodimas
1 sibling, 0 replies; 3+ messages in thread
From: Anshuman Khandual @ 2025-01-09 5:16 UTC (permalink / raw)
To: Koichiro Den, linux-mm; +Cc: akpm, lorenzo.stoakes, chenhuacai, linux-kernel
On 1/8/25 09:58, Koichiro Den wrote:
> The upstream commit adcfb264c3ed ("vmstat: disable vmstat_work on
> vmstat_cpu_down_prep()") introduced another warning during the boot phase
I did observe this warning during a boot on arm64 and a quick git bisect also
pointed to the above mentioned commit. But seems like you are already on this
problem.
[ 0.092532] ------------[ cut here ]------------
[ 0.092534] workqueue: work disable count underflowed
[ 0.092540] WARNING: CPU: 1 PID: 21 at kernel/workqueue.c:4313 enable_work+0xe8/0x100
[ 0.092550] Modules linked in:
[ 0.092554] CPU: 1 UID: 0 PID: 21 Comm: cpuhp/1 Not tainted 6.13.0-rc4-00018-gadcfb264c3ed #11
[ 0.092558] Hardware name: linux,dummy-virt (DT)
[ 0.092560] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.092564] pc : enable_work+0xe8/0x100
[ 0.092566] lr : enable_work+0xe8/0x100
[ 0.092569] sp : ffff800082de3d50
[ 0.092570] x29: ffff800082de3d50 x28: 0000000000000000 x27: 0000000000000000
[ 0.092574] x26: 0000000000000000 x25: 0000000000000000 x24: ffff00008001a1d0
[ 0.092578] x23: 0000000000000001 x22: 0000000000000001 x21: ffff0001ff2de110
[ 0.092582] x20: ffff8000827506a0 x19: ffff0001ff2e4d28 x18: 0000000000000038
[ 0.092585] x17: 000000040044ffff x16: 00500074b5503510 x15: fffffffffffe1698
[ 0.092589] x14: ffff8000827547d8 x13: 00000000000000ff x12: 0000000000000055
[ 0.092592] x11: fffffffffffe1698 x10: ffff8000827ac7d8 x9 : 00000000fffff000
[ 0.092596] x8 : ffff8000827547d8 x7 : ffff8000827ac7d8 x6 : 0000000000000000
[ 0.092599] x5 : 80000000fffff000 x4 : 000000000000aff5 x3 : 0000000000000000
[ 0.092603] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000080270000
[ 0.092607] Call trace:
[ 0.092608] enable_work+0xe8/0x100 (P)
[ 0.092612] enable_delayed_work+0x10/0x1c
[ 0.092615] vmstat_cpu_online+0x84/0xb8
[ 0.092621] cpuhp_invoke_callback+0x104/0x20c
[ 0.092626] cpuhp_thread_fun+0xa4/0x17c
[ 0.092629] smpboot_thread_fn+0x220/0x24c
[ 0.092635] kthread+0x118/0x11c
[ 0.092640] ret_from_fork+0x10/0x20
[ 0.092645] ---[ end trace 0000000000000000 ]---
> so was soon reverted on upstream by commit cd6313beaeae ("Revert "vmstat:
> disable vmstat_work on vmstat_cpu_down_prep()""). This commit resolves it
> and reattempts the original fix.
>
> Even after mm/vmstat:online teardown, shepherd may still queue work for
> the dying cpu until the cpu is removed from online mask. While it's quite
> rare, this means that after unbind_workers() unbinds a per-cpu kworker, it
> potentially runs vmstat_update for the dying CPU on an irrelevant cpu
> before entering atomic AP states. When CONFIG_DEBUG_PREEMPT=y, it results
> in the following error with the backtrace.
>
> BUG: using smp_processor_id() in preemptible [00000000] code: \
> kworker/7:3/1702
> caller is refresh_cpu_vm_stats+0x235/0x5f0
> CPU: 0 UID: 0 PID: 1702 Comm: kworker/7:3 Tainted: G
> Tainted: [N]=TEST
> Workqueue: mm_percpu_wq vmstat_update
> Call Trace:
> <TASK>
> dump_stack_lvl+0x8d/0xb0
> check_preemption_disabled+0xce/0xe0
> refresh_cpu_vm_stats+0x235/0x5f0
> vmstat_update+0x17/0xa0
> process_one_work+0x869/0x1aa0
> worker_thread+0x5e5/0x1100
> kthread+0x29e/0x380
> ret_from_fork+0x2d/0x70
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> So, for mm/vmstat:online, disable vmstat_work reliably on teardown and
> symmetrically enable it on startup.
>
> For secondary CPUs during CPU hotplug scenarios, ensure the delayed work
> is disabled immediately after the initialization. These CPUs are not yet
> online when start_shepherd_timer() runs on boot CPU. vmstat_cpu_online()
> will enable the work for them.
>
> Suggested-by: Huacai Chen <chenhuacai@kernel.org>
> Signed-off-by: Huacai Chen <chenhuacai@kernel.org>
> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
> ---
> v2: https://lore.kernel.org/all/20241221033321.4154409-1-koichiro.den@canonical.com/
> v1: https://lore.kernel.org/all/20241220134234.3809621-1-koichiro.den@canonical.com/
> ---
> mm/vmstat.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 4d016314a56c..16bfe1c694dd 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -2122,10 +2122,20 @@ static void __init start_shepherd_timer(void)
> {
> int cpu;
>
> - for_each_possible_cpu(cpu)
> + for_each_possible_cpu(cpu) {
> INIT_DEFERRABLE_WORK(per_cpu_ptr(&vmstat_work, cpu),
> vmstat_update);
>
> + /*
> + * For secondary CPUs during CPU hotplug scenarios,
> + * vmstat_cpu_online() will enable the work.
> + * mm/vmstat:online enables and disables vmstat_work
> + * symmetrically during CPU hotplug events.
> + */
> + if (!cpu_online(cpu))
> + disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> + }
> +
> schedule_delayed_work(&shepherd,
> round_jiffies_relative(sysctl_stat_interval));
> }
> @@ -2148,13 +2158,14 @@ static int vmstat_cpu_online(unsigned int cpu)
> if (!node_state(cpu_to_node(cpu), N_CPU)) {
> node_set_state(cpu_to_node(cpu), N_CPU);
> }
> + enable_delayed_work(&per_cpu(vmstat_work, cpu));
>
> return 0;
> }
>
> static int vmstat_cpu_down_prep(unsigned int cpu)
> {
> - cancel_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> + disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> return 0;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep()
2025-01-08 4:28 [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep() Koichiro Den
2025-01-09 5:16 ` Anshuman Khandual
@ 2025-01-10 20:14 ` Charalampos Mitrodimas
1 sibling, 0 replies; 3+ messages in thread
From: Charalampos Mitrodimas @ 2025-01-10 20:14 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-mm, akpm, lorenzo.stoakes, chenhuacai, linux-kernel
Koichiro Den <koichiro.den@canonical.com> writes:
> The upstream commit adcfb264c3ed ("vmstat: disable vmstat_work on
> vmstat_cpu_down_prep()") introduced another warning during the boot phase
> so was soon reverted on upstream by commit cd6313beaeae ("Revert "vmstat:
> disable vmstat_work on vmstat_cpu_down_prep()""). This commit resolves it
> and reattempts the original fix.
>
> Even after mm/vmstat:online teardown, shepherd may still queue work for
> the dying cpu until the cpu is removed from online mask. While it's quite
> rare, this means that after unbind_workers() unbinds a per-cpu kworker, it
> potentially runs vmstat_update for the dying CPU on an irrelevant cpu
> before entering atomic AP states. When CONFIG_DEBUG_PREEMPT=y, it results
> in the following error with the backtrace.
>
> BUG: using smp_processor_id() in preemptible [00000000] code: \
> kworker/7:3/1702
> caller is refresh_cpu_vm_stats+0x235/0x5f0
> CPU: 0 UID: 0 PID: 1702 Comm: kworker/7:3 Tainted: G
> Tainted: [N]=TEST
> Workqueue: mm_percpu_wq vmstat_update
> Call Trace:
> <TASK>
> dump_stack_lvl+0x8d/0xb0
> check_preemption_disabled+0xce/0xe0
> refresh_cpu_vm_stats+0x235/0x5f0
> vmstat_update+0x17/0xa0
> process_one_work+0x869/0x1aa0
> worker_thread+0x5e5/0x1100
> kthread+0x29e/0x380
> ret_from_fork+0x2d/0x70
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> So, for mm/vmstat:online, disable vmstat_work reliably on teardown and
> symmetrically enable it on startup.
>
> For secondary CPUs during CPU hotplug scenarios, ensure the delayed work
> is disabled immediately after the initialization. These CPUs are not yet
> online when start_shepherd_timer() runs on boot CPU. vmstat_cpu_online()
> will enable the work for them.
>
> Suggested-by: Huacai Chen <chenhuacai@kernel.org>
> Signed-off-by: Huacai Chen <chenhuacai@kernel.org>
> Signed-off-by: Koichiro Den <koichiro.den@canonical.com>
> ---
> v2: https://lore.kernel.org/all/20241221033321.4154409-1-koichiro.den@canonical.com/
> v1: https://lore.kernel.org/all/20241220134234.3809621-1-koichiro.den@canonical.com/
> ---
> mm/vmstat.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 4d016314a56c..16bfe1c694dd 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -2122,10 +2122,20 @@ static void __init start_shepherd_timer(void)
> {
> int cpu;
>
> - for_each_possible_cpu(cpu)
> + for_each_possible_cpu(cpu) {
> INIT_DEFERRABLE_WORK(per_cpu_ptr(&vmstat_work, cpu),
> vmstat_update);
>
> + /*
> + * For secondary CPUs during CPU hotplug scenarios,
> + * vmstat_cpu_online() will enable the work.
> + * mm/vmstat:online enables and disables vmstat_work
> + * symmetrically during CPU hotplug events.
> + */
> + if (!cpu_online(cpu))
> + disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> + }
> +
> schedule_delayed_work(&shepherd,
> round_jiffies_relative(sysctl_stat_interval));
> }
> @@ -2148,13 +2158,14 @@ static int vmstat_cpu_online(unsigned int cpu)
> if (!node_state(cpu_to_node(cpu), N_CPU)) {
> node_set_state(cpu_to_node(cpu), N_CPU);
> }
> + enable_delayed_work(&per_cpu(vmstat_work, cpu));
>
> return 0;
> }
>
> static int vmstat_cpu_down_prep(unsigned int cpu)
> {
> - cancel_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> + disable_delayed_work_sync(&per_cpu(vmstat_work, cpu));
> return 0;
> }
Hi Koichiro,
Tested this version of your patch and it seems to be working as expected
on my x86_64 QEMU.
Tested-by: Charalampos Mitrodimas <charmitro@posteo.net>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-10 20:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-08 4:28 [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep() Koichiro Den
2025-01-09 5:16 ` Anshuman Khandual
2025-01-10 20:14 ` Charalampos Mitrodimas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox