ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Bart Van Assche <bvanassche@acm.org>,
	 James Bottomley <James.Bottomley@hansenpartnership.com>,
	ksummit@lists.linux.dev,  Dan Williams <dan.j.williams@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	 Dan Carpenter <dan.carpenter@linaro.org>
Subject: Re: Clarifying confusion of our variable placement rules caused by cleanup.h
Date: Tue, 18 Nov 2025 13:05:13 -0800	[thread overview]
Message-ID: <CAHk-=wjMMSAaqTjBSfYenfuzE1bMjLj+2DLtLWJuGt07UGCH_Q@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=whQ9kzJWFrCY9C3bPkdfW5Zb0TdvKNdPCdzPSnrzHyhVw@mail.gmail.com>

On Tue, 18 Nov 2025 at 12:23, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Now, I think that your crazy case that wants to do alignment etc may
> never be a good example of this, but for the simpler case of "I just
> want a normal allocation for this" a couple of helper macros would
> make it quite nice.

Actually, if we made some "kmalloc_type()" helper macro also do
__alignof__ in addition to the sizeef() , and _if_ the type itself
already has the proper alignment information, it probably would work
fine for most cases.

Your particular example is still too specialized, though, with that
whole "I want a particular node" and a "I want the dynamic cacheline
alignment" rather than "current node" and "static alignment based on
type".

So I think *that* particular case is always specialized enough that
there won't be some simple helper macro to make it more readable, and
as a result you're actually better off just splitting it out, even if
it then results in some duplication, ie just doing

        struct ring_buffer_per_cpu *cpu_buffer __free(kfree);

        cpu_buffer = kzalloc_node(...);

as separate things (but probably next to each other, so that the
"__free(kfree)" part makes sense because the allocation is right
there).

But hey, you could also just make your own alloc/free wrapper
functions, and try to make it more legible that way.

Just a simple

    struct ring_buffer_per_cpu *cpu_buffer_alloc()
    { return kzalloc_node(...); }

would then make that otherwise nasty allocation then become just

    auto cpu_buffer __free(kfree) = cpu_buffer_alloc();

and suddenly it all looks pretty simple. No?

But please do _not_ spread one complex thing over three lines. Split
it up *somehow* to make it easier to read.

Either by just splitting it up into multiple parts, or maybe like the
above with a helper wrapper allocator or whatever.

Helper wrappers that are just used once or twice can still be nice
readability improvements just because they make each part be easier to
read on its own.

              Linus

  reply	other threads:[~2025-11-18 21:05 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 16:39 James Bottomley
2025-11-18 17:18 ` Bart Van Assche
2025-11-18 18:38   ` Linus Torvalds
2025-11-18 19:04     ` Bart Van Assche
2025-11-18 19:14       ` Linus Torvalds
2025-11-18 20:43         ` Al Viro
2025-11-18 19:15       ` H. Peter Anvin
2025-11-18 19:11     ` H. Peter Anvin
2025-11-18 19:16       ` Linus Torvalds
2025-11-18 19:19         ` H. Peter Anvin
2025-11-18 19:17     ` Steven Rostedt
2025-11-18 19:22       ` Linus Torvalds
2025-11-18 19:56         ` Steven Rostedt
2025-11-18 20:23           ` Linus Torvalds
2025-11-18 21:05             ` Linus Torvalds [this message]
2025-11-18 20:21       ` James Bottomley
2025-11-18 20:30         ` Linus Torvalds
2025-11-18 20:51         ` Steven Rostedt
2025-11-18 21:10           ` James Bottomley
2025-11-18 22:34             ` Steven Rostedt
2025-11-18 23:32               ` James Bottomley
2025-11-18 19:23 ` H. Peter Anvin
2025-11-18 20:28   ` James Bottomley
2025-11-25 13:09 ` Jani Nikula
2025-11-25 14:25 ` Alexey Dobriyan
2025-11-25 15:32   ` Stephen Hemminger
2025-11-25 16:04   ` Steven Rostedt
2025-11-25 17:57   ` H. Peter Anvin

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-=wjMMSAaqTjBSfYenfuzE1bMjLj+2DLtLWJuGt07UGCH_Q@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=bvanassche@acm.org \
    --cc=dan.carpenter@linaro.org \
    --cc=dan.j.williams@intel.com \
    --cc=ksummit@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox