Re: [PATCH v2 0/5] Move kvfree_rcu() into SLAB (v2)

[Vlastimil Babka <vbabka@suse.cz>]

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