Re: [PATCH] mm/page_alloc: fix NUMA stats update for cpu-less nodes

[Michal Hocko <mhocko@suse.com>]

On Wed 23-10-24 21:54:37, Dongjoo Seo wrote:
> On Thu, Oct 24, 2024 at 12:23:56AM +0200, Michal Hocko wrote:
> > On Wed 23-10-24 15:15:20, Dongjoo Seo wrote:
> > > Hi Andrew, Michal,
> > > 
> > > Thanks for the feedback.
> > > 
> > > The issue is that CPU-less nodes can lead to incorrect NUMA stats.
> > > For example, NUMA_HIT may incorrectly increase for CPU-less nodes
> > > because the current logic doesn't account for whether a node has CPUs.
> > 
> > Define incorrect
> > 
> > Current semantic doesn't really care about cpu less NUMA nodes because
> > current means whatever is required AFIU. This is certainly a long term
> 
> I agree that, in the long term, special logging for preferred_zone 
> and a separate counter might be necessary for CPU-less nodes.
> 
> > semantic. Why does this need to change and why it makes sense to 
> > pre-existing users?
> 
> This patch doesn't change existing logic; the additional logic only 
> applies when a CPU-less node is present, so there shouldn't be 
> concerns for pre-existing users. Currently, the NUMA stats for 
> configurations with CPU-less nodes are incorrect, as allocations
> are not properly accounted for.
> 
> I believe this approach improves logging accuracy with minimal impact
> on the memory allocation path, but I'm open to alternative solutions.
> This isn't the only way to address the issue—any suggestions?

I still do not understand the actual problem. CPU-less nodes are nothing
really special. They just never have local allocations for obvious
reasons. NUMA_HIT which your patch is special casing has a very well
defined meaning and that is that the memory allocated matches the
preferred node. That doesn't have any notion of CPU at all. Say somebody
explicitly requests to allocate from a CPU less node. Why should you
consider thiat as NUMA_OTHER just because that node has no CPUs? That
just seems completely wrong.
-- 
Michal Hocko
SUSE Labs