linux-kernel - Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu

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]

Date:   Wed, 15 Mar 2023 16:28:40 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     Uladzislau Rezki <urezki@...il.com>, Jens Axboe <axboe@...nel.dk>,
        LKML <linux-kernel@...r.kernel.org>, RCU <rcu@...r.kernel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>,
        Philipp Reisner <philipp.reisner@...bit.com>,
        Bryan Tan <bryantan@...are.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Bob Pearson <rpearsonhpe@...il.com>,
        Ariel Levkovich <lariel@...dia.com>,
        "Theodore Ts'o" <tytso@....edu>, Julian Anastasov <ja@....bg>
Subject: Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to
 k[v]free_rcu_mightsleep()

On Wed, 15 Mar 2023 15:57:02 -0400
Joel Fernandes <joel@...lfernandes.org> wrote:

> > I was going to suggest "kvfree_rcu_might_synchronize()" but that's just
> > getting ridiculous.  
> 
> No, synchronize() is incorrect. The code really can sleep for other
> reasons like memory allocation. It is not that simple of an
> implementation as one may imagine. mightsleep is really the correct
> wording IMHO.
> 
> > Still, I will replace that code back to a kfree() and rcu_synchonize() than
> > to let that other name get in.  
> 
> I think it is too late for that for now, we already have conversions
> going into the other subsystems, that means we'll have to redo all
> that over again (even if it sounded like a good idea, which it is
> not).
> 
> I would rather you just did: "#define kvfree_rcu_tracing
> #kvfree_rcu_mightsleep", or something like that, if it is really a
> problem. ;-)
> 
> Also you are really the first person I know of who has a problem with that name.

I guess you didn't read Jens's reply.

The main issue I have with this, is that "might_sleep" is just an
implementation issue. It has *nothing* to do with what the call is about.
It is only about freeing something with RCU. It has nothing to do with
sleeping. I don't use it because it might sleep. I use it to free something.

If you don't like kvfree_rcu_synchronization() then call it
kvfree_rcu_headless() and note that currently it can sleep. Because in
the future, if we come up with an implementation where we it doesn't sleep,
then we don't need to go and rename all the users in the future.

See where I have the problem with the name "might_sleep"?

-- Steve