All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: keescook@chromium.org, gregkh@linuxfoundation.org,
	pbonzini@redhat.com, linux-kernel@vger.kernel.org,
	ojeda@kernel.org, ndesaulniers@google.com, mingo@redhat.com,
	will@kernel.org, longman@redhat.com, boqun.feng@gmail.com,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
	vschneid@redhat.com, paulmck@kernel.org, frederic@kernel.org,
	quic_neeraju@quicinc.com, joel@joelfernandes.org,
	josh@joshtriplett.org, mathieu.desnoyers@efficios.com,
	jiangshanlai@gmail.com, rcu@vger.kernel.org, tj@kernel.org,
	tglx@linutronix.de, linux-toolchains@vger.kernel.org
Subject: Re: [PATCH v2 0/2] Lock and Pointer guards
Date: Tue, 6 Jun 2023 16:22:26 -0700	[thread overview]
Message-ID: <CAHk-=wgXN1YxGMUFeuC135aeUvqduF8zJJiZZingzS1Pao5h0A@mail.gmail.com> (raw)
In-Reply-To: <20230606180806.GA942082@hirez.programming.kicks-ass.net>

On Tue, Jun 6, 2023 at 11:08 AM Peter Zijlstra <peterz@infradead.org> wrote:>
> Would it all be less offensive if I did: s/guard/cleanup/ on the whole
> thing?

It's more than "guard" for me.

What is "ptr"? Why? We already know of at least one case where it's
not a pointer at all, ie 'struct fd'.

So I *really* hate the naming. Absolutely none of it makes sense to
me. One part is a nonsensical name apparently based on a special-case
operation, and the other part is a nonsensical type from just one
random - if common - implementation issue.

What you want to do is to have a way to define and name a
"constructor/desctructor" pair for an arbitrary type - *not*
necessarily a pointer - and then optionally a way to say "Oh, don't do
the destructor, because I'm actually going to use it long-term".

I said "cleanup", but that's not right either, since we always have to
have that initializer too.

Maybe just bite the bullet, and call the damn thing a "class", and
have some syntax like

     DEFINE_CLASS(name, type, exit, init, initarg...);

to create the infrastructure for some named 'class'. So you'd have

    DEFINE_CLASS(mutex, struct mutex *,
        mutex_unlock(*_P),
        ({mutex_lock(mutex); mutex;}), struct mutex *mutex)

to define the mutex "class", and do

    DEFINE_CLASS(fd, struct fd,
        fdput(*_P),
        fdget(f), int f)

for the 'struct fd' thing.

The above basically would just create the wrapper functions (and
typedefs) for the constructor and destructor.

So the 'DEFINE_CLASS(mutex ..)' thing would basically just expand to

    typedef struct mutex *class_mutex_type;
    static inline void class_mutex_destructor(class_mutex_type *_C)
    { mutex_unlock(*_C); }
    static inline class_mutex_type class_mutex_constructor(struct mutex *mutex)
    { return ({mutex_lock(mutex); mutex;}); }

Then to _instantiate_ one of those, you'd do

    INSTANTIATE_CLASS(name, var)

which would expand to

    class_name_type var
        __attribute__((__cleanup__(class_name_destructor))) =
class_name_constructor

and the magic of that syntax is that you'd actually use that
"INSTANTIATE_CLASS()" with the argument to the init function
afterwards, so you'd actually do

    INSTANTIATE_CLASS(mutex, n)(&sched_domains_mutex);

to create a variable 'n' of class 'mutex', where the
class_mutex_constructor gets the pointer to 'sched_domain_mutex' as
the argument.

And at THAT point, you can do this:

    #define mutex_guard(m) \
        INSTANTIATE_CLASS(mutex, __UNIQUE_ID(mutex))(m)

and now you can do

       mutex_guard(&sched_domains_mutex);

to basically start a guarded section where you hold 'sched_domains_mutex'.

And in that *very* least situation, 'guard' makes sense in the name.
But no earlier. And there is absolutely no 'ptr' anywhere here.

