| Message ID | 20230914235457.482710-2-gregory.price@memverge.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/mempolicy: set/get_mempolicy2 | expand |
On Thu, 14 Sep 2023 19:54:55 -0400 Gregory Price <gourry.memverge@gmail.com> wrote: > Refactors do_set_mempolicy into swap_mempolicy and do_set_mempolicy > so that swap_mempolicy can be re-used with set_mempolicy2. > > Signed-off-by: Gregory Price <gregory.price@memverge.com> Obviously this is an RFC, so you probably didn't give it the polish a finished patch might have. Still I was curious and reading it and I can't resist pointing out trivial stuff.. So.... > --- > mm/mempolicy.c | 44 +++++++++++++++++++++++++++++--------------- > 1 file changed, 29 insertions(+), 15 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 42b5567e3773..f49337f6f300 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -855,28 +855,21 @@ static int mbind_range(struct vma_iterator *vmi, struct vm_area_struct *vma, > return vma_replace_policy(vma, new_pol); > } > > -/* Set the process memory policy */ > -static long do_set_mempolicy(unsigned short mode, unsigned short flags, > - nodemask_t *nodes) > +/* Swap in a new mempolicy, release the old one if successful */ Not really swapping. More replacing given we don't get the old one back to do something else with it. > +static long swap_mempolicy(struct mempolicy *new, > + nodemask_t *nodes) Excessive wrapping. > { > - struct mempolicy *new, *old; > - NODEMASK_SCRATCH(scratch); > + struct mempolicy *old = NULL; > int ret; > + NODEMASK_SCRATCH(scratch); I'd avoid the reordering as makes it look like slightly more is happening in this change than is actually the case. > > if (!scratch) > return -ENOMEM; > > - new = mpol_new(mode, flags, nodes); > - if (IS_ERR(new)) { > - ret = PTR_ERR(new); > - goto out; > - } > - > task_lock(current); > ret = mpol_set_nodemask(new, nodes, scratch); > if (ret) { > task_unlock(current); > - mpol_put(new); > goto out; > } > > @@ -884,14 +877,35 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags, > current->mempolicy = new; > if (new && new->mode == MPOL_INTERLEAVE) > current->il_prev = MAX_NUMNODES-1; > - task_unlock(current); > - mpol_put(old); > - ret = 0; > out: > + task_unlock(current); > + if (old) > + mpol_put(old); It's protected against NULL parameter internally, so mpol_put(old); which has advantage that a block of diff will hopefully disappear making this patch easier to read. > + > NODEMASK_SCRATCH_FREE(scratch); > return ret; > } > > +/* Set the process memory policy */ > +static long do_set_mempolicy(unsigned short mode, unsigned short flags, > + nodemask_t *nodes) > +{ > + struct mempolicy *new; > + int ret; > + > + new = mpol_new(mode, flags, nodes); > + if (IS_ERR(new)) { > + ret = PTR_ERR(new); > + goto out; Given nothing to do at out lable, in keeping with at least some local style, you could do direct returns on errors. if (IS_ERR(new)) return PTR_ERR(new) ret = swap_mempolicy(new, nodes); if (ret) { mpol_put(new); return ret; } return 0; > + } > + > + ret = swap_mempolicy(new, nodes); > + if (ret) > + mpol_put(new); > +out: > + return ret; > +} > + > /* > * Return nodemask for policy for get_mempolicy() query > *
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 42b5567e3773..f49337f6f300 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -855,28 +855,21 @@ static int mbind_range(struct vma_iterator *vmi, struct vm_area_struct *vma, return vma_replace_policy(vma, new_pol); } -/* Set the process memory policy */ -static long do_set_mempolicy(unsigned short mode, unsigned short flags, - nodemask_t *nodes) +/* Swap in a new mempolicy, release the old one if successful */ +static long swap_mempolicy(struct mempolicy *new, + nodemask_t *nodes) { - struct mempolicy *new, *old; - NODEMASK_SCRATCH(scratch); + struct mempolicy *old = NULL; int ret; + NODEMASK_SCRATCH(scratch); if (!scratch) return -ENOMEM; - new = mpol_new(mode, flags, nodes); - if (IS_ERR(new)) { - ret = PTR_ERR(new); - goto out; - } - task_lock(current); ret = mpol_set_nodemask(new, nodes, scratch); if (ret) { task_unlock(current); - mpol_put(new); goto out; } @@ -884,14 +877,35 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags, current->mempolicy = new; if (new && new->mode == MPOL_INTERLEAVE) current->il_prev = MAX_NUMNODES-1; - task_unlock(current); - mpol_put(old); - ret = 0; out: + task_unlock(current); + if (old) + mpol_put(old); + NODEMASK_SCRATCH_FREE(scratch); return ret; } +/* Set the process memory policy */ +static long do_set_mempolicy(unsigned short mode, unsigned short flags, + nodemask_t *nodes) +{ + struct mempolicy *new; + int ret; + + new = mpol_new(mode, flags, nodes); + if (IS_ERR(new)) { + ret = PTR_ERR(new); + goto out; + } + + ret = swap_mempolicy(new, nodes); + if (ret) + mpol_put(new); +out: + return ret; +} + /* * Return nodemask for policy for get_mempolicy() query *
Refactors do_set_mempolicy into swap_mempolicy and do_set_mempolicy so that swap_mempolicy can be re-used with set_mempolicy2. Signed-off-by: Gregory Price <gregory.price@memverge.com> --- mm/mempolicy.c | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-)