linux-kernel - Re: [PATCH v2 0/5] Move kvfree

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: <55931fdd-1d5f-4ffd-8496-fe436171dee2@suse.cz>
Date: Mon, 20 Jan 2025 23:06:13 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: paulmck@...nel.org, Uladzislau Rezki <urezki@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 RCU <rcu@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
 Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
 David Rientjes <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>,
 Roman Gushchin <roman.gushchin@...ux.dev>,
 Hyeonggon Yoo <42.hyeyoo@...il.com>,
 Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>
Subject: Re: [PATCH v2 0/5] Move kvfree_rcu() into SLAB (v2)

On 12/16/24 17:46, Paul E. McKenney wrote:
> On Mon, Dec 16, 2024 at 04:55:06PM +0100, Uladzislau Rezki wrote:
>> On Mon, Dec 16, 2024 at 04:44:41PM +0100, Vlastimil Babka wrote:
>> > On 12/16/24 16:41, Uladzislau Rezki wrote:
>> > > On Mon, Dec 16, 2024 at 03:20:44PM +0100, Vlastimil Babka wrote:
>> > >> On 12/16/24 12:03, Uladzislau Rezki wrote:
>> > >> > On Sun, Dec 15, 2024 at 06:30:02PM +0100, Vlastimil Babka wrote:
>> > >> > 
>> > >> >> Also how about a followup patch moving the rcu-tiny implementation of
>> > >> >> kvfree_call_rcu()?
>> > >> >> 
>> > >> > As, Paul already noted, it would make sense. Or just remove a tiny
>> > >> > implementation.
>> > >> 
>> > >> AFAICS tiny rcu is for !SMP systems. Do they benefit from the "full"
>> > >> implementation with all the batching etc or would that be unnecessary overhead?
>> > >> 
>> > > Yes, it is for a really small systems with low amount of memory. I see
>> > > only one overhead it is about driving objects in pages. For a small
>> > > system it can be critical because we allocate.
>> > > 
>> > > From the other hand, for a tiny variant we can modify the normal variant
>> > > by bypassing batching logic, thus do not consume memory(for Tiny case)
>> > > i.e. merge it to a normal kvfree_rcu() path.
>> > 
>> > Maybe we could change it to use CONFIG_SLUB_TINY as that has similar use
>> > case (less memory usage on low memory system, tradeoff for worse performance).
>> > 
>> Yep, i also was thinking about that without saying it :)
> 
> Works for me as well!

Hi, so I tried looking at this. First I just moved the code to slab as seen
in the top-most commit here [1]. Hope the non-inlined __kvfree_call_rcu() is
not a show-stopper here.

Then I wanted to switch the #ifdefs from CONFIG_TINY_RCU to CONFIG_SLUB_TINY
to control whether we use the full blown batching implementation or the
simple call_rcu() implmentation, and realized it's not straightforward and
reveals there are still some subtle dependencies of kvfree_rcu() on RCU
internals :)

Problem 1: !CONFIG_SLUB_TINY with CONFIG_TINY_RCU

AFAICS the batching implementation includes kfree_rcu_scheduler_running()
which is called from rcu_set_runtime_mode() but only on TREE_RCU. Perhaps
there are other facilities the batching implementation needs that only
exists in the TREE_RCU implementation

Possible solution: batching implementation depends on both !CONFIG_SLUB_TINY
and !CONFIG_TINY_RCU. I think it makes sense as both !SMP systems and small
memory systems are fine with the simple implementation.

Problem 2: CONFIG_TREE_RCU with !CONFIG_SLUB_TINY

AFAICS I can't just make the simple implementation do call_rcu() on
CONFIG_TREE_RCU, because call_rcu() no longer knows how to handle the fake
callback (__is_kvfree_rcu_offset()) - I see how rcu_reclaim_tiny() does that
but no such equivalent exists in TREE_RCU. Am I right?

Possible solution: teach TREE_RCU callback invocation to handle
__is_kvfree_rcu_offset() again, perhaps hide that branch behind #ifndef
CONFIG_SLUB_TINY to avoid overhead if the batching implementation is used.
Downside: we visibly demonstrate how kvfree_rcu() is not purely a slab thing
but RCU has to special case it still.

Possible solution 2: instead of the special offset handling, SLUB provides a
callback function, which will determine pointer to the object from the
pointer to a middle of it without knowing the rcu_head offset.
Downside: this will have some overhead, but SLUB_TINY is not meant to be
performant anyway so we might not care.
Upside: we can remove __is_kvfree_rcu_offset() from TINY_RCU as well

Thoughts?

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slub-tiny-kfree_rcu