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
