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 2FA92E77197 for ; Wed, 8 Jan 2025 03:41:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B397C6B0082; Tue, 7 Jan 2025 22:41:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AE88B6B0089; Tue, 7 Jan 2025 22:41:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9B0C26B008A; Tue, 7 Jan 2025 22:41:36 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 802236B0082 for ; Tue, 7 Jan 2025 22:41:36 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 029EAA0DF8 for ; Wed, 8 Jan 2025 03:41:35 +0000 (UTC) X-FDA: 82982884992.04.1849736 Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) by imf12.hostedemail.com (Postfix) with ESMTP id AF1B440007 for ; Wed, 8 Jan 2025 03:41:33 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=i0IOkhkF; dmarc=pass (policy=none) header.from=canonical.com; spf=pass (imf12.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.123 as permitted sender) smtp.mailfrom=koichiro.den@canonical.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736307694; a=rsa-sha256; cv=none; b=H9C0kaGzSmNP2ADVF/b4gx3p+jjp3t03fTxZMu9wtWqU+7oNeFyD9KYR3jSW8/RvBVoSJ+ 69EL9LVZjdCiv6+1JH3csVRy0xFDrNEyjTHTUdXGvqqKGb0DsP9NMuA7GDQMk3aDi/YNWo 9GlhxdXMwZ3OVucKfRt5BYOJ6GvMGZ0= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=i0IOkhkF; dmarc=pass (policy=none) header.from=canonical.com; spf=pass (imf12.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.123 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=1736307694; 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=k6lMeZIfPylekEqwLKlsJSw3MIFfAKMhJmViidaaGf8=; b=BQSfBq0qPGSWdoekg7gxcBn6frTgEPlWXOvM0tzMiHXHXG+NP5/JLljLIcAjkAZOPJApfx U0ozGiAtx56jr01E+0zArKtFXJQBH/SZqy6f50GMGi1sPDVV05LUfgwCUBek4w2segUht0 RGmg+dgM+ISiB48ogUY92bN9xrCt/9w= Received: from mail-pj1-f69.google.com (mail-pj1-f69.google.com [209.85.216.69]) (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-1.canonical.com (Postfix) with ESMTPS id 06D6D40330 for ; Wed, 8 Jan 2025 03:41:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1736307691; bh=k6lMeZIfPylekEqwLKlsJSw3MIFfAKMhJmViidaaGf8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:In-Reply-To; b=i0IOkhkF/4fwE5XDF+l6zVe+FSBwH/5CBfru0R2LC+R9yHqYsYg52shVBhJ850i9+ Wxzpw7c2QmnkBdFl4HeV3hvK2XuCdtXvCNwBcilGGTPDJ6/tXSrjldzFC27zSm4B3x kva2YKJrF7hzZfCoFvbUT5nndb6xyo+SesfVFUB+QTB996q2W4B4+BYqEe207Wis6H 1K6jgSENf7nqhmyvGyfsHAH9MQzbsyHnU3sORGMW4vCMqjI/UN2wplmzrpKcaXyt+T bVKa/uop6quVOQTCGGakND2HXX2uclBtoeN7s5HnUKzsulrYIYKYv4FGLJTxYq2n1I b+OWviLYkzDHQ== Received: by mail-pj1-f69.google.com with SMTP id 98e67ed59e1d1-2ef728e36d5so24026299a91.3 for ; Tue, 07 Jan 2025 19:41:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736307689; x=1736912489; h=in-reply-to:content-transfer-encoding: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=k6lMeZIfPylekEqwLKlsJSw3MIFfAKMhJmViidaaGf8=; b=U2Y4QFGjrUCmSut3T0Go24HDjk5G+CBmuIVUTupEJHZs7Ex5C8MDJyXPurxRxr4w2d XS5nnQRkCEEKmroSNTBBJX4JzSAeUH6NkUM2ELsOQSkfgAO6SXS+/dB/pD6MiG1zR+FH /K/PAseo0Zv6gRPp2kqD81NXygf1aYKVXIdML1hzKT/qbL6pNw3jX2noSbLHS9kfyvNG n5O6JgThn2m8Hv5mfiren+76Aabocb8tlrh5NJu/VnLeEjxePVzNifnuC19de4MQK8Cs 1M/hFSgMEPTTTM8huz4QJEvJf80fjPyC+Q0LGaY59x/ZK3lNRhRzKEF3pzpEN/6Udg/f Sk5w== X-Forwarded-Encrypted: i=1; AJvYcCXBlfsEIOFLtEvkOaM59YW5XHUJKT2vOxRL8mwlZioo2U67N2yYkSr4cMBA6Du7wkcQGKesfAlkyQ==@kvack.org X-Gm-Message-State: AOJu0Yw8lTFP4fceXRLmqm20p/32Ed8K2HroIlAbeJkPSHb3/TT25pCU eeohTYuvcfJWyl7idcAEXKmjW2PNj6HpBlFx2MhBeWSwn/FCy5Qtqph2VBw5CJ+dXR06yDhbnPy NJ+lLOkZffhmdIsRjhRpwp05uzCe5qBClmoBdp8sHdHH19Y9gReoIM/x/IhfATAza X-Gm-Gg: ASbGncs2DtEOWD15fUH4Nac1yXPtqrtO8T94rkKCeFtbyELhaddA38B1Ab6u15ucJ7w ByT6ut/YH6y57Bzc8/8tKKKpcp84GjR4HhA/JT84C6JWUffk++MSHhGPLkxk3FZ04XztsJNe5Qz 0zRNEP6Lgo5V0g3DvaEv1GVTg+2MDsKcuLAbCbpZLup45VRJnKEqiqMI5VwxJLdqBY6S3zQEo+V lfPX+pE6VWAJV/wnBqNtnWA+Zq7MFGM+mgS05xR41kQQVXC5dt+HmIs9tc= X-Received: by 2002:a17:90b:5146:b0:2ee:c457:bf83 with SMTP id 98e67ed59e1d1-2f548eca259mr1947880a91.19.1736307689017; Tue, 07 Jan 2025 19:41:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IFRVrP8j1xdDrjy+nV8eMKLPooFBXo839/1nwVsnAYHqBM3Rx3JT+VoR9qx1h/Q4cpvz7qavQ== X-Received: by 2002:a17:90b:5146:b0:2ee:c457:bf83 with SMTP id 98e67ed59e1d1-2f548eca259mr1947862a91.19.1736307688647; Tue, 07 Jan 2025 19:41:28 -0800 (PST) Received: from localhost ([240f:74:7be:1:4e52:6214:fe82:b2d3]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f54a34dbeesm367614a91.41.2025.01.07.19.41.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jan 2025 19:41:28 -0800 (PST) Date: Wed, 8 Jan 2025 12:41:25 +0900 From: Koichiro Den To: Huacai Chen Cc: Lorenzo Stoakes , Huacai Chen , Andrew Morton , linux-mm@kvack.org, Sebastian Andrzej Siewior , Mark Rutland , Charalampos Mitrodimas Subject: Re: [PATCH] Simple fix Message-ID: References: <7gd4skn6l4i3liu6cvzmgqlrwxc3rukrnau7lnpyhmfyjuvqwl@gdwgybedp5gs> <20250107011848.689556-1-chenhuacai@loongson.cn> <3a5ad843-6d00-4e8c-9f77-f53282cef4b3@lucifer.local> <56flobk2ooduyk5465d47tgaz6jkeupbd2qjk7lt35ust2etnh@5l4os3td5imr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Stat-Signature: 67o4nbq4e5nx5rcds38cm95fjme1wmx6 X-Rspam-User: X-Rspamd-Queue-Id: AF1B440007 X-Rspamd-Server: rspam08 X-HE-Tag: 1736307693-420424 X-HE-Meta: U2FsdGVkX1+6zKjKWOy3/W9J8s87s1skyeTcoo0l7mtKx1XGvnvD5eonU867EiE6HMu3oambnj7lhTlMOjnNvZg3ooAYWI0MyadWbELT4p8Nz97XNWaH7chzHoOH3avXltI6cXCal+ofbfEEwrZHPZgaoaRXxycRycw9WtmP/6Rax34eRFhRc1S0Iiv8D0BMOUr6TKZG44sfcHGl6rsWKBe4Xsjrelt1+xbAFqnPtoECX8qdBNZcgBEsN0vJ3KjtO5K/asULceUFQ0VUcQkGJdynrvGX8hFKx2ukMvBHQ3BBpqSvMbTYqVN91xhJrOk1n33mdELcUeV1adRi2gtPO2mHMK4hV7S+f2VvtUSCVvGCRqEek9xrhOUIT3NlA3StwZaezB9ea4Ay+xP/kkzMV/Yzr0ykpDul8yU6Ne6hDGAY5THPK+HzLEqeR2BTPhpeiZwmhYdIA2wtpXmmPzNHDVzr58DTt48F8h0vxUvErPQYRJmdIe67Q44JbzZCxHqvbOpmvqcNeL/49cIHiJSDt89OvYINlPkvHVPjzu+RQ8/2CvBz1cOfnxfXdtBN+XE4XyEcS0rb4a9SVMVudcPScc/5Ovt8WaNJ1VLYP81cJajfHtcttvhCyqJm371wz8MD5mBbsqlCwFR/2bXt8vnpHkMqTatryswALGCFbIxhRYyhqOijq3KNT6l3h0Ua3ApwX3igQ3puKGyXIg9R0tq79hrMh3hFJ+vvwSUAAQXhKx9jahTEkGj8A1x7IjVilCPDAmqzRSE4I8OEunqZe+NDtdiDzDZb4xxyg/wy6rsOAk8Hx5C0ub6hQUaouyyi+7nJwVIJbde1tBOFEQHYbaYsxLyYEZvbLMBv6lXR2qOAd+NL+ZKG5XJoFInuoxNArCMeI4wwV3m4Z/acjB+nfp7BJaFK0fz8iXshUrRxIWujJFTV1dqAxQzi44mqkh8RALvUb+pUEZETkFJAh7zYqaL PAFb5v0+ 4hCy/2kmOx6koZTTxKOrm36tjfUo1r0GGCST7PPc5uGyKuuxic5OKw9EE4vBFTc1Q3nHReElypvqo3PIos+htlWi8TmVtp2DYJz0sQr6T2Z9uPbsn+PGy6znn7MtxtTfvPeKamXXSFh+oPCK3Sgvu0UZlJ3jtdCXx15WLHgKabDeEP3JdJ5DESEyg/UVI2eAfjFtckK/4Xa9WeaXi3p6mAnz37c2T0aYRuQhiH8LvhrhCckEse0HSL2B9tzSSrHSAK9zYYMP8BjD7qX0RM9JTfJgHf4tbzpGobL9/ZvsvJsVoXhczMAQx236FBClgSo/qE7fqli6uJ7w3e4H/Em/ib1YgCQVkiO30JZldijquyrs20MCS32QtC3vAFDXwLeZxTHeBYE6EgBkcnHI3O+mG8AKmZmBdfXXKVkBWNf8diq4jD7iVqmDWH8EBnuv1etKia/q3s06JwgRfHZY+9/azIAD7vQaPtKPqN9cZUf5vhVB+3EDNM1iSBv2O9fDCwflMy2y6 X-Bogosity: Ham, tests=bogofilter, spamicity=0.001145, 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 08, 2025 at 10:31:40AM +0800, Huacai Chen wrote: > Hi, Koichiro, > > On Wed, Jan 8, 2025 at 10:22 AM 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 PM 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 Koichiro > > > > > 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 miscommunication > > > > > 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 just do...' > > > > > 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. You > > > could simply have replied to the mail or given more information, you 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 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(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 suggestion > > 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_internals(). > > At this stage (i.e. start_shepherd_timer()), for cpuhp scenario, the secondary > > CPUs haven’t 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 and > > > 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_ONLINE <-> > > CPUHP_OFFLINE only.) > > > > [1]: https://lore.kernel.org/linux-mm/7ed97096-859e-46d0-8f27-16a2298a8914@lucifer.local/T/#m11d983715699d3cea295b8618aba7b6ccec4db55 > > [2]: https://lore.kernel.org/all/20241220134421.3809834-1-koichiro.den@canonical.com/ > > [3]: https://lore.kernel.org/all/20241220141538.4018232-1-koichiro.den@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, 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)); > > + } > > > > > > 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. If there’s any documentation that mentions a recommended method in this kind of situation, please let me know. Personally, I’m 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(vmstat_work, cpu)); > > > > > > + } > > > > > > + > > > > > > schedule_delayed_work(&shepherd, > > > > > > round_jiffies_relative(sysctl_stat_interval)); > > > > > > } > > > > > > -- > > > > > > 2.43.5 > > > > > > > > > > > > > > > > Cheers, Lorenzo