The above should work also for the 'struct fd' case, so you can do

       INSTANTIATE(fd, f)(fd);

to create a 'struct fd' named 'f' that is initialized with
'fdget(fd)', and will DTRT when going out of scope. Modulo any stupid
errors I did.

And ok, I didn't write out the exact macro magic to *get* those
expansions above (I just tried to write out the approximate end
result), but it should be mostly fairly straightforward.

So it would be the usual token pasting tricks to get the 'class type',
the 'class destructor' and the 'class constructor' functions.

Finally, note that the "initarg" part is a macro vararg thing. The
initargs can be empty (for things like RCU), but it could also
possibly be multiple arguments (like a "size and gfp_flags" for an
allocation?).

I'm sure there's something horribly wrong in the above, but my point
is that I'd really like this to make naming and conceptual sense.

And if somebody goes "The above is basically exactly what the original
C++ compiler did back when it was really just a pre-processor for C",
then you'd be 100% right. The above is basically (a part of) what
Bjarne did, except he did it as a separate pass.

And to answer the next question: why not just give up and do C++ -
it's because of all the *other* things Bjarne did.

             Linus

  reply	other threads:[~2023-06-06 23:22 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26 20:52 [PATCH v2 0/2] Lock and Pointer guards Peter Zijlstra
2023-05-26 20:52 ` [PATCH v2 1/2] locking: Introduce __cleanup__ based guards Peter Zijlstra
2023-05-26 21:24   ` Peter Zijlstra
2023-05-26 21:54     ` Linus Torvalds
2023-05-27  8:57       ` Peter Zijlstra
2023-05-26 20:52 ` [PATCH v2 2/2] sched: Use fancy new guards Peter Zijlstra
2023-05-27 17:21 ` [PATCH v2 0/2] Lock and Pointer guards Mathieu Desnoyers
2023-05-27 19:18 ` Linus Torvalds
2023-05-29 12:09   ` Paolo Bonzini
2023-05-29 19:04     ` Linus Torvalds
2023-05-29 21:27       ` Ian Lance Taylor
2023-05-30  0:06         ` Linus Torvalds
2023-05-30  9:23   ` Peter Zijlstra
2023-05-30  9:34     ` Peter Zijlstra
2023-05-30 13:58     ` Valentin Schneider
2023-06-06  9:42     ` Peter Zijlstra
2023-06-06 13:17       ` Linus Torvalds
2023-06-06 13:40         ` Peter Zijlstra
2023-06-06 14:50           ` Linus Torvalds
2023-06-06 16:06             ` Kees Cook
2023-06-06 18:08             ` Peter Zijlstra
2023-06-06 23:22               ` Linus Torvalds [this message]
2023-06-07  9:41                 ` Peter Zijlstra
2023-06-08  8:52                   ` Peter Zijlstra
2023-06-08  9:04                     ` Greg KH
2023-06-08 15:45                     ` Linus Torvalds
2023-06-08 16:47                       ` Kees Cook
2023-06-08 16:59                         ` Linus Torvalds
2023-06-08 17:20                         ` Nick Desaulniers
2023-06-08 18:51                           ` Peter Zijlstra
2023-06-08 20:14                             ` Nick Desaulniers
2023-06-09 10:20                               ` Paolo Bonzini
2023-06-08 20:06                       ` Peter Zijlstra
2023-06-09  2:25                         ` Linus Torvalds
2023-06-09  8:14                           ` Peter Zijlstra
2023-06-09 21:18                           ` Kees Cook
2023-06-09  8:27                       ` Rasmus Villemoes
2023-06-06 15:31       ` Kees Cook
2023-06-06 15:45         ` Linus Torvalds
2023-06-06 16:08           ` Kees Cook
2023-06-08 16:25           ` David Laight
2023-05-30  9:26   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgXN1YxGMUFeuC135aeUvqduF8zJJiZZingzS1Pao5h0A@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=frederic@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=juri.lelli@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=quic_neeraju@quicinc.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.