linux-kernel - Re: [PATCH] mm, slub: print CPU id on slab OOM

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] [day] [month] [year] [list]

Message-ID: <CAJHvVcgP_yOc7rgyEsyC2u+h0XLLCRAUp4Fd0nAX2fJ2KTvL9g@mail.gmail.com>
Date: Mon, 12 Aug 2024 15:45:44 -0700
From: Axel Rasmussen <axelrasmussen@...gle.com>
To: David Rientjes <rientjes@...gle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>, 
	Christoph Lameter <cl@...ux.com>, Hyeonggon Yoo <42.hyeyoo@...il.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, 
	Pekka Enberg <penberg@...nel.org>, Roman Gushchin <roman.gushchin@...ux.dev>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm, slub: print CPU id on slab OOM

On Sun, Aug 11, 2024 at 1:21 PM David Rientjes <rientjes@...gle.com> wrote:
>
> On Sun, 11 Aug 2024, Vlastimil Babka wrote:
>
> > > diff --git a/mm/slub.c b/mm/slub.c
> > > index c9d8a2497fd6..7148047998de 100644
> > > --- a/mm/slub.c
> > > +++ b/mm/slub.c
> > > @@ -3422,7 +3422,8 @@ slab_out_of_memory(struct kmem_cache *s, gfp_t gfpflags, int nid)
> > >     if ((gfpflags & __GFP_NOWARN) || !__ratelimit(&slub_oom_rs))
> > >             return;
> > >
> > > -   pr_warn("SLUB: Unable to allocate memory on node %d, gfp=%#x(%pGg)\n",
> > > +   pr_warn("SLUB: Unable to allocate memory for CPU %u on node %d, gfp=%#x(%pGg)\n",
> >
> > BTW, wouldn't "on CPU" be more correct, as "for CPU" might be misleading
> > that we are somehow constrained to that CPU?
> >
>
> Agreed.

No objection to the rewording.

>
> When I suggested this patch, I was trying to ascertain whether something
> was really wonky based on some logs that we were seeing.
>
>   node 0: slabs: 223, objs: 11819, free: 0
>   node 1: slabs: 951, objs: 50262, free: 218
>
> This is for a NUMA_NO_NODE allocation, so I wanted to know if the cpu was
> on node 0 or node 1.
>
> Even with the patch, that requires knowing the cpu-to-node mapping.  If we
> add the CPU output here, we likely also want to print out cpu_to_node().

Seems reasonable. Of course we could always look it up separately, but
it would be convenient to just print it directly. I can send a v2 to
add this.

>
> > > +           preemptible() ? raw_smp_processor_id() : smp_processor_id(),
> >
> > Also could we just use raw_smp_processor_id() always here? I don't see
> > this has any advantage or am I missing something?
> >
>
> This matches my understanding as well.

That's fair, in any case it seems to matter very little for this use
case whether the read is "stable" or not. Better to keep it simple. I
can send a v2 with this tweak too.