Re: [PATCH] mm: mempolicy: fix automatic numa balancing for shmem

[David Hildenbrand (Arm)]

On 6/29/26 18:33, Johannes Weiner wrote:
> Neha reports that mapped shmem aren't considered for NUMA balancing,
> noting convergence problems and bandwidth bottlenecking for cachelib
> based workloads on tiered memory systems.
> 
> Looking at the code and going through the git history, this doesn't
> actually seem intentional:
> 
> Commit fc3147245d19 ("mm: numa: Limit NUMA scanning to migrate-on-fault
> VMAs") added a vma_policy_mof() gate to task_numa_work() so VMAs whose
> policy lacks MPOL_F_MOF are skipped from NUMA balancing scans. The
> motivation was a real usecase: Oracle was pinning shared segments with
> mbind(MPOL_BIND) so trapping faults was both expensive and pointless.
> 
> The handling of NULL from vm_ops->get_policy, however, treated "user
> explicitly opted out" the same as "user never specified anything." For
> VMAs whose shared policy is absent - the common case for shmem - the
> scan was disabled too.
> 
> This issue is old. It probably hurts less in conventional NUMA. But it's
> very noticable on tiered systems, where entire tmpfs workingsets can get
> stuck on lower-bandwidth memory.

Sounds bad enough to warrant CC: stable?

> 
> Fix this by having vma_policy_mof() use __get_vma_policy() directly, and
> thereby handle the fallback to task policy (-> preferred_node_policy()
> has MPOL_F_MOF per default). Every other consumer of vm_ops->get_policy
> already handles it this way, the scan-eligibility check was the outlier.
> 
> This preserves Mel's intended fix: don't scan stuff the user explicitly
> pinned. But allow default policy vmas to participate in balancing.
> 
> Reported-by: Neha Gholkar <nehagholkar@xxxxxxxxx>
> Tested-by: Neha Gholkar <nehagholkar@xxxxxxxxx>
> Fixes: fc3147245d19 ("mm: numa: Limit NUMA scanning to migrate-on-fault VMAs")



> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> ---
>  mm/mempolicy.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 36699fabd3c2..bba65898aee1 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2057,24 +2057,15 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
>  bool vma_policy_mof(struct vm_area_struct *vma)
>  {
>  	struct mempolicy *pol;
> +	pgoff_t ilx;
> +	bool mof;
>  
> -	if (vma->vm_ops && vma->vm_ops->get_policy) {
> -		bool ret = false;
> -		pgoff_t ilx;		/* ignored here */
> -
> -		pol = vma->vm_ops->get_policy(vma, vma->vm_start, &ilx);
> -		if (pol && (pol->flags & MPOL_F_MOF))
> -			ret = true;
> -		mpol_cond_put(pol);
> -
> -		return ret;
> -	}

Okay, we used the fallback of vma->vm_policy before (if vma->vm_ops->get_policy
was not available), which is what __get_vma_policy() does at well.

But if vma->vm_ops->get_policy now returns NULL, we fallback to get_task_policy().


Makes sense to me although this is a source of confusion for me.

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David