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 A9DA6E77197 for ; Wed, 8 Jan 2025 02:26:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 361B56B0088; Tue, 7 Jan 2025 21:26:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C3FE6B0089; Tue, 7 Jan 2025 21:26:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0EF046B008A; Tue, 7 Jan 2025 21:26:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id E01D06B0088 for ; Tue, 7 Jan 2025 21:26:15 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 8A8921601B1 for ; Wed, 8 Jan 2025 02:26:15 +0000 (UTC) X-FDA: 82982695110.16.32F3FEF Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) by imf05.hostedemail.com (Postfix) with ESMTP id 5273710000C for ; Wed, 8 Jan 2025 02:26:13 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b="R4T/JTsi"; spf=pass (imf05.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.122 as permitted sender) smtp.mailfrom=koichiro.den@canonical.com; dmarc=pass (policy=none) header.from=canonical.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736303173; 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=KsJRlj4rmNt37Ct0GbiY1P4NsM3BzcIb2ks++JxDUIg=; b=v9ttk4GD2qzW/2/JGOnlgGTlLupm8NzEP9a8L4BCrkg6Zs0KJat7F6yZN5ov45ZdxDhoMg 5+GGQQKx+RVQMwP4Gg8WB4ebr9pz0f11YOVL1bemSLLT+sJpt0W/TxctlQx6pFRjSVL6dt 3QdRckZkIklHxQZEMor5HcPancoLrWA= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b="R4T/JTsi"; spf=pass (imf05.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.122 as permitted sender) smtp.mailfrom=koichiro.den@canonical.com; dmarc=pass (policy=none) header.from=canonical.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736303173; a=rsa-sha256; cv=none; b=eyvBYdjxsB0+ORqVhe4J55pYlqq+I/FcDLw8ES3cg+QHLsYXeqHxsgpF3dVHm/7ygghYNh 3rqiQ8HlGF9cK73emiaOA9FPi5p5NAf91Ng2809WUUbAi2pfYCadZGQWXVDaNzd4RqBp5Y Zpblp622JWLwev6gdeQKZSAarKnM9UU= Received: from mail-pl1-f198.google.com (mail-pl1-f198.google.com [209.85.214.198]) (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 22D223F86F for ; Wed, 8 Jan 2025 02:26:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1736303171; bh=KsJRlj4rmNt37Ct0GbiY1P4NsM3BzcIb2ks++JxDUIg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:In-Reply-To; b=R4T/JTsiqtlV2FrqroRrACmgK6EWFX+FJKX+S0P6zlNEMmUkS0jI/QqwdMIDY//em IBJ9WPNmCkmJqoK+W07vZJ4vrno563YT+5tgP7MVtjOW3j6F2POalR/BilmFFW0Dod GHnQ9FcFY3dRVAaER4S4EHwLqjxGNVDtfCxLvFklgGTWlbClDaI9ZruiGDkZkIbT1u G2SDA330iiLVSmHwMQuDHh424DHW3YeuwiodixEfYNwcm5EoPQPDjJ5AOfz15HMxRK 42tTdvPoOyC/feRTMjrvqumiD1l06O1quk9rWV895+YdNqEyCkEtqCl3wKtppOkrW6 0VO2G41WKxeUA== Received: by mail-pl1-f198.google.com with SMTP id d9443c01a7336-216311faa51so7634985ad.0 for ; Tue, 07 Jan 2025 18:26:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736303170; x=1736907970; 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=KsJRlj4rmNt37Ct0GbiY1P4NsM3BzcIb2ks++JxDUIg=; b=eoYQTvhGNxsSKijEihfw3n526zNVyqc1kSM8eHKQefWpY9lvw3gN/XoDdFJ/P3r7bK qJyK+Rf2Q4fr3OJEnc3nJUqrr8ZR5D1jKJUYVYCxPGc2vIPP4nUrGMA9jxpnG6e6qadC I17A7bMvocgGWJTLC0C3hAox+NGnEm97GAyH3gk8AByiHjqAO0fL3ELj/u9tdd0TG4oe O4tTmOV+y1Uut+AvN4vsC7iwkXoqkWKKPgiMTKRAzd0l5yotmx0BDAKcgz82cgm0kGyt x2UaHuXvdMQXIdqLnBqJI2f5s9v/UkyPJXg3rRoMfzO2jYND8x5JZbhQyFtg2CiM1I0e FP8A== X-Forwarded-Encrypted: i=1; AJvYcCVGvgbrdIo5+uW8KHT0xzBY5J/eciDtd6eqNhsX0HVg5Qo6n0Nj3WhfM6qANygvea6wb/im6Q1laA==@kvack.org X-Gm-Message-State: AOJu0Yx9Oi6c779bkyIvIk39OcC9gL6CoOBnDynmU0+1tw8nX4YHzKCk dz6PIxXETiqCgBx3H/kylEHj2ZC0igqmRRTPoal6lAzVJ/7Vk4as7H2nf5KkWZ1AOVrCTxz7C6L 6vzru08gX31OhK2AdON1+Zm2+KGaw62mAwUuz1TDDR63pULVyxbv7X7jAxSBmZvHv X-Gm-Gg: ASbGncsk8SNDxUZJSlfaVn4hoCMGTWaf9u2d4I42bOhbXgldo8wSsoPW3RIg/2bztXc DKkxHoflrMAthwX47rYVprbG42QK46t4kYrEFsulhOsgWVKG9yCCwd+EV/jpRBJ3TO2vw+wwbvT WoOjkNSmtCbaUbTAtWXatGTP0U1l9E50X7qixDI1cbpHlcl6rbTnPrHd8nrY5vfCFd9ddLeUO1v GwPhDESpBn7Kszyg8oU7ecNEY8iRZcMPeIAjiQL9Gsw2uGNlfsvPw7GUzg= X-Received: by 2002:a17:903:190:b0:215:7e49:8202 with SMTP id d9443c01a7336-21a83c16666mr26334625ad.13.1736303169620; Tue, 07 Jan 2025 18:26:09 -0800 (PST) X-Google-Smtp-Source: AGHT+IF+Sb9MThTAqFqrfGXiIzRpzWqOSYWzI3uULEsGd78+8JkoFRSl6iZ9UHhSOGhzq1tYN2ZLNA== X-Received: by 2002:a17:903:190:b0:215:7e49:8202 with SMTP id d9443c01a7336-21a83c16666mr26334365ad.13.1736303169177; Tue, 07 Jan 2025 18:26:09 -0800 (PST) Received: from localhost ([240f:74:7be:1:4e52:6214:fe82:b2d3]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-219dc9cdec1sm317044505ad.136.2025.01.07.18.26.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jan 2025 18:26:08 -0800 (PST) Date: Wed, 8 Jan 2025 11:26:06 +0900 From: Koichiro Den To: Lorenzo Stoakes Cc: Huacai Chen , 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: <56flobk2ooduyk5465d47tgaz6jkeupbd2qjk7lt35ust2etnh@5l4os3td5imr> X-Rspamd-Queue-Id: 5273710000C X-Rspamd-Server: rspam12 X-Stat-Signature: pn735fwmfpigxt65xmwmbwc1g5xxbhqz X-Rspam-User: X-HE-Tag: 1736303173-492481 X-HE-Meta: U2FsdGVkX1/wlOyeqcgjylzFL32QdUlk+z1eVvr9gyaM8/WPbtCxDGIz75TR5S7eGNnPfI0yS5VtqlA99fnlym2PC1OBDVUiFMBStn4ajEU6Au19zsKjEnMge2yX30YnkF2s93ZNtLd5KSw9jsj/0pcMVIi/bK7bsAJb3Kbga6KrMNuFtO5YmaAyAbybqsr5XhmEq5OOM/snZm1biH6ikiD4QP7TtihmXBY1AyR9+GcSWCi4Dmhm8wBNQu+5GYBIr2hmIgRCcE8nrtzgwSg3zpBk64C10eChIYWcuWTp30qU349XA0mrzxWRA2hYVeuvzdJ1FNpDXZLkYLWrkITwPGsqVL+nqoU8uU+30v+btYKY/HjUcyAT+Pf0Vc7IyPL4FRuXejJp8qc3mqFYK9wZ9FPh16zx1uRb1I7liFdSy7dBzKi29M8tkArtx8adaSEyKQqsd8op5b6zl9NDKaXWf2MtyKOQTVGabqHWg+8NZVnH8ypw4nX3gOBUDZK670Iqiw2PcdWXxJYfEJsUAqQIuVZU+nYF6pEyqSB7Vx9ZD4Y73pWQD3uqP1ynrb/s5HB0fufpZk0sjxTysU83Ccg5hyC1dy1vxUZ/dOZsLnKLb5aHhONm7+XNPIZJKIBUyKSbwRki40cpHTY3kzN6uMtVTfKAS2tEoe+4HN9DDAz5SitDqv23uij3c9hf5zY9Mu0TlX6wIzIsbWuWKM42OyF4/LQ/pnU5PeHL2EMnCNtG48e4joZT+UcQmjlAAeSRwnxIULgofSEYiQHjPSW9TtK77uCc2WI5XpJWa71B3YlogDfBpKAam+6j7GHvBbM1cjVISA0bh0mmYDeYLgNELp6z/TGuOJKpXkF/GMC6x5czZyBxpFw7VlV0YzlTmqGgO6piL2vJs0P6rnp0C/WvLr6zaoMnR3Jld/tRXMXHTh92fvDHM0UrgxkUXLk6719LHXxZtVd5iKKBKfvMs1SOd7+ YvPRg6wa b/v9Zzi0pbZ9uxrpZHOote+mbWES88smJbby49RSTXFeU0Ouj1+5G59yWRYgBihN9IlT9AM/wloLDku69KjBdrq4cxUf+R5wJo1FT34E3YFmrd7SmwwTiWeUDULfQVABXEvc2w4jt4uLGV/E5p/laXWFR+55xEie1GeRhAZYOjmtyOplaNQJ9TysJkednTaQUN2jtK25OYe2Em3E3KW98PD3DidOLrEbUilc+Wv1GliISWI4R+yIESnP+x/DspxCkqlNrAshcUSh6oJqhPJleKQbbZfiPjAMcFHLSP1JM0e6vSt+CNi9WQ7zKHOWE5c4JeUiDt6NsdCoFQZRXhzVTnOjGhzZV5mttp15N3lcJBU0GT2ayfYQVfNy+r2KOHOe0C3+nJp1UTJ4zNsdjYR2Ft29I+hJjVG87DwwpWp4/6B518MC6zlsXn6okcL5YF98LzLFBJCXFJ/nvFCF8dtAj5L8gTFIZJV4Mx4uxGcL1cId4wm33H3RwjRePjxURxnnFyrX7 X-Bogosity: Ham, tests=bogofilter, spamicity=0.008804, 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 11:22:42AM +0900, 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(). Oops, critical typo. s/before/after/. > 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? > > 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 >