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 B184DE77197 for ; Wed, 8 Jan 2025 02:22:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 29C766B0082; Tue, 7 Jan 2025 21:22:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 24B8E6B0083; Tue, 7 Jan 2025 21:22:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0EC3F6B0088; Tue, 7 Jan 2025 21:22:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E4B0B6B0082 for ; Tue, 7 Jan 2025 21:22:54 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 664694515C for ; Wed, 8 Jan 2025 02:22:54 +0000 (UTC) X-FDA: 82982686668.24.9DDFDF1 Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) by imf21.hostedemail.com (Postfix) with ESMTP id EC4BC1C0011 for ; Wed, 8 Jan 2025 02:22:51 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=STdTc0LL; spf=pass (imf21.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.123 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=1736302972; 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=F7VcWKnH2zpMqsElNvY9G5Pbio+EQJR3fOcJk9XGQcY=; b=KsTqy1qMANILFZBtSloGuG1e0NA43TUbXia5X7ZHQUFWhRUTB2WYFUxFR15Efph6FIpUiS YnjLB0wpY8gIzeWIjL8qg2jdX3O7nWrHU0V5lsKwJLl2L7HTSFR/yX9H4a7vyleeLgZdne d1gpxuWEBnG2fd/YftWGcgqR9d3iC84= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=canonical.com header.s=20210705 header.b=STdTc0LL; spf=pass (imf21.hostedemail.com: domain of koichiro.den@canonical.com designates 185.125.188.123 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=1736302972; a=rsa-sha256; cv=none; b=EFF6aFwgfCoMrmrNDp/cj44VxhV2Lni71JgsjBOcJF8dptHBxY64ZmY7HZhYbKDggzK72x 6x2W1D0a+xGB0sVNdintgbf6KnpomlQXcGNnW6K73IElZevzdKA45Hr3N7ZpCOsZ13Lzll vRkI6VUQmViGyroQAVQnJs2Jjw2MKcI= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) (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 998F740330 for ; Wed, 8 Jan 2025 02:22:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1736302967; bh=F7VcWKnH2zpMqsElNvY9G5Pbio+EQJR3fOcJk9XGQcY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:In-Reply-To; b=STdTc0LLFW/2TVhZxPCBjjjNNOd3h9pxyNv2qfwgrcnBjZDBdlFc6AZFrHY/lRya7 OiwJpwVLw2W3XKKBaQZMOLhO7XGQ47YJ0XLGZuXUPNuS6GxoA0nQqna448xxFUZGKC QqILyzEi55SwqUK1gK8AtxOD+mczSu+4/iTPB2DzY0fHoEEK20ntH5SkKjKL7MI/dr ppWm4R02Hm5ZiW9iMRJlL/CYCZq2HAO0IZI7vZ7H54a7Li9mc6kO9f0CtkL/0cxhr7 iB2ORbONgoBc4KDkIH+ApFAvmq9EHIOUCFfe4Gs5uNuOdDJnhXIV5zccD0UI9Dy9h1 01/msgkjXTcIA== Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-2ef80d30df1so23172585a91.1 for ; Tue, 07 Jan 2025 18:22:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736302966; x=1736907766; 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=F7VcWKnH2zpMqsElNvY9G5Pbio+EQJR3fOcJk9XGQcY=; b=eIAhoqCLvjgg6IcI2qEAImgjkzAkrYZ4mlhrN9P8PxSCCRgOiMR6btkUxDe9ho28sr koojRQYga/oLI7R7S24tHu9x51XlaVD5f5bp0QZu5wqJa/DWqHcO+ilXn2DKt+/gjDu7 +DJnju0dpQVvrEPy68cdCyiTsiHE2v8MEqE1you7h21NbgUN8QuhwGnDLJKhJZtEIYPc 75Pn+BtrWhjf344ZdngIGoKQVCX7Xci34N1vuUwi6Dh4aBafBBrmXAoFQ2KrFyFDJMCp jilt7mWVPXpd9n4aifk7MkUzdYh4LLiG6xWZeK9SaxJFlu+FlHyvhK9YpEIbE6hMbCYv Rp4Q== X-Forwarded-Encrypted: i=1; AJvYcCVeb3uBDhtPKb9fwicuLXvL3oNMNxRVK4AsWfsFChBWhlZ03KpmYywXa7Ou6StyzbjpoX/SRkjI1w==@kvack.org X-Gm-Message-State: AOJu0YxB+SzLBJqL9DF18Z8URIwucrNS/DIRzlR0Crx11FDBtFfZrGgg LMnhKQe1saSmtH/DCkuMSdBoiz3DJG44p5TFR4/FSB8j+OL4/9z1ICDzmpmXqzNgPvOQNEzgu5v N9FabM9cv2mJaDRo4rbmUys0lyMWqTgRrtCnWbafNeedeuC0oevuJIx9LppAhCPUc X-Gm-Gg: ASbGncvDMdo7pAp9yXAUUEm85ItwH2x73ueveg0fTfyOTU8bSuIndg781mV9boQMgpr MnLFCmsKjjy1OJ9M3oglAkJNWtIvUXNsDipS9+zWPuwqQ27rS82rfWoBooORODHMjlts3GCGGZ6 o2II319n4Y78+6ubaX36v/FXp61Ncym6oPg0YoHmKxtyKBRolDEiJSLM26yKfHHOBEADTI7QIKT Y5pc693EjCeAKOYr+Hl8BKIyZuReipp0D4d3zeHN1z4N+oL1mczey1l+og= X-Received: by 2002:a05:6a00:368d:b0:727:99a8:cd31 with SMTP id d2e1a72fcca58-72d21f47fc5mr1835867b3a.14.1736302965558; Tue, 07 Jan 2025 18:22:45 -0800 (PST) X-Google-Smtp-Source: AGHT+IEaPa3DN13SayhEHdLomrA3jD9Ax/czEAuXDcxJgyJpcJoZHPwMTmwKI0cVOXJ6ISLWeB/U5Q== X-Received: by 2002:a05:6a00:368d:b0:727:99a8:cd31 with SMTP id d2e1a72fcca58-72d21f47fc5mr1835830b3a.14.1736302965145; Tue, 07 Jan 2025 18:22:45 -0800 (PST) Received: from localhost ([240f:74:7be:1:4e52:6214:fe82:b2d3]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72aad835469sm35353870b3a.60.2025.01.07.18.22.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Jan 2025 18:22:44 -0800 (PST) Date: Wed, 8 Jan 2025 11:22:42 +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: <56flobk2ooduyk5465d47tgaz6jkeupbd2qjk7lt35ust2etnh@5l4os3td5imr> References: <7gd4skn6l4i3liu6cvzmgqlrwxc3rukrnau7lnpyhmfyjuvqwl@gdwgybedp5gs> <20250107011848.689556-1-chenhuacai@loongson.cn> <3a5ad843-6d00-4e8c-9f77-f53282cef4b3@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: EC4BC1C0011 X-Stat-Signature: rdu7u53dwoenqyae8n741ornd35qk9ua X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1736302971-71798 X-HE-Meta: U2FsdGVkX194IyybA/D/MyJiPsZwRDygD9QvJCdoAGniyHHixLyuOlDnI7SVfO1f45WqF7YQp4jbIgKE2WmVRcAA3pI5Yfg6Q5rNikLj4RT3oyMjHMN1E6UWh/JESAYKrurVWfXqzPffttDBk9N1t3oZrYZhsFmmlInhZIXQ3eQYb3fvrRLZVgQwG/BRnaY/kVmY4En5QaZ7zA4tIfLpUEAIiTh1A530o6o2HVr9xVkur22uZzFlBCbG5i04pEkNF0QUiGqwrMPblcn1Y+eW/uBBC6I7ryQpdODCaxs49VD5QwUKlGzB4309+7g/B5D7q4AI2QSfgCdu6Ay7QM8xpuT5j9/NSuBCrixWtdVQGRPWy2t7AQnFAzEJxqTRKPR2OwfARpVRp5noGSpPE9zaIV6rM5zSMSxizd677aLN/Psh3pWXQdwoteX2KUOgYe+tDouTbzQST9qRD3Ss1Ouj+ln5B5gnQt5M2+DOMMd/gpD/z1lpx1DTQizpFJe3DsuHX2jpBtJmEzSuH7qx/D8M1pC30ZBC9kwnKAwN5xUBk/j1AMAlmMNIjCGt7nQlXU38SnNuF4//kGOgkWEiPiQRD5kVE3SC2ewB4u6EZgOI+yo9/QfiQNUR47Ku2TyckuWGyZpJj9Vlflaz5fFKazAYUX1ZpaJrgTC1QkaHgvs9CmAKhXYTUi95SY17HVyD79dJZ3+fmaFX9PJoNNODIv+f8i5bUgLzQcPXJfONyI+fyK+ScwVPxb3txbkoi/F0SwuroISBBvLW/+jVYosxNOG4CNseIEdoDZum8l0hBhcFhXs/CMdLCgoKwj0cwpeRFUAamREUB8nEM1WyvkZB9wn1BK+VjR+JonB49sdzk27umbeORbPM4GKVti/E5+xh04m2ybuXy7DlUtCyaiUlFTBI1yWE8OLAXyVOfzdABQ3PUN7+HG6DR8nPDw3dXPUNQSHO0YQkKZpnkw1+PrHtDck NepMKxMJ tTG31qv+Y/UgIrTsdR2RFmgPBzvjo69QrHcdi1367byDuGpBlo0OtfdKCJ6FHeJBdSHL7j/TwSrrtodaE2l2zukzwcwVKNRZP/xoeALE4uOhKJX72nWtSAPxoDXaEEAIe5YY7ohsoD7WoV1mlzWRwU+SUgGRbebWE6O94n2zwxw4UCYnqnXHDfh7PsfghJe28trjd/qOQ8IjenTv1IpMt7vS23tUXkAaffm1LftyvIfQgxHuocAM7Lj8u0Mf7gwNWAXfQMn0/1bzEJWLu3cw1KUF01PHBdLqlNsbdUKE/7SebG0kXdAApFmV99QMTBJHvHBU2QzvwAJht6grPcqMLus1CY0++YfKQ+O5ZMsv3yqyJu9UR9CsvyzTWvv6eI8Ots/FB/YCnv+PKhFZGWnTJ0JYoy1b7kVApGpm0nbJFjBTriSWET9GTpKZITPbOyxnk7I5QysIbMxQELYIgT2AcBhzd3LozFFbo998mV86q6u+kZhr34alDZIutSABRMIia/wNP X-Bogosity: Ham, tests=bogofilter, spamicity=0.051840, 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 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? 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