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 15:57:02 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Steven Rostedt <rostedt@...dmis.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()

Hey Steve,

On Wed, Mar 15, 2023 at 3:35 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 15 Mar 2023 20:25:10 +0100
> Uladzislau Rezki <urezki@...il.com> wrote:
>
> > > This was my main complaint too, kvfree_rcu_mightsleep() is an absolutely
> > > horrible name for an API... But nobody seemed to care about that!
> > >
> > > I like the _synchronize() suggestion, as it matches other RCU naming.
> > >
> > This is basically about what it does. If you renamed it to "_synchronize()"
> > in reality it would not mean that it always a synchronous call, most of the
> > time it is not whereas the name would point that it is.
>
> No, just comment it.
>
> 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.

 - Joel