On 10/13/26 10:52, Kees Cook wrote: > On Mon, Oct 12, 2026 at 11:19:01PM +0200, Vlastimil Babka (SUSE) wrote: >> On 10/10/26 05:18, 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: >> >> > 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? >> >> I think we can indeed simply make sure buckets have the same alignment as >> kmalloc. It comes naturally and should (more than) satisfy everyone. I doubt >> anyone would be needing a more specific alignment than that for a class of >> allocations with a variable size? > > Right, the whole point of kmem_buckets was to replace general kmalloc > uses, where no callers cared about alignment. > >> ctor removal is clear I think > > Completely. I was blinded by the API of kmem_cache_create(). :) > >> for __GFP_ACCOUNT and __GFP_RECLAIMABLE I think what we could do is not to >> drop the flags argument kmem_buckets_create and instead require that it >> matches the usage: >> >> __GFP_RECLAIMABLE requires SLAB_RECLAIM_ACCOUNT and vice versa and I can't >> imagine there would be a buckets instance that would allocate both kinds >> (either with or without __GFP_RECLAIMABLE) > > Okay. (We have no users of this yet, FWIW.) > >> SLAB_ACCOUNT requires __GFP_ACCOUNT, memcg objexts are always created and >> used > > Right. > >> SLAB_MAY_ACCOUNT allows __GFP_ACCOUNT, memcg objexts are always created, and >> used when __GFP_ACCOUNT given >> - perhaps not as optimal as the separate kmalloc types, but good enough?
Ah there's already a precedent, e.g. specifying __GFP_RECLAIM means you get a KMALLOC_RECLAIM cache, but it may or may not be accounted depending on passing also __GFP_ACCOUNT and there are no separate sets of slabs for these. Also same can happen with kmem_cache_create() cache when only some allocations are __GFP_ACCOUNT. > I think it's fine from studying what happens with the skb data. Though > right now, SLAB_ACCOUNT and SLAB_MAY_ACCOUNT are functionally identical, > yes? Both mean that objcg pointers might exist for the slabs from the cache, but SLAB_ACCOUNT also implies __GFP_ACCOUNT for every allocation from the cache even if not given. But for kmem_bucket_alloc() with SLAB_BUCKETS=n with we have nowhere to store that this implication should be done (that would be the "compatibility layer"). >> __GFP_ACCOUNT without either SLAB_ACCOUNT or SLAB_MAY_ACCOUNT is wrong, >> __GFP_ACCOUNT would be ignored > > *ignored* is, I think, wrong here. It would create differing behavior > between SLAB_BUCKETS=y/n. Yes that's why we must avoid it. SLAB_MAY_ACCOUNT is a new thing and because there were already many caches that may or may not pass __GFP_ACCOUNT, we just assume it for now in __kmem_cache_create_args(). But that's a TODO to start tightening it. With buckets we only have a handful of users so we could try doing that right away, and have warnings in place to catch any changes that push it into the "wrong" state. > If we passed __GFP_ACCOUNT through to general > kmalloc when the buckets lacked SLAB_ACCOUNT and SLAB_MAY_ACCOUNT, you'd > lose the isolation, but not the accounting. (And could WARN_ONCE about > it.) But if it was ignored, you'd lose accounting under > SLAB_BUCKETS=y. We could warn and fallback then, I guess. >> with SLAB_BUCKETS=y these rules could be checked e.g. with a >> VM_WARN_ON_ONCE() >> >> with SLAB_BUCKETS=n these rules won't be checked but also not matter in >> practice > > Agreed. That's what I was going for on this version. But like above, > "SLAB_ACCOUNT requires __GFP_ACCOUNT" means what? Under SLAB_BUCKETS=y > the allocation returns NULL? Or it passes through with WARN_ONCE? I'd "SLAB_ACCOUNT requires __GFP_ACCOUNT" means that if it's not done and SLAB_BUCKETS=n, then we get the issue that patch 2 fixes, and have no way to actually find out and warn that it's the case (with SLAB_BUCKETS=n... the warning can be issued with SLAB_BUCKETS=y and then it passes through as if there was __GFP_ACCOUNT). > rather not break something completely; better to fallback on a mismatch > so the requested type happens (be it DMA, ACCOUNTing, etc). Also instead of the scheme above, maybe we could explore the idea of "compatibility layer" too. It would mean that even with SLAB_BUCKETS=n, kmem_buckets_create() would create some object to hold the gfp_flags we want to append to all allocations from that bucket. But then kmem_buckets_alloc() would have to do something a bit more than map to __kmalloc_node_noprof (with PASS_KMALLOC_PARAMS directly). Of course SLAB_BUCKETS=y would be using this way too then to avoid further complication. Maybe the result would be more cleaner (simplifying PASS_KMALLOC_PARAMS and getting rid of PASS_BUCKET_PARAMS?) but I can't say immediately how well it would work.
