linux-kernel - Re: [PATCH v4 12/22] lib/stackdepot: use read/write lock

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]

Date: Thu, 11 Jan 2024 10:48:19 +0100
From: Marco Elver <elver@...gle.com>
To: Andi Kleen <ak@...ux.intel.com>
Cc: Oscar Salvador <osalvador@...e.de>, andrey.konovalov@...ux.dev, 
	Andrew Morton <akpm@...ux-foundation.org>, Andrey Konovalov <andreyknvl@...il.com>, 
	Alexander Potapenko <glider@...gle.com>, Dmitry Vyukov <dvyukov@...gle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	kasan-dev@...glegroups.com, Evgenii Stepanov <eugenis@...gle.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH v4 12/22] lib/stackdepot: use read/write lock

On Thu, 11 Jan 2024 at 00:01, Andi Kleen <ak@...ux.intel.com> wrote:
>
> Oscar Salvador <osalvador@...e.de> writes:
> >>
> >> With this change, multiple users can still look up records in parallel.
>
> That's a severe misunderstanding -- rwlocks always bounce a cache line,
> so the parallelism is significantly reduced.
>
> Normally rwlocks are only worth it if your critical region is quite long.
>
> >>
> >> This is preparatory patch for implementing the eviction of stack records
> >> from the stack depot.
> >>
> >> Reviewed-by: Alexander Potapenko <glider@...gle.com>
> >> Signed-off-by: Andrey Konovalov <andreyknvl@...gle.com>
> >
> > Reviewed-by: Oscar Salvador <osalvador@...e.de>
>
>
> Has anyone benchmarked this on a high core count machine? It sounds
> pretty bad if every lock aquisition starts bouncing a single cache line.
>
> Consider using RCU or similar.

stackdepot is severely limited in what kernel facilities it may use
due to being used by such low level facilities as the allocator
itself.

I've been suggesting percpu-rwsem here, but looking at it in more
detail that doesn't work because percpu-rwsem wants to sleep, but
stackdepot must work in non-sleepable contexts. :-/