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/6e9ca062-939b-af96-c8ff-56ad485d6e79@xxxxxx/ 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 <elfring@xxxxxxxxxxxxxxxxxxxxx> > --- > 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 > > >
