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 8F9A8C369C2 for ; Thu, 24 Apr 2025 05:49:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A65E96B0006; Thu, 24 Apr 2025 01:49:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A15016B0007; Thu, 24 Apr 2025 01:49:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DBC36B0008; Thu, 24 Apr 2025 01:49:52 -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 6F72F6B0006 for ; Thu, 24 Apr 2025 01:49:52 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 0D4831402C4 for ; Thu, 24 Apr 2025 05:49:52 +0000 (UTC) X-FDA: 83367861024.30.FF4AE5A Received: from invmail4.hynix.com (exvmail4.hynix.com [166.125.252.92]) by imf09.hostedemail.com (Postfix) with ESMTP id 670A0140002 for ; Thu, 24 Apr 2025 05:49:49 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf09.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745473790; a=rsa-sha256; cv=none; b=nfKtZDZ7c7wG6YjfL2tM00OJIX0Ya2paDmIYTYwcOyipf5IhI5aG+wsgE54HUc7TMk+F6Z 2xmRjFFSFLJOsfsurDkOVG2eWzykf9zl8SqTD2ZQPW/VZTBkFX5MBYuepkw22Fe745wBkO hZ1Ikr3dHJOsjjlhspsHGtUhCp6OPvk= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf09.hostedemail.com: domain of rakie.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=rakie.kim@sk.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1745473790; 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=gQtjo4MYDS9bz1iDPqyDdYmG5QtySqrvibdZ/am+gc8=; b=gI3qVuWqCOKG41GQEgExzsxfb9Ebivd569lR0SIMjNCVHPkKmU/rYWxjpkPg8ALu6Dk0z5 w6vsUAzpOhCQO0O7LfLP8VfAJAaOHVL7aigOc9kXpC6xfdkN3GLQoI+3NAKO8URIQeTnAr DdDaCfgUXv3GFRzB6hSIJRUJBeSVWMo= X-AuditID: a67dfc5b-681ff7000002311f-b5-6809d0fac5b7 From: Rakie Kim To: Dan Carpenter Cc: Rakie Kim , Gregory Price , Andrew Morton , Honggyu Kim , Dan Williams , Joshua Hahn , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, kernel_team@skhynix.com Subject: Re: [PATCH next] mm/mempolicy: Fix error code in sysfs_wi_node_add() Date: Thu, 24 Apr 2025 14:49:36 +0900 Message-ID: <20250424054942.120-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+NgFjrLLMWRmVeSWpSXmKPExsXC9ZZnoe6vC5wZBmdnWlnMWb+GzeLDvFZ2 i+lTLzBa/Lx7nN3i+NZ57BZbb0lbXN41h83i3pr/rA4cHjtn3WX36G67zO6xeM9LJo9Nnyax e9y5tofN48SM3ywenzfJBbBHcdmkpOZklqUW6dslcGX0nJrDVHBRpqLj3xLmBsblYl2MnBwS AiYSz04sYYGxf3/4xtbFyMHBJqAkcWxvDEhYREBH4t/fyUAlXBzMAveYJD5N/sYKkhAW8JVY /B/CZhFQlTjQMwFsDq+AscTaa9/YIWZqSjRcuscEMpNTwEziypkikLCQAI/Eqw37GSHKBSVO znwC1sosIC/RvHU2M8guCYEjbBL9m5qgbpOUOLjiBssERv5ZSHpmIelZwMi0ilEoM68sNzEz x0QvozIvs0IvOT93EyMwkJfV/onewfjpQvAhRgEORiUeXo+7HBlCrIllxZW5hxglOJiVRHh/ ubFnCPGmJFZWpRblxxeV5qQWH2KU5mBREuc1+laeIiSQnliSmp2aWpBaBJNl4uCUamDU2XG4 9dU8D4+kE2+fiMX+PrK4k8fozdmLi1OWdyWfMLx4VdlZYX2kXfXLtTXMKWz1uXZSlekftHMl pZY4ng51uP56rVj4l6xl1y+sui94+0Rv/Ocr3Pu/31r8Q08m6MCze42fw1et15Tar/ZSZwHn tBpOlUkfLDxM/z65GMhy5HvSxO9npxXyKbEUZyQaajEXFScCAHQKOGFgAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrELMWRmVeSWpSXmKPExsXCNUNNS/fXBc4Mg4WzdSzmrF/DZvFhXiu7 xfSpFxgtft49zm7x+dlrZovjW+exW2y9JW1xeO5JVovLu+awWdxb85/V4tC156wO3B47Z91l 9+huu8zusXjPSyaPTZ8msXvcubaHzePEjN8sHt9ue3gsfvGByePzJrkAzigum5TUnMyy1CJ9 uwSujJ5Tc5gKLspUdPxbwtzAuFysi5GTQ0LAROL3h29sXYwcHGwCShLH9saAhEUEdCT+/Z3M 0sXIxcEscI9J4tPkb6wgCWEBX4nF/yFsFgFViQM9E1hAbF4BY4m1176xQ8zUlGi4dI8JZCan gJnElTNFIGEhAR6JVxv2M0KUC0qcnPkErJVZQF6ieets5gmMPLOQpGYhSS1gZFrFKJKZV5ab mJljqlecnVGZl1mhl5yfu4kRGLLLav9M3MH45bL7IUYBDkYlHt6AJxwZQqyJZcWVuYcYJTiY lUR4f7mxZwjxpiRWVqUW5ccXleakFh9ilOZgURLn9QpPTRASSE8sSc1OTS1ILYLJMnFwSjUw hjw1Maue+a36s6xElYGFvVfvNus2kYk6Nivl3Xar2f5uO7pelnn9Fnd9s5bVP3lPuUk3r97h IuKbs/H4xVPMgdzHtu4/yCTsMT+pJSmlcUrP2S1zwtT+s62zO3PV/x1r3rMupyvPQievnfLQ vpY/4+FsZq+HG0NaeP0XnNkUPeE957cY41wjJZbijERDLeai4kQApDncKlUCAAA= X-CFilter-Loop: Reflected X-Rspam-User: X-Rspamd-Queue-Id: 670A0140002 X-Rspamd-Server: rspam04 X-Stat-Signature: xj73qefkwpou6n7q7nekebdbkip1pxgb X-HE-Tag: 1745473789-624081 X-HE-Meta: U2FsdGVkX18N7eiCiHDNpGYwL8340+Sgq28S6EKAn1vhZ/aDYkwikHY/o4bYW2tR0eaNqS2Ak8BtgfS2vyC+iIisuwIIxrNAwN/Nr1oM1Irt6oTKMV9nxnUPLjtk0uEVzLW0t9FmHSDeBYQy6/W/EUevC70wEOjXW6e95zPAfhgUHV6K3HVL9EAUqUNhsf8NPKNzHJC8xfbi5Ij5n7r8YemN5Gn/kGwM439zJ4Irql54juqmTJZ14GsTu78W9xNO7uW1FJxHfJEvHyN6aEkptIyy/0v5G+M6tbhq4SlxKnTH5RWgV3VWEY2jgjQ5/sOzw1pT3jNMtl+feW4XIhqUdfzAJxFuTIac8HdJWMHJu0uOaLs6orFJQEjr85MuOK+PP+d3L4qp987DOYd5MYVajcUzo9gAnqh/FZQ9QbpgA4Zm/EXFiQ0YFqkhTK7+x8gujocWyzgo6K9JP9Gge+frCrr4PrHWXD+zFROsfzO6XSxQ0nJUTjhHYko5K7XzDiAFS5yKhMbzym8MJkES9cNfhjWNSGjLayGM9F1/qq39r3AJJtjFY+qn9BrBdBCqu33aTJ0RMZkLgC0SEeBzhNTpGrIf5aWsAAX6rbDqtwu6qUpdkh7QYfAAilSIuuPX2VhkRTbjkHIt8YuMq+GNjEhGBfK/p9WgqSkqLdbyIjuBkFAFKbThByuBXMPWCADjQE1xRNoe0672CbksFOzpVUHiSd6C7Tap3oIwWcRL4aB0hAf1NMHx/jAKT8iL/pyc/o99P9qWXgIxloT9DDRhBrN+C7+/FoP3dwWBPZJ2TsdfCsQx75tW61vsLLT2jY57oQema74siIJy83hUFLozhXTWsPxG9HLTEVnUlvuwJp+EpufEUuoUAB/fZ1aBNUIl/4TdJB+PTwWjk/V7f7p5fGnW1+ofDSm3vqc0OX9fq+Xcw+e3XLjUK72x6gmZK74eWA2jXLQHq9UxzPcqTRYFRlR nx8E2/tM jnnGYysQ0D/H0DsfNt3p/H5LPvj6w8OqxWOGMOTZjzG+JlZS3Gqib2gIEnR0Yge4TBDc6IUxEhAhjA1NaKsqlHyF0yCaw5imG5t03uVbA8NoiQEnJAz8Wn6jxGIY328M43kMNgEvvBy5019vBkshFIK2AOJBwOBoN49qUlP/zINkF+6TSZPFRHekmOt2fvcLqRwE/7KCfgABfvjY= 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 Wed, 23 Apr 2025 12:33:50 -0400 Gregory Price wrote: > On Wed, Apr 23, 2025 at 11:24:58AM +0300, 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. > > I think this was intentional to allow hotplug callbacks to continue > executing. I will let the SK folks who wrote the patch confirm/deny. > > If it is intentional, then we need to add a comment here to explain. > > ~Gregory > > > > > 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; > > goto out; > > } > > > > -- > > 2.47.2 > > > Hi Dan, Thank you very much for analyzing the issue in this code and for sharing a detailed patch to address it. Your review is greatly appreciated. However, the current behavior of returning success instead of an -EEXIST or other error code was intentional. I would like to explain the rationale for this choice and would appreciate your further thoughts. The condition: if (wi_group->nattrs[nid]) { mutex_unlock(&wi_group->kobj_lock); pr_info("node%d already exists\n", nid); goto out; } is triggered in the following two cases: 1. If `sysfs_wi_node_delete()` fails: - This function only performs `sysfs_remove_file()` and frees memory, and these operations do not fail in a way that would leave the system in an inconsistent state. 2. If `sysfs_wi_node_add()` is invoked multiple times for the same node: - While repeated additions for the same node would indicate a potential issue in logic, simply skipping the redundant addition does not cause a functional problem. The original sysfs entry remains valid and continues to work as expected. Therefore, I chose to return success in this case to avoid interrupting the flow unnecessarily. Also, as you pointed out, even if we returned -EEXIST here, it would not change the runtime behavior. This is because `sysfs_wi_node_add()` is called from the following memory notifier: 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; } As discussed in prior reviews (including suggestions by David Hildenbrand), returning NOTIFY_BAD on failure can interfere with other notifier chains due to NOTIFY_STOP_MASK behavior. Hence, we always return NOTIFY_OK to preserve consistent hotplug handling across subsystems. I would sincerely appreciate it if you could share any further thoughts or concerns you may have regarding this decision. Rakie