Re: [PATCH next] mm/mempolicy: Fix error code in sysfs_wi_node_add()

[Joshua Hahn <joshua.hahnjy@gmail.com>]

On Wed, 23 Apr 2025 11:24:58 +0300 Dan Carpenter <dan.carpenter@linaro.org> wrote:

Hi Dan,

This makes sense to me! I think that the only way the node can already exist
is if an offlining node didn't properly clean up its sysfs entry, which is
a bug, of course. With that said, I don't think the previous state would have
caused any functional problems, since the same node offlining and onlining
should share the same sysfs entry anyways (unless I'm overlooking something
important...)

This fix will help when the cleanup does fail though, and I think that will
help us assess whether a failed cleanup does indeed cause other problems.

Thank you for spotting this & fixing it! I hope you have a great day : -)

Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>

> 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 <dan.carpenter@linaro.org>
> ---
> 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;
>  		goto out;
>  	}
>  
> -- 
> 2.47.2