| Message ID | 1621499404-67756-2-git-send-email-feng.tang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/mempolicy: some fix and semantics cleanup | expand |
On Thu, 20 May 2021, Feng Tang wrote: > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index d79fa29..1964cca 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -2098,7 +2098,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask) > * > * If tsk's mempolicy is "default" [NULL], return 'true' to indicate default > * policy. Otherwise, check for intersection between mask and the policy > - * nodemask for 'bind' or 'interleave' policy. For 'preferred' or 'local' > + * nodemask for 'bind' policy. For 'interleave', 'preferred' or 'local' > * policy, always return true since it may allocate elsewhere on fallback. > * > * Takes task_lock(tsk) to prevent freeing of its mempolicy. > @@ -2111,29 +2111,13 @@ bool mempolicy_nodemask_intersects(struct task_struct *tsk, > > if (!mask) > return ret; > + > task_lock(tsk); > mempolicy = tsk->mempolicy; > - if (!mempolicy) > - goto out; > - > - switch (mempolicy->mode) { > - case MPOL_PREFERRED: > - /* > - * MPOL_PREFERRED and MPOL_F_LOCAL are only preferred nodes to > - * allocate from, they may fallback to other nodes when oom. > - * Thus, it's possible for tsk to have allocated memory from > - * nodes in mask. > - */ > - break; > - case MPOL_BIND: > - case MPOL_INTERLEAVE: > + if (mempolicy && mempolicy->mode == MPOL_BIND) > ret = nodes_intersects(mempolicy->v.nodes, *mask); If MPOL_INTERLEAVE is deemed only a suggestion, the same could be considered true of MPOL_BIND intersection as well, no? > - break; > - default: > - BUG(); > - } > -out: > task_unlock(tsk); > + > return ret; > } > > -- > 2.7.4 > >
Hi David, Thanks for the review! On Sun, May 23, 2021 at 10:15:00PM -0700, David Rientjes wrote: > On Thu, 20 May 2021, Feng Tang wrote: > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index d79fa29..1964cca 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -2098,7 +2098,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask) > > * > > * If tsk's mempolicy is "default" [NULL], return 'true' to indicate default > > * policy. Otherwise, check for intersection between mask and the policy > > - * nodemask for 'bind' or 'interleave' policy. For 'preferred' or 'local' > > + * nodemask for 'bind' policy. For 'interleave', 'preferred' or 'local' > > * policy, always return true since it may allocate elsewhere on fallback. > > * > > * Takes task_lock(tsk) to prevent freeing of its mempolicy. > > @@ -2111,29 +2111,13 @@ bool mempolicy_nodemask_intersects(struct task_struct *tsk, > > > > if (!mask) > > return ret; > > + > > task_lock(tsk); > > mempolicy = tsk->mempolicy; > > - if (!mempolicy) > > - goto out; > > - > > - switch (mempolicy->mode) { > > - case MPOL_PREFERRED: > > - /* > > - * MPOL_PREFERRED and MPOL_F_LOCAL are only preferred nodes to > > - * allocate from, they may fallback to other nodes when oom. > > - * Thus, it's possible for tsk to have allocated memory from > > - * nodes in mask. > > - */ > > - break; > > - case MPOL_BIND: > > - case MPOL_INTERLEAVE: > > + if (mempolicy && mempolicy->mode == MPOL_BIND) > > ret = nodes_intersects(mempolicy->v.nodes, *mask); > > If MPOL_INTERLEAVE is deemed only a suggestion, the same could be > considered true of MPOL_BIND intersection as well, no? IIUC, 'bind' and 'interleave' are different regarding memory allocation. In alloc_pages_vma(), there are: nmask = policy_nodemask(gfp, pol); preferred_nid = policy_node(gfp, pol, node); page = __alloc_pages(gfp, order, preferred_nid, nmask); mpol_cond_put(pol); and in plicy_nodemask(), only 'bind' policy may return its desired nodemask, while all other returns NULL including 'interleave'. And this 'NULL' enables the 'interleave' policy can get memory from other nodes than its nodemask. So when allocating memory, 'interleave' can get memory from all nodes. I did some experements which also confirm this. Thanks, Feng > > > - break; > > - default: > > - BUG(); > > - } > > -out: > > task_unlock(tsk); > > + > > return ret; > > } > > > > -- > > 2.7.4 > > > >
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index d79fa29..1964cca 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -2098,7 +2098,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask) * * If tsk's mempolicy is "default" [NULL], return 'true' to indicate default * policy. Otherwise, check for intersection between mask and the policy - * nodemask for 'bind' or 'interleave' policy. For 'preferred' or 'local' + * nodemask for 'bind' policy. For 'interleave', 'preferred' or 'local' * policy, always return true since it may allocate elsewhere on fallback. * * Takes task_lock(tsk) to prevent freeing of its mempolicy. @@ -2111,29 +2111,13 @@ bool mempolicy_nodemask_intersects(struct task_struct *tsk, if (!mask) return ret; + task_lock(tsk); mempolicy = tsk->mempolicy; - if (!mempolicy) - goto out; - - switch (mempolicy->mode) { - case MPOL_PREFERRED: - /* - * MPOL_PREFERRED and MPOL_F_LOCAL are only preferred nodes to - * allocate from, they may fallback to other nodes when oom. - * Thus, it's possible for tsk to have allocated memory from - * nodes in mask. - */ - break; - case MPOL_BIND: - case MPOL_INTERLEAVE: + if (mempolicy && mempolicy->mode == MPOL_BIND) ret = nodes_intersects(mempolicy->v.nodes, *mask); - break; - default: - BUG(); - } -out: task_unlock(tsk); + return ret; }
mempolicy_nodemask_intersects() is used in oom case to check if a task may have memory allocated on some memory nodes. MPOL_INTERLEAVE has set a nodemask, which is not a forced requirement, but just a hint for chosing a node to allocate memory from, and the task may have memory allocated on other nodes than this nodemask. So skip the check. Suggested-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Feng Tang <feng.tang@intel.com> --- mm/mempolicy.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)