Re: [PATCH 3/3] mm/numa_balancing:Allow migrate on protnone reference with MPOL_PREFERRED_MANY policy

[Donet Tom]

On 2/19/24 19:50, Michal Hocko wrote:
> On Sat 17-02-24 01:31:35, Donet Tom wrote:
> [...]
> > +static inline bool mpol_preferred_should_numa_migrate(int exec_node, int folio_node,
> > +					    struct mempolicy *pol)
> > +{
> > +	/* if the executing node is in the policy node mask, migrate */
> > +	if (node_isset(exec_node, pol->nodes))
> > +		return true;
> > +
> > +	/* If the folio node is in policy node mask, don't migrate */
> > +	if (node_isset(folio_node, pol->nodes))
> > +		return false;
> > +	/*
> > +	 * both the folio node and executing node are outside the policy nodemask,
> > +	 * migrate as normal numa fault migration.
> > +	 */
> > +	return true;
> > +}
> I have looked at this again and only now noticed that this doesn't
> really work as one would expected.
>
>          case MPOL_PREFERRED_MANY:
>                  /*
>                   * use current page if in policy nodemask,
>                   * else select nearest allowed node, if any.
>                   * If no allowed nodes, use current [!misplaced].
>                   */
>                  if (node_isset(curnid, pol->nodes))
>                          goto out;
>                  z = first_zones_zonelist(
>                                  node_zonelist(numa_node_id(), GFP_HIGHUSER),
>                                  gfp_zone(GFP_HIGHUSER),
>                                  &pol->nodes);
>                  polnid = zone_to_nid(z->zone);
>                  break;
>
> Will collapse the whole MPOL_PREFERRED_MANY nodemask into the first
> notde into that mask. Is that really what we want here? Shouldn't we use
> the full nodemask as the migration target?

With this patch it will take full nodemask and find out the correct migration target. It will not collapse into first node.

For example if we have 5 NUMA nodes in our system N1 to N5, all five are in nodemask and the execution node is N3.
with this fix mpol_preferred_should_numa_migrate() will return true because the execution node is there in the nodemask.
So mpol_misplaced() will select N3 as the migration target since MPOL_F_MORON is set and migrate the pages to N3.

    /* Migrate the folio towards the node whose CPU is referencing it */
    if (pol->flags & MPOL_F_MORON) {
        polnid = thisnid;

So with this patch pages will get migrated to the correct migration target.

>