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 518FCC3ABA3 for ; Fri, 2 May 2025 07:40:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B93E36B0092; Fri, 2 May 2025 03:40:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B42466B0093; Fri, 2 May 2025 03:40:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A0D8F6B0095; Fri, 2 May 2025 03:40:18 -0400 (EDT) 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 83A1C6B0092 for ; Fri, 2 May 2025 03:40:18 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 4832A120C50 for ; Fri, 2 May 2025 07:40:18 +0000 (UTC) X-FDA: 83397169716.20.A65D65B Received: from invmail4.hynix.com (exvmail4.hynix.com [166.125.252.92]) by imf02.hostedemail.com (Postfix) with ESMTP id 2413380004 for ; Fri, 2 May 2025 07:40:15 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746171616; a=rsa-sha256; cv=none; b=3C4fzqPY7uzfY1ClK16BAR09ts5byLAY2lQkv2pK5Pe4UzidFlR1L8VN284gJZZjYjQLAi w2foWBso7LUjdiE7mjw1TC/+jxjQ6GYGTactNnKc97+MiA1AWccTB+dgMZh0BsjAMhK0W8 KaOm77jbGqXCmr6kxsZXL5msCn/6TAo= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=none; spf=pass (imf02.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746171616; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AWovFM5/2z2YF3drYof0NVs2Iaz+Kw6I+vqHzFJswlQ=; b=M43Srgq0mMmlytqIWE2Pm7ewBiK/ghbERrOoRqwR5ST3hntNGF8TeMLBMbTJB2IXkwDYUx 055Qlf3DTl09pFD6wjONnxRjffgJK7NK9MFZORl2HF1Fg8EU6poQqyREYwscNJWmjSBVmi B6XgN5WeNAaUKY4wTF2ThkMP2cvL2yY= X-AuditID: a67dfc5b-681ff7000002311f-af-681476dea69f From: Rakie Kim To: Dan Carpenter Cc: Rakie Kim , kernel_team@skhynix.com, Andrew Morton , Dan Williams , Joshua Hahn , Gregory Price , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Honggyu Kim Subject: Re: [PATCH next] mm/mempolicy: Fix error code in sysfs_wi_node_add() Date: Fri, 2 May 2025 16:40:02 +0900 Message-ID: <20250502074010.153-1-rakie.kim@sk.com> X-Mailer: git-send-email 2.48.1.windows.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrLLMWRmVeSWpSXmKPExsXC9ZZnoe69MpEMgwuz1C3mrF/DZvFhXiu7 xfSpFxgtft49zm5xfOs8doutt6QtLu+aw2Zxb81/VgcOj52z7rJ7dLddZvdYvOclk8emT5PY Pe5c28PmcWLGbxaPz5vkAtijuGxSUnMyy1KL9O0SuDL2PjvOVrBUumJZwzGWBsYZol2MnBwS AiYSMxb+YIKxp748ydzFyMHBJqAkcWxvDEhYREBH4t/fySxdjFwczAL3mCTO/r/LDpIQFvCV WPz/GyuIzSKgKtH7exkjSC+vgLHE1xn+ECM1JRou3QMbzylgILFx+zcWEFtIgEfi1Yb9jCA2 r4CgxMmZT8DizALyEs1bZzOD7JIQOMEmcfX9aajbJCUOrrjBMoGRfxaSnllIehYwMq1iFMrM K8tNzMwx0cuozMus0EvOz93ECAzkZbV/oncwfroQfIhRgINRiYc3oEA4Q4g1say4MvcQowQH s5IIb4wBUIg3JbGyKrUoP76oNCe1+BCjNAeLkjiv0bfyFCGB9MSS1OzU1ILUIpgsEwenVAOj rNjed+uZX9xUnNQt52lepVWddfBhFcchyfI9LorWHaoTwlb4dvpdNGW7sJG97UfyR9YFCzYd e7i01lvFdsWWibOD+aKFE1wrld+vOKZ24zivG1vhiYtiHMGLNjubPD7w42cM87L1blEtcezh H8NM+FMan3+7IZvImWauXNG/4fcHnoCKp3+VWIozEg21mIuKEwEokfb7YAIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrELMWRmVeSWpSXmKPExsXCNUNNS/demUiGQVeLnMWc9WvYLD7Ma2W3 mD71AqPFz7vH2S0+P3vNbHF86zx2i623pC0Ozz3JanF51xw2i3tr/rNaHLr2nNWB22PnrLvs Ht1tl9k9Fu95yeSx6dMkdo871/aweZyY8ZvF49ttD4/FLz4weXzeJBfAGcVlk5Kak1mWWqRv l8CVsffZcbaCpdIVyxqOsTQwzhDtYuTkkBAwkZj68iRzFyMHB5uAksSxvTEgYREBHYl/fyez dDFycTAL3GOSOPv/LjtIQljAV2Lx/2+sIDaLgKpE7+9ljCC9vALGEl9n+EOM1JRouHSPCcTm FDCQ2Lj9GwuILSTAI/Fqw35GEJtXQFDi5MwnYHFmAXmJ5q2zmScw8sxCkpqFJLWAkWkVo0hm XlluYmaOqV5xdkZlXmaFXnJ+7iZGYMguq/0zcQfjl8vuhxgFOBiVeHgDCoQzhFgTy4orcw8x SnAwK4nwxhgAhXhTEiurUovy44tKc1KLDzFKc7AoifN6hacmCAmkJ5akZqemFqQWwWSZODil Ghg3bQ6ymbd+m/2uVduqH+beTcwXVE67vVbaYF7tvHXFUnISbR1NR59M4A8Vi/oczOIkfOoy i2D8lmy9vooi8UPTWjyzRQuKNt7X/Kmf/Z5zzccF+741m31Q/s0c6+H1nXlx3e43XOyyWw/O FK0UEGQ6wNqQOa9qpnhmdInRKrlrh7hmtD/c5qjEUpyRaKjFXFScCACWzsRDVQIAAA== X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: 2413380004 X-Stat-Signature: qppgzekth4ne6f8kw5gbob4un89nrmz1 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1746171615-3508 X-HE-Meta: U2FsdGVkX19GAtRUdWzN1xdco2xSfekTRhBayJqwHcnBMuQHkLdU+CguWRLhUmOHb8tnqAq1gaXopF+Fbr70T68HHym8noQ11rotSGLIctC/edrlmcTLdavLBkuXnF3QdrJ7PscMDFLqoAb0AFjjbgV30mhF60seKI83A1eYh79TQfdCG38f9bIPezuFxt9vrmWKhMIZa5Zm3tKVPD20qqAEtF/IQT6jkXo+9tYa7ap65ZASip8w7tZQEFjHl0S1QtbB19hh/auDAYW2TVvI77iZ0EqfFDhC7kdf3YPPvLyVFf7Eijyz/tUIoai75mPDf9j9y1hPPOKiepoLawRv70ynfvoAy5ubmzlfld+bjJxGQpWChMa60jBv1nGcqpEKCl/HxYU3K3+fV+ofzDxlRy2rvI8CauuYe5rhyhMghM4F7i64dKU0k/OKdBimz/AmZ49WI+RobQh1kbsCirDPqomuOR7PKfvnBNTLZX5V05Yck2APhIelWdpvSo3448DaBNepsT6FMaJ4cTGnIEFLxD1/hWRBISBYffvIr+wSCopIkJJqccgZtli7APS8pN9aCi4OqDl0I3qGb9sprq+Wmgq0Xo7MGuZ5we3Kd2+J49+kjc7DeZrONmyq+Pp1vzC+CK53BaPcXK0sIjeHrzltmJWXEgqTiOOCsoSiRNV7Sjjt88eIUFrAhuR2w3WtA5UNF+0IcPsJmLWCk/B2fa9LMsvG3cVwC0g9+P/jhqI8UsjkUaMZZCqoEIF0SIDuvT5uTyfhH/yAy6nw2YClEL/s8ce/wEMB5osYnWkcOTKglCOUcTOXzpN/tIOljj/KfdwNQbqJahnr379brFO8H78603iKMjUkn2joN0bmIxSOU0qWiNwfKFMqnkczIIvu6c71tm3qEpVeTk66P3cT9knhqbHXfwh095qLGikF0im+rCLMg0g6BnRZ58uPM4H7CWCTN2l+OABBi0FJDla+312 PgFYv8fw 46gEhYhLtXxrysrAhGdc9OK1TtdTuMMAa6DZxvtigmI/MxP+vDUB4+x0OY7PZHS83SM2pPCHucOT6C7jzbnsp2kwKosDGwU4UoUE97v1XnCT4+zH0jOWHriJoGrb6Xa5fyDvjwIicxcRkcuMPI9hIkSjnX3PEqV92t60Je9LssXURR/6eYDpTcGIrGCmZWMLw+yoAmdcPJD1yR0KHu7Aj4Qx73OiG3WlNPRTB0P6e6ViMmky49CjZqPfJLWPcBuBz+scp X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 Fri, 2 May 2025 10:10:33 +0300 Dan Carpenter wrote: > On Fri, May 02, 2025 at 03:46:21PM +0900, Honggyu Kim wrote: > > Hi Dan, > > > > On 4/23/2025 5:24 PM, Dan Carpenter wrote: > > > Return -EEXIST if the node already exists. Don't return success. > > > > > > Fixes: 1bf270ac1b0a ("mm/mempolicy: support memory hotplug in weighted interleave") > > > Signed-off-by: Dan Carpenter > > > --- > > > Potentially returning success was intentional? This is from static > > > analysis and I can't be totally sure. > > > > > > mm/mempolicy.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > > index f43951668c41..0538a994440a 100644 > > > --- a/mm/mempolicy.c > > > +++ b/mm/mempolicy.c > > > @@ -3539,7 +3539,7 @@ static const struct kobj_type wi_ktype = { > > > static int sysfs_wi_node_add(int nid) > > > { > > > - int ret = 0; > > > + int ret; > > > char *name; > > > struct iw_node_attr *new_attr; > > > @@ -3569,6 +3569,7 @@ static int sysfs_wi_node_add(int nid) > > > if (wi_group->nattrs[nid]) { > > > mutex_unlock(&wi_group->kobj_lock); > > > pr_info("node%d already exists\n", nid); > > > + ret = -EEXIST; > > > > Returning -EEXIST here looks good to me, but could you remove the above pr_info > > as well? I mean the following change is needed. > > > > - pr_info("node%d already exists\n", nid) > > + ret = -EEXIST; > > > > We don't need the above pr_info here because we delegate a warning message to > > its caller wi_node_notifier(). > > > > This can close another warning report below. > > https://lore.kernel.org/all/202505020458.yLHRAaW9-lkp@intel.com > > > > If you apply my suggestion then please add > > > > Reviewed-by: Honggyu Kim > > > > Rakie Kim was pretty confident that returning 0 was intentional. Btw, > Smatch considers it intentional if the "ret = 0;" is within 5 > lines of the goto. Or we could add a comment, which wouldn't silence > the warning but it would help people reading the code. > Hi Dan, Thank you for taking the time to review this code and point out the issue. I believe there may have been some confusion related to the behavior in previous versions. In the latest revision, the `wi_node_notifier()` function that calls `sysfs_wi_node_add()` has been updated to always return `NOTIFY_OK`, regardless of the return value from `sysfs_wi_node_add()`. This ensures that no memory hotplug event will be blocked by our notifier logic. static int wi_node_notifier(struct notifier_block *nb, unsigned long action, void *data) { ... switch (action) { case MEM_ONLINE: err = sysfs_wi_node_add(nid); if (err) pr_err("failed to add sysfs for node%d during hotplug: %d\n", nid, err); break; ... return NOTIFY_OK; } Given that, it is appropriate for `sysfs_wi_node_add()` to return `-EEXIST` when the node already exists. Since the error message is already logged by `wi_node_notifier()`, I agree with Honggyu's suggestion to remove the redundant `pr_info()` statement as well: - pr_info("node%d already exists\n", nid); + ret = -EEXIST; Once again, thank you very much for your review and for helping us improve the code. Reviewed-by: Rakie Kim Rakie > regards, > dan carpenter > >