Re: [PATCH v6 1/6] mm/mempolicy: Add MPOL_PREFERRED_MANY for multiple preferred nodes
From: Michal Hocko <mhocko@suse.com>
Date: 2021-07-28 12:31:08
Also in:
linux-mm, lkml
[Sorry for a late review] On Mon 12-07-21 16:09:29, Feng Tang wrote: [...]
quoted hunk ↗ jump to hunk
@@ -1887,7 +1909,8 @@ nodemask_t *policy_nodemask(gfp_t gfp, struct mempolicy *policy) /* Return the node id preferred by the given mempolicy, or the given id */ static int policy_node(gfp_t gfp, struct mempolicy *policy, int nd) { - if (policy->mode == MPOL_PREFERRED) { + if (policy->mode == MPOL_PREFERRED || + policy->mode == MPOL_PREFERRED_MANY) { nd = first_node(policy->nodes); } else { /*
Do we really want to have the preferred node to be always the first node in the node mask? Shouldn't that strive for a locality as well? Existing callers already prefer numa_node_id() - aka local node - and I belive we shouldn't just throw that away here.
quoted hunk ↗ jump to hunk
@@ -1931,6 +1954,7 @@ unsigned int mempolicy_slab_node(void) switch (policy->mode) { case MPOL_PREFERRED: + case MPOL_PREFERRED_MANY: return first_node(policy->nodes);
Similarly here but I am not really familiar with the slab numa code enough to have strong opinions here.
quoted hunk ↗ jump to hunk
@@ -2173,10 +2198,12 @@ struct page *alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma, * node and don't fall back to other nodes, as the cost of * remote accesses would likely offset THP benefits. * - * If the policy is interleave, or does not allow the current - * node in its nodemask, we allocate the standard way. + * If the policy is interleave or multiple preferred nodes, or + * does not allow the current node in its nodemask, we allocate + * the standard way. */ - if (pol->mode == MPOL_PREFERRED) + if ((pol->mode == MPOL_PREFERRED || + pol->mode == MPOL_PREFERRED_MANY)) hpage_node = first_node(pol->nodes);
Same here.
quoted hunk ↗ jump to hunk
@@ -2451,6 +2479,9 @@ int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long break; case MPOL_PREFERRED: + case MPOL_PREFERRED_MANY: + if (node_isset(curnid, pol->nodes)) + goto out; polnid = first_node(pol->nodes); break;
I do not follow what is the point of using first_node here. Either the node is in the mask or it is misplaced. What are you trying to achieve here? -- Michal Hocko SUSE Labs