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

[Uladzislau Rezki <urezki@gmail.com>]

On Wed, Jan 22, 2025 at 05:43:06PM +0100, Vlastimil Babka wrote:
> On 1/22/25 16:04, Joel Fernandes wrote:
> > On Tue, Jan 21, 2025 at 3:32 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >>
> >> On Tue, Jan 21, 2025 at 03:14:16PM +0100, Uladzislau Rezki wrote:
> >> > On Tue, Jan 21, 2025 at 02:49:13PM +0100, Vlastimil Babka wrote:
> >> > > Right so that's the first Possible solution, but without the #ifdef. So
> >> > > there's an overhead of checking __is_kvfree_rcu_offset() even if the
> >> > > batching is done in slab and this function is never called with an offset.
> >> > >
> >> > Or fulfilling a missing functionality? TREE is broken in that sense
> >> > whereas a TINY handles it without any issues.
> >> >
> >> > It can be called for SLUB_TINY option, just call_rcu() instead of
> >> > batching layer. And yes, kvfree_rcu_barrier() switches to rcu_barrier().
> >>
> >> Would this make sense?
> >>
> >>                 if (IS_ENABLED(CONFIG_TINY_RCU) && __is_kvfree_rcu_offset((unsigned long) f)) {
> >>
> >> Just to be repetitive, other alternatives include:
> >>
> >> 1.      Take advantage of SLOB being no longer with us.
> >>
> >> 2.      Get rid of Tiny RCU's special casing of kfree_rcu(), and then
> >>         eliminate the above "if" statement in favor of its "else" clause.
> >>
> >> 3.      Make Tiny RCU implement a trivial version of kfree_rcu() that
> >>         passes a list through RCU.
> >>
> >> I don't have strong feelings, and am happy to defer to your guys'
> >> decision.
> > 
> > If I may chime in with an opinion, I think the cleanest approach would
> > be to not special-case the func pointer and instead provide a callback
> > from the SLAB layer which does the kfree. Then get rid of
> 
> Right.
> 
> > __is_kvfree_rcu_offset() and its usage from Tiny. Granted, there is
> > the overhead of function calling, but I highly doubt that it is going
> > to be a bottleneck, considering that the __is_kvfree_rcu_offset() path
> > is a kfree slow-path.  I feel in the long run, this will also be more
> > maintainable.
> > 
> > Or is there a reason other than the theoretical function call overhead
> > why this may not work?
> 
> My concern was about the overhead of calculating the pointer to the object
> starting address, but it's just some arithmetics, so it should be
> negligible. So I'm prototyping this approach now. Thanks all.
> 
You are welcome :)

--
Uladzislau Rezki