lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZxoEWBakAv64wfhD@tiehlicka>
Date: Thu, 24 Oct 2024 10:24:56 +0200
From: Michal Hocko <mhocko@...e.com>
To: Dongjoo Seo <dongjoo.linux.dev@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, dave@...olabs.net,
	dan.j.williams@...el.com, nifan@...look.com,
	a.manzanares@...sung.com
Subject: Re: [PATCH] mm/page_alloc: fix NUMA stats update for cpu-less nodes

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