On Wed 23-10-24 13:41:21, Andrew Morton wrote:
> On Wed, 23 Oct 2024 20:03:24 +0200 Michal Hocko <mhocko@xxxxxxxx> wrote:
>
> > On Wed 23-10-24 10:50:37, Dongjoo Seo wrote:
> > > This patch corrects this issue by:
> >
> > What is this issue? Please describe the problem first,
>
> Actually, relocating the author's second-last paragraph to
> top-of-changelog produced a decent result ;)
>
> > ideally describe
> > the NUMA topology, workload and what kind of misaccounting happens
> > (expected values vs. really reported values).
>
> I think the changelog covered this adequately?
>
> So with these changelog alterations I've queued this for 6.12-rcX with
> a cc:stable. As far as I can tell this has been there since 2018.
>
> : In the case of memoryless node, when a process prefers a node with no
> : memory(e.g., because it is running on a CPU local to that node), the
> : kernel treats a nearby node with memory as the preferred node. As a
> : result, such allocations do not increment the numa_foreign counter on the
> : memoryless node, leading to skewed NUMA_HIT, NUMA_MISS, and NUMA_FOREIGN
> : stats for the nearest node.
I am sorry but I still do not underastand that. Especially when I do
look at the patch which would like to treat cpuless nodes specially.
Let me be more specific. Why ...
> - if (zone_to_nid(z) != numa_node_id())
> + if (zone_to_nid(z) != numa_node_id() || z_is_cpuless)
> local_stat = NUMA_OTHER;
>
> - if (zone_to_nid(z) == zone_to_nid(preferred_zone))
> + if (zone_to_nid(z) == zone_to_nid(preferred_zone) && !z_is_cpuless)
> __count_numa_events(z, NUMA_HIT, nr_account);
> else {
> __count_numa_events(z, NUMA_MISS, nr_account);
> - __count_numa_events(preferred_zone, NUMA_FOREIGN, nr_account);
> + if (!pref_is_cpuless)
> + __count_numa_events(preferred_zone, NUMA_FOREIGN, nr_account);
... a (well?) established meaning of local needs to be changed? Why
prefrerred policy should have a different meaning for cpuless policies?
Those are memory specific rather than cpu specific right?
Quite some quiestions to have it in linux-next IMHO....
--
Michal Hocko
SUSE Labs
