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: <c210c44d-d1b1-4dc3-839d-f99ad73c3ee9@suse.cz>
Date: Wed, 22 Jan 2025 17:43:06 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Joel Fernandes <joel@...lfernandes.org>, paulmck@...nel.org
Cc: Uladzislau Rezki <urezki@...il.com>, 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 1/22/25 16:04, Joel Fernandes wrote:
> On Tue, Jan 21, 2025 at 3:32 PM Paul E. McKenney <paulmck@...nel.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.

> thanks,
> 
>  - Joel