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
