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 C3254E77197 for ; Wed, 8 Jan 2025 02:31:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5B00E6B0088; Tue, 7 Jan 2025 21:31:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 55F666B0089; Tue, 7 Jan 2025 21:31:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4007C6B008A; Tue, 7 Jan 2025 21:31:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 210696B0088 for ; Tue, 7 Jan 2025 21:31:57 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id C9F16140E97 for ; Wed, 8 Jan 2025 02:31:56 +0000 (UTC) X-FDA: 82982709432.03.6460A46 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf18.hostedemail.com (Postfix) with ESMTP id D8E421C000B for ; Wed, 8 Jan 2025 02:31:54 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=T43cD6XP; spf=pass (imf18.hostedemail.com: domain of chenhuacai@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=chenhuacai@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736303514; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=LRF1WhcOJdJs3UZFUELwGoYjVfqQdr2bCki6MA6+yNE=; b=xvpuE6VGGJaUpW8xSs/5wDD4dsB7/GgzoJWHMHVqSANdXsS0saIC8TyylpRu9Zbb3Jw5LW XadTdnOVumTHVK3r7QqDGdDM8AmIImYjp1nPJk5TR0vsAOuCisEXf84ZF5CIGGt0JdJ3CC kPyN4gfq51zyCbRJ8/I5RDZknvMAX3Y= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=T43cD6XP; spf=pass (imf18.hostedemail.com: domain of chenhuacai@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=chenhuacai@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736303514; a=rsa-sha256; cv=none; b=4pYAStWdDqlOC6bwms6O3Dq5H5Kwk/tvaAZ5+g6JGI9PTXgsLNDBWvBoPlhaBFezWQ5EgC I19wI86/dgnuAruei4ATtuNg8dIfLYMUkrc86ffhiFfoXSmuxFTONXqPPnTIEHPS+Hb/oB RoY7GWMrbkQvsq6sXv6U+jsRGYOPU74= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 4F64EA41246 for ; Wed, 8 Jan 2025 02:30:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A6BE8C4AF09 for ; Wed, 8 Jan 2025 02:31:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736303513; bh=9OHE9yp9OQHBK0rAZ0ZJ1WsQl0zZJ2mep8yOky/plAA=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=T43cD6XPkSt9Zq+ryal/uKegnNYsOB0o864nKsmNWZlKASv3Q0YjEfEyAKAhxnwjf z2ZYqo9HVEAGwwMVE5xZUS1qdng3yhsu4hdx3ZJcRXI/gPEzYdgTorjNgTNnm6kxrn DLoUYGC9mGlAJJp0yWsadmN63mNJ9jo896KfZTXTxDnwwPXlXt2sngEUiFmurPmEE8 NKhd1XJg0JK8oEaTvP0SRr1MxY8KqjZfiNNEWFv7zcU9TxJgvSkv5VoOKW6WsRX84E XoaqFOtQfMYwgUgViOPgPWfKgQxMEcuGpDwpJ66kQ6t5vaqt5nfo6MxxzsBTUV/rgs KAtt2bFexOxAw== Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-aaeec07b705so1896471066b.2 for ; Tue, 07 Jan 2025 18:31:53 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCVhoGOLcCxWABwj/J2CfI0JzIxy/R0r34XTLjlMtQUgXO9ChK/jSD38pXBwpDTl5fgO8QDRZWot6w==@kvack.org X-Gm-Message-State: AOJu0YzOvI2xbQdYAlsHIBF/G0jTq9f64upLKG0Y9ojeu4lkq5j7UKKR G3r/tkjwXdmnEhdS7bPy127HX+H7WgcW53turVDGfR2jPqhtpgNPTNyY2ZJBAZ9jwCjOddZEJ9v Zp2RygiJL0aPYVo0O02NUNA+GWkc= X-Google-Smtp-Source: AGHT+IGKKQosEI/m+qikGaDIi+Ex3AGnjLTmy+CzoJgYHB0zt/miw1YN4v3MO6MI/6uSp8r91rSAJ0qr2Oo93mPy1co= X-Received: by 2002:a17:906:dc8f:b0:aa6:a21b:2a9 with SMTP id a640c23a62f3a-ab2abde5fc9mr83382666b.57.1736303512212; Tue, 07 Jan 2025 18:31:52 -0800 (PST) MIME-Version: 1.0 References: <7gd4skn6l4i3liu6cvzmgqlrwxc3rukrnau7lnpyhmfyjuvqwl@gdwgybedp5gs> <20250107011848.689556-1-chenhuacai@loongson.cn> <3a5ad843-6d00-4e8c-9f77-f53282cef4b3@lucifer.local> <56flobk2ooduyk5465d47tgaz6jkeupbd2qjk7lt35ust2etnh@5l4os3td5imr> In-Reply-To: <56flobk2ooduyk5465d47tgaz6jkeupbd2qjk7lt35ust2etnh@5l4os3td5imr> From: Huacai Chen Date: Wed, 8 Jan 2025 10:31:40 +0800 X-Gmail-Original-Message-ID: X-Gm-Features: AbW1kva5Mc2mCZlAMhL2d03w-9QppdB2xQDY44ooziT_7si6ep18PCMSvL3Vgxs Message-ID: Subject: Re: [PATCH] Simple fix To: Koichiro Den Cc: Lorenzo Stoakes , Huacai Chen , Andrew Morton , linux-mm@kvack.org, Sebastian Andrzej Siewior , Mark Rutland , Charalampos Mitrodimas Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D8E421C000B X-Stat-Signature: bbjq9rp3s18efit79nefgbpscy3ogctm X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1736303514-312454 X-HE-Meta: U2FsdGVkX1+JH+3lRqqknbp8sAqMjtCqSc5zfx2TCIzP8MUXFe48e9o8he6xt07DHe7T+pPv+V96tAMfH/Xjzog5MN+FaTNOgfFULiI+ej2j8ietaIIDzmTaNblcgHwdXDJVBwtklrZJmwa6NJyl5r5pMnw5NVh1SnFDHBKxyW3qkRTkwI7vIbXpZEZLCtIUvWosLIG8ZTPt3sH79tez+27OhzMkygYkNa4pCbNp4RDopsjRSpk/KaVwcP8E90Z+biGSwIM9Ak47ImPzGN0XLY2o/De9JEiRWpcokkOHWBsCb30VW8pnqyTBGXRHTGKzKLHPw2f61VNV5mTd76LuRlsJ/MxJHwck9oQkmsH/mNJs77wSQMeg2XA+RbZT1zKomBUs6wLlqYGG4RJhqPurFt/COVm+7Lt47IM4UmJn6C5AHZ4ZMRRIyMMEgcKfpyTAO0cVDEC/2RKCmaoiZ7GXg2IzcAlCwH0uFDxpkWAK05TBMdACNVth+P+ZAU/8DYnPAY9qnVzbMtBTmKQk2NW60NJa7JgVlGqmrtSUq8C4ViS2vGeuE3Tl/Cyjs46X+oN3X7D3QGknnEIGarXrUJr+aPFpCLA/R3pWIE7+AKF0PfJ1LloCYlOHWlqzZ9tveEg5CnsX58oD5Q4Vg36W6zvjIji687pN7vVY7Q6Ur8IBTylKOe/RXHOyxk9shu4joM8h6e3aFEVVXV8WpCeOXyOOB7709swU5fDnpDWg2bvAyiImiiunTDR64cO9Pq/tsOKRRE/WltbAB67XKkYmVNgIGONad5jEuC42tW+bOodRvQVsWk9s6QxB0EP5BJT8SD95+Nvzk24OxYZLdtftryiLGoXgKTIViy9+eVg3lsvJZabbRLh3wDRJa7B1d4JIAdJP9Ltag+eMxXqrmSxDqxO1+NXo+lMu2r05Is3nE+yILWH4xIDw1yIxlusg6VZPqa9V1HNr9VmkfrK+VNJzbbF njOYfurs QQT4Nj4RzKcRxELcDunQFSvXyJeAg759BeciEATh/qElH/qJWxYrLyD1buOvZkbbuDs5cmB8ZQyapqA91lMsbpq2y0TQu6H6TFQ6V/FmUHLBRC5svckXjvBcqsH+HH2/hMNpCB2O4JiLT8G3jjLZlnxqkdJlTnQcCxZUVAyLkNI518aF6tgBbyf+16Qg663OLWI472WkTowu9pHqg3mTOLEaCR9EiP+yaKPCREzjTwEugFvHkM/kRvosGr8u4g1loOiHFuBxj7Gn3F2GRttBr4V8SH8HWRbDgMwgrPEt9Du+RXeANLv133SaWpU1bkyVLkJvfHcFD2Qc+UVNLdZu/CKMQWS2bI73u/X+X0H55156FuM4GebkZv9EwkrK+dMpDiB1iijYawv0pAxU= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000101, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi, Koichiro, On Wed, Jan 8, 2025 at 10:22=E2=80=AFAM Koichiro Den wrote: > > On Tue, Jan 07, 2025 at 11:00:10AM +0000, Lorenzo Stoakes wrote: > > On Tue, Jan 07, 2025 at 06:29:03PM +0800, Huacai Chen wrote: > > > Hi, Lorenzo, > > > > > > On Tue, Jan 7, 2025 at 4:48=E2=80=AFPM Lorenzo Stoakes > > > wrote: > > > > > > > > On Tue, Jan 07, 2025 at 09:18:48AM +0800, Huacai Chen wrote: > > > > > Hi, all, I like simple fixes, so is this acceptable (based on an = early > > > > > version from Koichiro Den)? > > > > > > > > No not at all. This is bizarre - in the mail you are replying to Ko= ichiro > > > > agrees with me that the approach of his code that you've sent here = (I don't > > > > know why) is fundamentally broken and suggest another. > > > > > > > > I am at a loss as to why you've sent this? Perhaps a miscommunicati= on > > > > somewhere? :) > > > > > > > > In any case, please don't send '[PATCH] xxx' mails that aren't inte= nded to > > > > be patches, a better form of this would be to say 'oh can we just d= o...' > > > > then to put this code in the mail underneath, without any '[PATCH]'= prefix. > > > I wasn't in the CC list, and I also found the bug yesterday, so I can > > > only reply to this email with "git send-email --in-reply-to". This is > > > the reason why my reply looks so stranne. > > > > > > > > > > > But please do review the discussion - the below is insufficient as = simple > > > > as it is (sadly) because the boot CPU's delayed work will never be > > > > executed. > > > Koichiro's simple fix causes the boot CPU's delayed work to never be > > > executed, this is obvious, and of course I have read the early > > > discussion. And so I improve it, with a "cpu_online()" checking, then > > > the boot CPU is unaffected. > > > > With respect Haucai, this is not how you engage in kernel discussion. Y= ou > > could simply have replied to the mail or given more information, you no= w > > have two people telling you this, please take it on board. > > > > And it caused me to miss your actually quite valuable suggestion so thi= s is > > beneficial for all! :) > > > > ANYWAY, moving on to the technical side: > > > > > > > > Huacai > > > > > > > > > > > I will take a look at Koichiro's new approach as soon as I am able. > > > > > > > > Cheers! > > > > > > > > > --- > > > > > mm/vmstat.c | 7 ++++++- > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/mm/vmstat.c b/mm/vmstat.c > > > > > index 0889b75cef14..1badc24a21ff 100644 > > > > > --- a/mm/vmstat.c > > > > > +++ b/mm/vmstat.c > > > > > @@ -2122,10 +2122,15 @@ static void __init start_shepherd_timer(v= oid) > > > > > { > > > > > 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() */ > > > > > + if (!cpu_online(cpu)) > > > > This might actually be workable as something simpler, on assumption the= re > > Sorry about late response. And thank you very much, Huacai. Your suggesti= on > looks great. Much simpler and intuitive. > > > can be no race here (I don't think so right?). > > IIUC, there is no race since smp_init() always runs before init_mm_intern= als(). > At this stage (i.e. start_shepherd_timer()), for cpuhp scenario, the seco= ndary > CPUs haven=E2=80=99t even been brought up yet. So the simple 'cpu_online'= check looks > simple and the best. > > > > > Koichiro - could you check this and see whether it resolves the issue a= nd > > whether you feel this is sensible? > > I tested it and it seems to work well both on booting and cpuhp events. > > (By the way, in my previous email comment [1], I forgot to mention that I= also > applied other unmerged patches, [2] and [3], just to be able to run such = random > transisioning test. This time here, I just tested by switching CPUHP_ONLI= NE <-> > CPUHP_OFFLINE only.) > > [1]: https://lore.kernel.org/linux-mm/7ed97096-859e-46d0-8f27-16a2298a891= 4@lucifer.local/T/#m11d983715699d3cea295b8618aba7b6ccec4db55 > [2]: https://lore.kernel.org/all/20241220134421.3809834-1-koichiro.den@ca= nonical.com/ > [3]: https://lore.kernel.org/all/20241220141538.4018232-1-koichiro.den@ca= nonical.com/ > > > > > Might be worth expanding comment to say that we disable on offline, ena= ble > > on online and we're just providing symmetry here. > > Sounds good. Like this? > > - 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_wor= k, cpu)); > + } > > > Before submitting a revised patch, I'd like to confirm: > Huacai, would you be comfortable with me adding your Signed-off-by to the > commit, or would you prefer a Suggested-by tag instead? Since the original V2 patch has been reverted in mainline, so I think you will send a V3 which integrate my suggestion, right? Both Signed-off-by and Suggested-by is OK for me. Huacai > > Thanks. > > -Koichiro Den > > > > > > > > + disable_delayed_work_sync(&per_cpu(vmstat_w= ork, cpu)); > > > > > + } > > > > > + > > > > > schedule_delayed_work(&shepherd, > > > > > round_jiffies_relative(sysctl_stat_interval)); > > > > > } > > > > > -- > > > > > 2.43.5 > > > > > > > > > > > > Cheers, Lorenzo