From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA59AE77188 for ; Mon, 6 Jan 2025 13:53:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4E8186B008A; Mon, 6 Jan 2025 08:53:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4988F6B008C; Mon, 6 Jan 2025 08:53:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 338EF6B0093; Mon, 6 Jan 2025 08:53:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 122FF6B008A for ; Mon, 6 Jan 2025 08:53:51 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id B29C912152E for ; Mon, 6 Jan 2025 13:53:50 +0000 (UTC) X-FDA: 82977170220.01.9078EF3 Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) by imf02.hostedemail.com (Postfix) with ESMTP id 868EA80004 for ; Mon, 6 Jan 2025 13:53:48 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=J5OFsvDH; dmarc=pass (policy=none) header.from=canonical.com; spf=pass (imf02.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.122 as permitted sender) smtp.mailfrom=koichiro.den@canonical.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736171628; a=rsa-sha256; cv=none; b=MwAhMGFGBWZAdxHTAUFsETkF18M2+MWufOG5BH9wTLkpuHyoUn9mKj1zWJPJ/jGFMuLlOU xG2rnwu289EXi+ZKQrjcd7mkpl4kSL7TWA3mj9aKbCYSupdhG7GXwF1uzIlLq6EzeY3maW mz/EVh/KSMQSLrkdcbtCh/Ijn9vYvQU= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=J5OFsvDH; dmarc=pass (policy=none) header.from=canonical.com; spf=pass (imf02.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.122 as permitted sender) smtp.mailfrom=koichiro.den@canonical.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736171628; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Y80W8rYXG1krv1n2HbnGYmUoOVrU8/hZxYmlkGgkX20=; b=GucHjhwRt9IPz77YrjN/0GljF3H0O5nUmYRpqHbXgb7YlZUJ8pfPyxgEzUhvUAZMgoaxE0 8auOgL1XX3Q/8kNvaWFDUr8Jx19I16AahCVCMvCHAAfGd+oqe/uWp2aPQz+AunkxG3dSY0 263TzsgDBxTpQ0WwufqR56oKJ/iJOHw= Received: from mail-pl1-f199.google.com (mail-pl1-f199.google.com [209.85.214.199]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id D62D73F480 for ; Mon, 6 Jan 2025 13:53:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1736171627; bh=Y80W8rYXG1krv1n2HbnGYmUoOVrU8/hZxYmlkGgkX20=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:In-Reply-To; b=J5OFsvDH1XSVT+bmfPzyWim+foS4Bao0lBIbwFu3qHtHDM1FyegvJ9UkZLh/zTQO6 6FyP6cr0nQGLOPpskiB3/qeOtXlPKYtP/CUa3Q3ORMkz8KYEyp2XQ/XQGiu7kCNzPZ 4qnr1Lc0IXB35giBBxRJwYNDeWkwkt2B2DBklAclsHfKdHSYGBckhH4X7eQrHY6yPD PqW8UnNEB4RfVoGCgasP6WYlbovDhB2edOo4oH12v5YwLbG4xBb3D9eSOv6FL7g0Nb KHLZq0F/dMNbGwzX8M5TVP8C2z3XT6WRSMIX86P0dXkmGATstQrgsYpM/2lems25XC iJ+xCMuN+aObw== Received: by mail-pl1-f199.google.com with SMTP id d9443c01a7336-2165433e229so207500625ad.1 for ; Mon, 06 Jan 2025 05:53:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736171625; x=1736776425; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Y80W8rYXG1krv1n2HbnGYmUoOVrU8/hZxYmlkGgkX20=; b=rjtI6wbUt1LNL/X8bv9KeUmiDDNOKnoEcA0iAOL7l+CBzVLSlwvznQpvw5BpO6TxKb w/J6MbpJCpa7y0L8JyaBn8eMx99V1aauDrLCd6SamJqqBIutAz6dWVsbOyvuR3xLWYZc 4iPXZFo8JmHlc2WT85bpepevcNYDQgxxEgbr+aaJYahHx9Mx2O0ZlI7QhFbtczFi1y2p zD8ZupWlH8g/vUkP2qFVqgBak9VuN1OD0KMN1G/jlLYPK/igydJ7Z43fEgJrPw/ZiOaw lfJZ2ust5NaRVPAfEUv6OSCP8iuVuwfgcw+bHZn3mqLenRVWSxeJrFkhYj1uES5aMlij vc4w== X-Gm-Message-State: AOJu0Yxiovf8+gAQCU2faiZfILAINvL9w1zQSAP5YVYUd1o0wksaamDC qzNkHPJiFJeqbjnXZ9xXOaG1iDq4nPy5JggEO5LzmuP9erdOw9B1qgkc1h72nJTkWjD+rw1HHma VEipgBvdClUbdH9twQrPPKSA1Uwb8PLsLGQGzA0GLCX7fTuhTBmX3NrrxFTF/1je5tD7+WTJ3 X-Gm-Gg: ASbGncvErO3ceBVpzlpJA5XtEecMP+ZWfgNlV98RdtT00euWfEwAkdKPy2YhKCMDQkT 8ke2IJPif7g+7ySV8z2e0wrKszHIKJ6Jbpo8Td7ltnW2aQzPF6unnRlURCa7W4RekmqcJKPs2hv B+WSceXWXptnk5+NefFoFK10XWvEiZ79flPH5z+8ORgnnNFr/j4/+iYxHdBPvpRt0Cz0hUVvGhY Ye4iHf1l/5Lx8dKDzCOBT5067SrjWlbgPoxoPQRSY+y5mJp1a/dney/ X-Received: by 2002:a17:902:dac6:b0:216:59d4:40e7 with SMTP id d9443c01a7336-219e6f25e08mr575482055ad.55.1736171625069; Mon, 06 Jan 2025 05:53:45 -0800 (PST) X-Google-Smtp-Source: AGHT+IH3iIflsAQ2w322Xsmg0dELrXrVQZsftm0bALTC07XAVv3rjKVFUzz9JO+vtuTtB20HVSU48Q== X-Received: by 2002:a17:902:dac6:b0:216:59d4:40e7 with SMTP id d9443c01a7336-219e6f25e08mr575481875ad.55.1736171624653; Mon, 06 Jan 2025 05:53:44 -0800 (PST) Received: from localhost ([240f:74:7be:1:de1c:d045:9b:980c]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc962c8csm294853575ad.49.2025.01.06.05.53.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jan 2025 05:53:44 -0800 (PST) Date: Mon, 6 Jan 2025 22:53:42 +0900 From: Koichiro Den To: Lorenzo Stoakes Cc: linux-mm@kvack.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, Thomas Gleixner , Peter Zijlstra Subject: Re: [PATCH v2] vmstat: disable vmstat_work on vmstat_cpu_down_prep() Message-ID: References: <20241221033321.4154409-1-koichiro.den@canonical.com> <2q7ge6cgzeowqffyn6w6ed4trhaaumv5ubdgud2tsoolen7wpw@4akuomhbacyh> <7ed97096-859e-46d0-8f27-16a2298a8914@lucifer.local> <7gd4skn6l4i3liu6cvzmgqlrwxc3rukrnau7lnpyhmfyjuvqwl@gdwgybedp5gs> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7gd4skn6l4i3liu6cvzmgqlrwxc3rukrnau7lnpyhmfyjuvqwl@gdwgybedp5gs> X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 868EA80004 X-Stat-Signature: gfg4onbrtiodctauxdbiqxkh7i4k1udk X-HE-Tag: 1736171628-356838 X-HE-Meta: U2FsdGVkX1+yMuEzUZsy9LfBU5SPOwXXE9Qx8KNajgXV1WAhWSz6yZfHcDVSKS8tgvHb5P43EuX3DGDKEZuJrIGy80AbAHv/L6JRQ9IN4mdKrl/EyGELW2C8YWDJiG2AgPMXjbfECvhW4Rrb7US/Zfg3Soo6HYhzuduqb1tBiUDuQAhNW0yRaKeVMEQqStVgGqEWFKRvJT6ZM9iXg6LzUOnMGZi0V1dFD7MpKZzo0KeQ7nYEleGJwlyfZeGFG5d3LNfYM7ecmmJjkHWtvd0QMb7H/cb1tfV6SI03pox/aKNyLQOog/uXrRg1BzT1GjaLR99pBSEufqQFh2bT/FNt+a5W7WljFyl5mw8i3nmeMd9vRTrFhA/lahDDdibEHOzUXj/k7mWihup6VS2bQCCHh0VOMSgwK7CkveUSfdsrMXcfk+qt0ipv6ZSlc474AB1nXuqE/E0ejS0eJaXMVXMgEye6MFLVZ+i2cS2LhratOW5779of5Ocp+onNMe9r7xws9xpX+CwE97ajC85JlvYS9vzSHMQ9Q7gKSDo+CiJAvni0BcSOEGeipokm55GN+zti1zpbFIKGjnKBp1zLnSXfAARPS0OIx12B59XsTT1ov03ESVlB7CTxP0u4rpFUFzxrjOaZBfE1UqikfpelXx4wBccpmnYuNPeNtSbToa8xEteqoZ26ZaoYMdTUhEnv6I4HpJbHeRVeOiNQTVw3gIpYVh1oLjem137OV/rlPQ8IOQ1fn2ZsU1TslV2A6craAaJabbK9NKFjw6hyS0B3/YwS/GHPQo2CHE3Mj/i7xg7fEzud1qtsGhdmWq+Dx8Uylcz/igklsmTnNwUj9kIrh6IrHiapDGqSrcrruiaWFjUzyznVuZAuQQTfE6LNLREW54YOSaVp/z2H3/Eb9q/3iGTjQnqeL+ulK/X3JWGdEHK+g6aI8m3cj7Br1bsnJyQQDP+Ig2L13vhFIMGddfoQzgR Mly1VTIV W7wTpuil3lnmH+Hf2e9Kd9wXot7T/PGFI3eO1YtTGgXATCfkdxrCTGHTYroQGqxRwGPXicErK+UlWHe7mUeNrnXu5chDXdl7Y0SBbsTujkSapSmGpAxHGhlqCeCYq3M+BPFLOYbft8S1IdRS79HoN5u8LTsSS0vtmGaU86C2Lgc+5X7guYJXsarQpnD9vWtUCdOFfdqtx0GmikL+08DnitoTJ2XOdO7NIARlq0rxlAXUWo2UIqsDWJK7f+IAUcGInUjsXMh+jx3nDbJdKGkEih89tKDTFViHT76gywkdje/oFVmEip+6gFvSUojwLtGxwWcRUS/rG2y2JiB3R8ULH6/EuKJfajgNUq9npv6P764z2s/oS6dKKpQvks5QQO0rpU4YObbLkOKc0aXt3tQZgC+oRxVAwcamgcYuhbqjUm71I6maeKb2pqxJ2M5W5D1nAfvi9ahTZy/Sy/N1XdLbaJ3jMvFGI26NpTXBYYT8QblS9HpPFEHVZcan2Ew== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Jan 06, 2025 at 10:03:57PM +0900, Koichiro Den wrote: > On Mon, Jan 06, 2025 at 10:52:37AM +0000, Lorenzo Stoakes wrote: > > +cc tglx, peterz for insight on CPU hot plug > > > > On Sat, Jan 04, 2025 at 01:00:17PM +0900, Koichiro Den wrote: > > > On Fri, Jan 03, 2025 at 11:33:19PM +0000, Lorenzo Stoakes wrote: > > > > On Sat, Dec 21, 2024 at 12:33:20PM +0900, Koichiro Den wrote: > > > > > 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: > > > > > > > > > > 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 > > > > > > > > > > > > > > > So, for mm/vmstat:online, disable vmstat_work reliably on teardown and > > > > > symmetrically enable it on startup. > > > > > > > > > > Signed-off-by: Koichiro Den > > > > > > > > Hi, > > > > > > > > I observed a warning in my qemu and real hardware, which I bisected to this commit: > > > > > > > > [ 0.087733] ------------[ cut here ]------------ > > > > [ 0.087733] workqueue: work disable count underflowed > > > > [ 0.087733] WARNING: CPU: 1 PID: 21 at kernel/workqueue.c:4313 enable_work+0xb5/0xc0 > > > > > > > > This is: > > > > > > > > static void work_offqd_enable(struct work_offq_data *offqd) > > > > { > > > > if (likely(offqd->disable > 0)) > > > > offqd->disable--; > > > > else > > > > WARN_ONCE(true, "workqueue: work disable count underflowed\n"); <-- this line > > > > } > > > > > > > > So (based on this code) presumably an enable is only required if previously > > > > disabled, and this code is being called on startup unconditionally without > > > > the work having been disabled previously? I'm not hugely familiar with > > > > delayed workqueue implementation details. > > > > > > > > [ 0.087733] Modules linked in: > > > > [ 0.087733] CPU: 1 UID: 0 PID: 21 Comm: cpuhp/1 Not tainted 6.13.0-rc4+ #58 > > > > [ 0.087733] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014 > > > > [ 0.087733] RIP: 0010:enable_work+0xb5/0xc0 > > > > [ 0.087733] Code: 6f b8 01 00 74 0f 31 d2 be 01 00 00 00 eb b5 90 0f 0b 90 eb ca c6 05 60 6f b8 01 01 90 48 c7 c7 b0 a9 6e 82 e8 4c a4 fd ff 90 <0f> 0b 90 90 eb d6 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 > > > > [ 0.087733] RSP: 0018:ffffc900000cbe30 EFLAGS: 00010092 > > > > [ 0.087733] RAX: 0000000000000029 RBX: ffff888263ca9d60 RCX: 0000000000000000 > > > > [ 0.087733] RDX: 0000000000000001 RSI: ffffc900000cbce8 RDI: 0000000000000001 > > > > [ 0.087733] RBP: ffffc900000cbe30 R08: 00000000ffffdfff R09: ffffffff82b12f08 > > > > [ 0.087733] R10: 0000000000000003 R11: 0000000000000002 R12: 00000000000000c4 > > > > [ 0.087733] R13: ffffffff81278d90 R14: 0000000000000000 R15: ffff888263c9c648 > > > > [ 0.087733] FS: 0000000000000000(0000) GS:ffff888263c80000(0000) knlGS:0000000000000000 > > > > [ 0.087733] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > [ 0.087733] CR2: 0000000000000000 CR3: 0000000002a2e000 CR4: 0000000000750ef0 > > > > [ 0.087733] PKRU: 55555554 > > > > [ 0.087733] Call Trace: > > > > [ 0.087733] > > > > [ 0.087733] ? enable_work+0xb5/0xc0 > > > > [ 0.087733] ? __warn.cold+0x93/0xf2 > > > > [ 0.087733] ? enable_work+0xb5/0xc0 > > > > [ 0.087733] ? report_bug+0xff/0x140 > > > > [ 0.087733] ? handle_bug+0x54/0x90 > > > > [ 0.087733] ? exc_invalid_op+0x17/0x70 > > > > [ 0.087733] ? asm_exc_invalid_op+0x1a/0x20 > > > > [ 0.087733] ? __pfx_vmstat_cpu_online+0x10/0x10 > > > > [ 0.087733] ? enable_work+0xb5/0xc0 > > > > [ 0.087733] vmstat_cpu_online+0x5c/0x70 > > > > [ 0.087733] cpuhp_invoke_callback+0x133/0x440 > > > > [ 0.087733] cpuhp_thread_fun+0x95/0x150 > > > > [ 0.087733] smpboot_thread_fn+0xd5/0x1d0 > > > > [ 0.087734] ? __pfx_smpboot_thread_fn+0x10/0x10 > > > > [ 0.087735] kthread+0xc8/0xf0 > > > > [ 0.087737] ? __pfx_kthread+0x10/0x10 > > > > [ 0.087738] ret_from_fork+0x2c/0x50 > > > > [ 0.087739] ? __pfx_kthread+0x10/0x10 > > > > [ 0.087740] ret_from_fork_asm+0x1a/0x30 > > > > [ 0.087742] > > > > [ 0.087742] ---[ end trace 0000000000000000 ]--- > > > > > > > > > > > > > --- > > > > > v1: https://lore.kernel.org/all/20241220134234.3809621-1-koichiro.den@canonical.com/ > > > > > --- > > > > > mm/vmstat.c | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/mm/vmstat.c b/mm/vmstat.c > > > > > index 4d016314a56c..0889b75cef14 100644 > > > > > --- a/mm/vmstat.c > > > > > +++ b/mm/vmstat.c > > > > > @@ -2148,13 +2148,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)); > > > > > > > > Probably needs to be 'if disabled' here, as this is invoked on normal > > > > startup when the work won't have been disabled? > > > > > > > > Had a brief look at code and couldn't see how that could be done > > > > however... and one would need to be careful about races... Tricky! > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > Let me know if you need any more details, .config etc. > > > > > > > > I noticed this warning on a real box too (in both cases running akpm's > > > > mm-unstable branch), FWIW. > > > > > > Thank you for the report. I was able to reproduce the warning and now > > > wonder how I missed it.. My oversight, apologies. > > > > > > In my current view, the simplest solution would be to make sure a local > > > vmstat_work is disabled until vmstat_cpu_online() runs for the cpu, even > > > during boot-up. The following patch suppresses the warning: > > > > > > diff --git a/mm/vmstat.c b/mm/vmstat.c > > > index 0889b75cef14..19ceed5d34bf 100644 > > > --- a/mm/vmstat.c > > > +++ b/mm/vmstat.c > > > @@ -2122,10 +2122,14 @@ 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); > > > > > > + /* will be enabled on vmstat_cpu_online */ > > > + disable_delayed_work_sync(&per_cpu(vmstat_work, cpu)); > > > + } > > > + > > > schedule_delayed_work(&shepherd, > > > round_jiffies_relative(sysctl_stat_interval)); > > > } > > > > > > If you think of a better solution later, please let me know. Otherwise, > > > I'll submit a follow-up fix patch with the above diff. > > > > Thanks, this resolves the problem, but are we sure that _all_ CPUs will > > definitely call vmstat_cpu_online()? > > > > I did a bit of printk output and it seems like this _didn't_ online CPU 0, > > presumably the boot CPU which calls this function in the first instance? > > > > I also see that init_mm_internals() invokes cpuhp_setup_state_nocalls() > > explicitly which does _not_ call the callback, though even if it did this > > would be too early as it calls start_shepherd_timer() _after_ this anyway. > > > > So yeah, unless I'm missing something, I think this patch is broken. > > You're absolutely right, thanks a lot. > I also appreciate you testing it, thanks everyone. > > > > > I have added Thomas and Peter to give some insight on the CPU hotplug side. > > > > It feels like the patch really needs an 'enable if not already enabled' > > call in vmstat_cpu_online(). > > Right. While not fully polished yet, I've tested the following diff: > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 0889b75cef14..f967aa22392f 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -1937,7 +1937,11 @@ static const struct seq_operations vmstat_op = { > #endif /* CONFIG_PROC_FS */ > > #ifdef CONFIG_SMP > -static DEFINE_PER_CPU(struct delayed_work, vmstat_work); > +struct vmstat_work { > + struct delayed_work dwork; > + bool enabled; > +}; > +static DEFINE_PER_CPU(struct vmstat_work, vmstat_work); > int sysctl_stat_interval __read_mostly = HZ; > static int vmstat_late_init_done; > > @@ -2015,7 +2019,7 @@ static void vmstat_update(struct work_struct *w) > * update worker thread. > */ > queue_delayed_work_on(smp_processor_id(), mm_percpu_wq, > - this_cpu_ptr(&vmstat_work), > + &this_cpu_ptr(&vmstat_work)->dwork, > round_jiffies_relative(sysctl_stat_interval)); > } > } > @@ -2059,7 +2063,7 @@ void quiet_vmstat(void) > if (system_state != SYSTEM_RUNNING) > return; > > - if (!delayed_work_pending(this_cpu_ptr(&vmstat_work))) > + if (!delayed_work_pending(&this_cpu_ptr(&vmstat_work)->dwork)) > return; > > if (!need_update(smp_processor_id())) > @@ -2091,7 +2095,7 @@ static void vmstat_shepherd(struct work_struct *w) > cpus_read_lock(); > /* Check processors whose vmstat worker threads have been disabled */ > for_each_online_cpu(cpu) { > - struct delayed_work *dw = &per_cpu(vmstat_work, cpu); > + struct delayed_work *dw = &per_cpu(vmstat_work, cpu).dwork; > > /* > * In kernel users of vmstat counters either require the precise value and > @@ -2120,11 +2124,14 @@ static void vmstat_shepherd(struct work_struct *w) > > static void __init start_shepherd_timer(void) > { > + struct vmstat_work *vmstat; > int cpu; > > - for_each_possible_cpu(cpu) > - INIT_DEFERRABLE_WORK(per_cpu_ptr(&vmstat_work, cpu), > - vmstat_update); > + for_each_possible_cpu(cpu) { > + vmstat = per_cpu_ptr(&vmstat_work, cpu); > + INIT_DEFERRABLE_WORK(&vmstat->dwork, vmstat_update); > + vmstat->enabled = true; > + } > > schedule_delayed_work(&shepherd, > round_jiffies_relative(sysctl_stat_interval)); > @@ -2142,20 +2149,28 @@ static void __init init_cpu_node_state(void) > > static int vmstat_cpu_online(unsigned int cpu) > { > + struct vmstat_work *vmstat = per_cpu_ptr(&vmstat_work, cpu); > + > if (vmstat_late_init_done) > refresh_zone_stat_thresholds(); > > 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)); > + > + if (!vmstat->enabled) { > + enable_delayed_work(&vmstat->dwork); > + vmstat->enabled = true; > + } > > return 0; > } > > static int vmstat_cpu_down_prep(unsigned int cpu) > { > - disable_delayed_work_sync(&per_cpu(vmstat_work, cpu)); > + struct vmstat_work *vmstat = per_cpu_ptr(&vmstat_work, cpu); > + > + disable_delayed_work_sync(&vmstat->dwork); The line `vmstat->enabled = false;` was missing here. With that fixed, I've conducted the following tests: * Boot-up check: confirmed that vmstat_update() runs on all cpus after boot-up. * Cpuhp transitions: verified resilience during random CPU hotplug transitions using the steps below: 1. enabled dynamic ftrace on vmstat_update() 2. ran the following simple silly script for a while #!/bin/bash while :; do now=$(cat /sys/devices/system/cpu/cpu7/hotplug/state) next=$(shuf --random-source=/dev/urandom -i 0-238 -n 1) echo "${now} -> ${next}" echo $next > /sys/devices/system/cpu/cpu7/hotplug/target 2>/dev/null sleep 0.001 done 3. after stopping the script, ensured that the cpu was fully online again by writing the largest value to the 'state' file. 4. verified that vmstat_update() was working on the cpu. Let me know if you have any suggestions or concerns. Thanks. -Koichiro Den > return 0; > } > > Lorenzo, and Thomas and Peter if you're available, I'd greatly appreciate > any thoughts or feedback on this. > > Thanks. > > -Koichiro Den > > > > > > > > > > Thanks. > > > > > > -Koichiro