| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0b6763fa-6225-436a-a24a-c6b4029c0d68@arm.com>
Date: Thu, 9 Jan 2025 10:46:08 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Koichiro Den <koichiro.den@...onical.com>, linux-mm@...ck.org
Cc: akpm@...ux-foundation.org, lorenzo.stoakes@...cle.com,
chenhuacai@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] vmstat: disable vmstat_work on vmstat_cpu_down_prep()
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@...nel.org>
> Signed-off-by: Huacai Chen <chenhuacai@...nel.org>
> Signed-off-by: Koichiro Den <koichiro.den@...onical.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;
> }
>
Powered by blists - more mailing lists