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 34837E77188 for ; Wed, 8 Jan 2025 03:58:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B6D0C6B0093; Tue, 7 Jan 2025 22:58:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AF4D56B0095; Tue, 7 Jan 2025 22:58:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96DCD6B0096; Tue, 7 Jan 2025 22:58:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 70E836B0093 for ; Tue, 7 Jan 2025 22:58:10 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 3403E12039E for ; Wed, 8 Jan 2025 03:58:10 +0000 (UTC) X-FDA: 82982926740.20.5693A6A Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf09.hostedemail.com (Postfix) with ESMTP id 47CA814000C for ; Wed, 8 Jan 2025 03:58:08 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=bHbjvmNK; spf=pass (imf09.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=1736308688; 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=wFoIswyAnWtMn5+ALwZd+78LV/oUdLLvBSEGvOOIPuc=; b=es9sS5P2Swet3huSuXsuRFtD/JeotZmKHdumUPykY9pb3P4d+FmWVd7ArmPo1jPH7P+axJ M1nXIiXDEoeyv7I6Fg5RrcsoBrRtSlrC4frfaaUZz9VmSqVdIYDj04L/HOyLk3TPtwyL5O Xt10ZGAKggObLj0w/Fs5qlgcfvN8Gm0= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=bHbjvmNK; spf=pass (imf09.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=1736308688; a=rsa-sha256; cv=none; b=dXp8g2A8hnrFriLD/xd382AsVAOFeIxlS94MSrEnTJbZYrxgkdyrepLQH2/HwJq3JlAMNr W2sLtHdP9+DcAiPtdExvd+n/90ZTz/qhtpla60iswiU5cP2U9WmjZaVauqwRQq/vT2aVsc zTGGzPVSk3hUNaooaM9zFkjccc/o3q0= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id AE3DBA40A69 for ; Wed, 8 Jan 2025 03:56:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B2043C4CEE5 for ; Wed, 8 Jan 2025 03:58:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736308686; bh=QnUEF+LVeIxjfASy/YhI2mUhhu8uUDxx9s8k11xgNGo=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=bHbjvmNKuIrNcfdIUfOyjqTngfeRlsjIVZ8+z0tNttvLuMsI8AzWIad4IXMBIpU6i hnDWLxrAd7SejZZ1gT9ENskG1elnrNLOj1GkYY/POH8b/t13Fkj3oJsLy/6Jm7MC0J 0PkW9FLH2VNtLQ3IhueZAuLUPGBxdI+Qp4ZHXUIvOyF1De11ZphRHRM/+Gk+ZSDzsu S6uitJkbIn56oJQvPz1yqz3pI3AdZHrYy9Uer4wdVXrSYL980nFi3YCRXaGcMka37d /SgIne/lAkK+G6RCuQ1pqgiJ5RQ8L2FlojU7pO6fQQfsZAeyWg2J8X/vH3x+1zShix 936b9EB6zyROg== Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-ab2aea81cd8so46497266b.2 for ; Tue, 07 Jan 2025 19:58:06 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCWYGCyrrRSFI/S/PbCoSXSLHA4dUqG60iOk1qHXrz4JNxlsPO08gSCRNl0E4A5oH7uQ6YOBquDvlg==@kvack.org X-Gm-Message-State: AOJu0Yz8C+KfoDJKaXJfMmz+l/rCID2OAZ3xY1y1Lbs0dKJRMV0qayTU txOItxslHUy9O3+iZXB0ilzezqBz/m1oxKz3wmZXMQ5nq9rJ1VUBi4eWh/Se95RF9saup20QarA a0GM4a5jnNRc0nLuQMbB3bdcgRwI= X-Google-Smtp-Source: AGHT+IG8ZupUeY/qkGU5p14pZB6fXPAiVMp2EvoJSGfMbwp1P7iRO6GJwy3TbskLebRykzUNz0C8JWuRq6NqjBMyTUQ= X-Received: by 2002:a17:907:d24:b0:aa6:7ab9:e24d with SMTP id a640c23a62f3a-ab2abc94efemr85448466b.57.1736308685137; Tue, 07 Jan 2025 19:58:05 -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: From: Huacai Chen Date: Wed, 8 Jan 2025 11:57:53 +0800 X-Gmail-Original-Message-ID: X-Gm-Features: AbW1kvbx8kzLjqtBwj79NhFVjJw9GdElGdvNggmFl8nl2D7bq95byv2VsbEqAIo 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-Server: rspam05 X-Stat-Signature: kzrkcscdmfk7t9n4e8t7bzax49w188e1 X-Rspamd-Queue-Id: 47CA814000C X-Rspam-User: X-HE-Tag: 1736308688-527610 X-HE-Meta: U2FsdGVkX1/t6dw+UBAAuk1DWnbadnpmorC+/RM8JsqYwpbnVs4bPNshe8HzF6eLn/sV0Z0grJsUn056E1MMZdfrwv/C03m6wh3594iC3BpEQjC/0/WN2mLiZrzgOgUBLOeRCoQLyo3t5UsGrjTWzJGaualQaHD5hmw5vxyVSiCmVb3lW7lu8dWE83JZhORa2qDPGcxFGka1cqrdCGBU/yEHKs0ClOhM1mOUEy0+eQJZYeun4CRcfjZkfaCMjqoDhRTkEsV0FoT5+6rC0CggbYv1ufTyGQ4VC72mlP4h0juE3GDkUu/h2N2i2JeEVKruLB57aGBKQsQYzpE1xFHG2Pui5oGDAXophIfROx6NU1LH3N1FtuwtrLix9JkUdcHI3KdRmERtzswLGV/jFrTHEnflPEi3XvWsQrznPWkPKyksfD9MtHvI4n/mos/0occ9rthBjTlIcpzx/57W1l1Ci+Fc7jbBZ65HZPtM5XDkyYlQPlP/sRbtd66SdPzBwwDUbazO7wgfzFlKqMDgWf6Bj0k2ELwvxJDL+Zd1+IbHyg7xdI/QtTZvLP6TKblTJg7XfX+rks57qKOM6bgUBLrrSgYO6lK4g9LXShLZBIK2wrm5UjjTXTEJoYWYGf2by9GLr0i+ITaO6PMa25zxOPUILy/39yvlS+P6NgiYhoQcBoooA4vYadTvFsf5xx2AiCfjsaSa6i8OztRv6/0sqnAq68c+gXloAyErZgAd+nvgApv7SLkvem4ERqAnIqVQ3RxvISPPugdIoX+YXJHeRvFB3RWcYRB+0FeMwnoRDthnKmq9OXlaZ7t6D6rDrDHSMaY6W3q75OyC+W3NL3tD4IWuKZNM6T085wWi69Jck+gT+lpzbKrw1ePYSlBeaFFxxMMYxVcnjdo2L0qfUsR8t0o2rYgng0qzsfAkSVa4KMe2Xrj8002syOBJ1yrBtRmhaZswha/wTWHutFNp3jYuZwo Z//7LG53 TbojDCuFJbz+skhp5YA4MwPONA3Qy+O8CNYiEQ0S69khkBb7XVCO4KB1SxBJ1rMMoo1U8IsJZBae1+eYQFXLOgnC7ZqoqnqzLmPguIBlk9gKeH534akPmC5jxYwColRp3NXGAEo4+sGR1haf+VG/2dUX6d9rY5vaPOmgcu6/zC/h9IYuONS7k3Qh+YN8QcmvBWDpJqQoB8WWQsUWKRU4OCUDxG9gfM9D2Te57gn1ifqzjqfE5NDLp02C1SfBWiwKv4SaMkFixtW7GtQeHrgntuHfujVaTXv8jlK76zg4MMd+H2vQnOsIRMC6jOHpYnfi9cU5afWLMEuiup6S6AThF9CyMOHBk2MH0Cl7vCN3yqAE+7zu8VYCHCPmMlzodGVA8aZB3H8IxWb8Ddww= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000023, 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 Wed, Jan 8, 2025 at 11:41=E2=80=AFAM Koichiro Den wrote: > > On Wed, Jan 08, 2025 at 10:31:40AM +0800, Huacai Chen wrote: > > 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 t= o Koichiro > > > > > > agrees with me that the approach of his code that you've sent h= ere (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 miscommuni= cation > > > > > > somewhere? :) > > > > > > > > > > > > In any case, please don't send '[PATCH] xxx' mails that aren't = intended to > > > > > > be patches, a better form of this would be to say 'oh can we ju= st do...' > > > > > > then to put this code in the mail underneath, without any '[PAT= CH]' 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". Thi= s 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 discussio= n. You > > > > could simply have replied to the mail or given more information, yo= u now > > > > have two people telling you this, please take it on board. > > > > > > > > And it caused me to miss your actually quite valuable suggestion so= this 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 a= ble. > > > > > > > > > > > > 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_tim= er(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() */ > > > > > > > + if (!cpu_online(cpu)) > > > > > > > > This might actually be workable as something simpler, on assumption= there > > > > > > Sorry about late response. And thank you very much, Huacai. Your sugg= estion > > > 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_in= ternals(). > > > At this stage (i.e. start_shepherd_timer()), for cpuhp scenario, the = secondary > > > CPUs haven=E2=80=99t even been brought up yet. So the simple 'cpu_onl= ine' check looks > > > simple and the best. > > > > > > > > > > > Koichiro - could you check this and see whether it resolves the iss= ue and > > > > whether you feel this is sensible? > > > > > > I tested it and it seems to work well both on booting and cpuhp event= s. > > > > > > (By the way, in my previous email comment [1], I forgot to mention th= at I also > > > applied other unmerged patches, [2] and [3], just to be able to run s= uch random > > > transisioning test. This time here, I just tested by switching CPUHP_= ONLINE <-> > > > CPUHP_OFFLINE only.) > > > > > > [1]: https://lore.kernel.org/linux-mm/7ed97096-859e-46d0-8f27-16a2298= a8914@lucifer.local/T/#m11d983715699d3cea295b8618aba7b6ccec4db55 > > > [2]: https://lore.kernel.org/all/20241220134421.3809834-1-koichiro.de= n@canonical.com/ > > > [3]: https://lore.kernel.org/all/20241220141538.4018232-1-koichiro.de= n@canonical.com/ > > > > > > > > > > > Might be worth expanding comment to say that we disable on offline,= enable > > > > 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, cp= u), > > > vmstat_update); > > > > > > + /* > > > + * For secondary CPUs during CPU hotplug scenario= s, > > > + * vmstat_cpu_online() will enable the work. > > > + * mm/vmstat:online enables and disables vmstat_w= ork > > > + * symmetrically during CPU hotplug events. > > > + */ > > > + if (!cpu_online(cpu)) > > > + disable_delayed_work_sync(&per_cpu(vmstat= _work, 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? > > I was considering sending a separate patch with a title distinct from the > original, while referencing both the original (v2) patch and the revert > commit. However, I'm fine with either approach. Don't separate, just sending a single V3 patch is OK for me. Huacai > > If there=E2=80=99s any documentation that mentions a recommended method i= n this > kind of situation, please let me know. Personally, I=E2=80=99m not a fan = of > multiple commits with identical titles appearing in a branch. > > > > > Both Signed-off-by and Suggested-by is OK for me. > > Alright, thanks! > > -Koichiro Den > > > > > Huacai > > > > > > > > Thanks. > > > > > > -Koichiro Den > > > > > > > > > > > > > > + disable_delayed_work_sync(&per_cpu(vmst= at_work, cpu)); > > > > > > > + } > > > > > > > + > > > > > > > schedule_delayed_work(&shepherd, > > > > > > > round_jiffies_relative(sysctl_stat_interval)); > > > > > > > } > > > > > > > -- > > > > > > > 2.43.5 > > > > > > > > > > > > > > > > > > > > Cheers, Lorenzo