Your patch doesn't apply, seems like it uses spaces instead of tabs. Also I
can't use 'b4' to download it as there are multiple different patches using
the same message-id:
https://lore.kernel.org/all/[email protected]/
Re: subject, I don't see a bug that this would fix. You could say it's
"cleanup" and this function could use one, but for a cleanup it's not
improving the situation much.
On 3/23/23 18:30, Markus Elfring wrote:
> Date: Thu, 23 Mar 2023 18:18:59 +0100
>
> The label “err_out” was used to jump to another pointer check despite of
> the detail in the implementation of the function “shared_policy_replace”
> that it was determined already that a corresponding variable contained a
> null pointer because of a failed call of the function “kmem_cache_alloc”.
>
> 1. Use more appropriate labels instead.
>
> 2. The implementation of the function “mpol_put” contains a pointer check
> for its single input parameter.
> Thus delete a redundant check in the caller.
>
>
> This issue was detected by using the Coccinelle software.
>
> Fixes: 42288fe366c4f1ce7522bc9f27d0bc2a81c55264 ("mm: mempolicy: Convert shared_policy mutex to spinlock")
Again this is not a fix.
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> mm/mempolicy.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index a256a241fd1d..fb0485688dcb 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2736,13 +2736,12 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
> sp_insert(sp, new);
> write_unlock(&sp->lock);
> ret = 0;
> +put_mpol:
> + mpol_put(mpol_new);
>
> -err_out:
> - if (mpol_new)
> - mpol_put(mpol_new);
> if (n_new)
> kmem_cache_free(sn_cache, n_new);
> -
> +exit:
> return ret;
>
> alloc_new:
> @@ -2750,10 +2749,10 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
> ret = -ENOMEM;
> n_new = kmem_cache_alloc(sn_cache, GFP_KERNEL);
> if (!n_new)
> - goto err_out;
> + goto exit;
Just "return ret" and no need for exit label?
> mpol_new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
> if (!mpol_new)
> - goto err_out;
> + goto put_mpol;
We are doing this because mpol_new == NULL, so we know there's no reason to
do mpol_put(), we could jump to the freeing of n_new.
> atomic_set(&mpol_new->refcnt, 1);
> goto restart;
> }
> --
> 2.40.0
>
>
>