mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Harry Yoo <harry@kernel.org>
To: Kees Cook <kees@kernel.org>
Cc: Vlastimil Babka <vbabka@kernel.org>,
	 Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	 Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	 Shakeel Butt <shakeel.butt@linux.dev>,
	Muchun Song <muchun.song@linux.dev>,
	cgroups@vger.kernel.org,  linux-mm@kvack.org,
	Pedro Falcato <pfalcato@suse.de>,
	 Kuniyuki Iwashima <kuniyu@google.com>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v6 2/8] ipc, msg: Account msg_msg allocations with GFP_KERNEL_ACCOUNT again
Date: Mon, 12 Oct 2026 22:05:04 +0100	[thread overview]
Message-ID: <as03h8sgRzpw7JXI@thinkstation> (raw)
In-Reply-To: <202610092014.AAB20B81@keescook>

On Fri, Oct 09, 2026 at 08:18:59PM -0700, Kees Cook wrote:
> On Fri, Oct 09, 2026 at 09:39:48PM +0200, Harry Yoo wrote:
> > On Fri, Oct 09, 2026 at 04:22:03PM +0200, Harry Yoo wrote:
> > > And now I see the initial kmem_buckets design did not sufficiently
> > > tackle the question "How this should work when kmem_buckets falls back
> > > to kmalloc?"
> > > 
> > > I suppose the kmem_buckets' abstraction should not be too tightly
> > > coupled with kmalloc caches. Creating a kmem_buckets should be
> > > conceptually equivalent to creating a set of caches with speicifc
> > > slab flags, size, align, useroffset/size. (for variable size allocation).
> > 
> > Just to clarify... I have a few concerns on this area.
> > 
> > 1. Complexity.
> > 
> > It's very counter-intuitive to understand how it works when
> > SLAB_BUCKETS is not compiled. My brain cannot easily process two
> > different implementations w/ and w/o SLAB_BUCKETS.
> > 
> > A "compatibility layer" allows us to assume certain expectations
> > and ignore all implementation details when SLAB_BUCKETS is not compiled.
> 
> I'd rather make kmem_buckets not have a SLAB_BUCKETS option. :)

We'll need some data but looking at the discussion thread [1]:
> I wasn't able to measure any performance impact at all. It does add a
> small bit of memory overhead, but it's on the order of a dozen pages
> used for the 2 extra sets of buckets. (E.g. it's well below the overhead
> introduced by CONFIG_RANDOM_KMALLOC_CACHES, which adds 16 extra sets
> of buckets.)

[1] https://lore.kernel.org/all/202404280921.A7683D511@keescook 

If the data still shows that the cost is still "dozens of pages"
for a few buckets w/o measurable performance overhead, I think that's
certainly doable.

> > 2. Making kmem_buckets tightly coupled with kmalloc's implementation
> >    details is not reasonable.
> > 
> > Let's say, at some point we make SLAB_BUCKETS always enabled and drop
> > the config option. Then all kmem_buckets users will now have certain
> > assumptions inherted from kmalloc (e.g. alignment), which is not
> > reasonable.
> 
> Until it's always enabled, I don't see a reasonable way to separate
> those assumptions.
> 
> > 3. __GFP_ACCOUNT and __GFP_DMA should be applied to all allocations
> > in the slab cache. For kmalloc, these flags are only used to select
> > the kmalloc cache (which has equivalent SLAB_* flags for __GFP_ACCOUNT
> > or __GFP_DMA) to serve the allocation.
> > 
> > Specifying it on individual allocations (rather than as SLAB_ flags)
> > makes it easy to break the assumption.
> 
> As in, you don't want a "pass-through" mode for __GFP_DMA?
> 
> It's a lot of overhead for __GFP_DMA to have multiple providers, so I'd
> rather allow kmem_buckets to allow pass-through (perhaps explicitly
> opt-in) where then the other options (e.g. alignment) must match the
> general kmalloc caches so that pass-through is consistent.

Having both "Specifying __GFP_DMA for slab caches without SLAB_DMA is
illegal" and "However kmem_buckets is an exception because creating
separate buckets for SLAB_DMA is costly" in mind is confusing and
easy to get wrong...

Perhaps the user should bypass kmem_buckets and fall back
to kmalloc?

> > > When it falls back to kmalloc, kmem_buckets itself should provide a
> > > compatibility layer when falling back to kmalloc.
> > 
> > For example, calculate_sizes() converts certain SLAB_* flags into
> > GFP flags (s->allocflags) for allocation. We can have a similar
> > logic when falling back to kmalloc.
> > 
> > Though current implementation does not allow this and requires
> > some rework.
> 
> I think the current series does everything correctly in the sense that
> nothing is allowing for mismatched fallbacks/passthrus. We could iterate
> from there once we have a need for this where a non-kmalloc-alignment is
> wanted, etc?

Making kmalloc tightly coupled with DMA alignment requirement was
a mistake, and now it's too late to fix that. It's better to not
repeat the same mistake again?

-- 
Cheers,
Harry / Hyeonggon

  reply	other threads:[~2026-10-12 21:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-06  9:20 [PATCH net-next v6 0/8] net: skb: isolate skb data area allocations into a separate bucket Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 1/8] mm/slab: Mark the kmem_buckets_create() context as a Context: section Kees Cook
2026-10-12 16:55   ` Vlastimil Babka (SUSE)
2026-10-06  9:20 ` [PATCH net-next v6 2/8] ipc, msg: Account msg_msg allocations with GFP_KERNEL_ACCOUNT again Kees Cook
2026-10-09 14:22   ` Harry Yoo
2026-10-09 19:39     ` Harry Yoo
2026-10-10  3:18       ` Kees Cook
2026-10-12 21:05         ` Harry Yoo [this message]
2026-10-12 21:19         ` Vlastimil Babka (SUSE)
2026-10-13  8:52           ` Kees Cook
2026-10-13  9:48             ` Vlastimil Babka (SUSE)
2026-10-10  3:14     ` Kees Cook
2026-10-10 22:37       ` Andrew Morton
2026-10-12 16:46         ` Vlastimil Babka (SUSE)
2026-10-13  3:40           ` Kees Cook
2026-10-12 17:55   ` Vlastimil Babka (SUSE)
2026-10-12 21:03     ` Vlastimil Babka (SUSE)
2026-10-13  3:46       ` Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 3/8] mm/slab: Drop the ctor and flags arguments from kmem_buckets_create() Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 4/8] mm/slab: Give bucket caches the alignment of the caches they mirror Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 5/8] mm/slab: Add kmem_buckets_destroy() Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 6/8] mm/slab: Add tests for the existing kmem_buckets behaviour Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 7/8] mm/slab: Provide kmalloc type fallback for bucket allocations Kees Cook
2026-10-06  9:20 ` [PATCH net-next v6 8/8] net: skb: isolate skb data area allocations into a separate bucket Kees Cook
2026-10-08 21:25 ` [PATCH net-next v6 0/8] " Harry Yoo
2026-10-09  6:36   ` Kees Cook
2026-10-09  7:02   ` Vlastimil Babka

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=as03h8sgRzpw7JXI@thinkstation \
    --to=harry@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=jack@suse.cz \
    --cc=kees@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=pfalcato@suse.de \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=vbabka@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®