Re: [PATCH -V2 2/2] autonuma: Migrate on fault among multiple bound nodes

[Mel Gorman]

On Wed, Oct 28, 2020 at 10:34:11AM +0800, Huang Ying wrote:
> Now, AutoNUMA can only optimize the page placement among the NUMA nodes if the
> default memory policy is used.  Because the memory policy specified explicitly
> should take precedence.  But this seems too strict in some situations.  For
> example, on a system with 4 NUMA nodes, if the memory of an application is bound
> to the node 0 and 1, AutoNUMA can potentially migrate the pages between the node
> 0 and 1 to reduce cross-node accessing without breaking the explicit memory
> binding policy.
> 
> So in this patch, if mbind(.mode=MPOL_BIND, .flags=MPOL_MF_LAZY) is used to bind
> the memory of the application to multiple nodes, and in the hint page fault
> handler both the faulting page node and the accessing node are in the policy
> nodemask, the page will be tried to be migrated to the accessing node to reduce
> the cross-node accessing.
> 
> [Peter Zijlstra: provided the simplified implementation method.]
> 
> Questions:
> 
> Sysctl knob kernel.numa_balancing can enable/disable AutoNUMA optimizing
> globally.  But for the memory areas that are bound to multiple NUMA nodes, even
> if the AutoNUMA is enabled globally via the sysctl knob, we still need to enable
> AutoNUMA again with a special flag.  Why not just optimize the page placement if
> possible as long as AutoNUMA is enabled globally?  The interface would look
> simpler with that.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>

I've no specific objection to the patch or the name change. I can't
remember exactly why I picked the name, it was 8 years ago but I think it
was because the policy represented the most basic possible approach that
could be done without any attempt at being intelligent and established
a baseline. The intent was that anything built on top had to be better
than the most basic policy imaginable. The name reflected the dictionary
definition at the time and happened to match the acronym closely enough
and I wanted to make it absolutely clear to reviewers that the policy
was not good enough (ruling out MPOL_BASIC or variants thereof) even if
it happened to work for some workload and there was no intent to report
it to the userspace API.

The only hazard with the patch is that applications that use MPOL_BIND
on multiple nodes may now incur some NUMA balancing overhead due to
trapping faults and migrations. It might still end up being better but
I was not aware of a *realistic* workload that binds to multiple nodes
deliberately. Generally I expect if an application is binding, it's
binding to one local node.

If it shows up in regressions, it'll be interesting to get a detailed
description of the workload. Pay particular attention to if THP is
disabled as I learned relatively recently that NUMA balancing with THP
disabled has higher overhead (which is hardly surprising).

Lacking data or a specific objection

Acked-by: Mel Gorman <mgorman@xxxxxxx>

-- 
Mel Gorman
SUSE Labs