Re: [PATCH] rust: slab: add basic slab module

Vlastimil Babka
2025-09-26T17:12:03Z

On 9/26/25 18:58, Danilo Krummrich wrote:
> On Fri Sep 26, 2025 at 6:32 PM CEST, Vlastimil Babka wrote:
>> On 9/26/25 17:55, Danilo Krummrich wrote:
>
>>> // Frees foo and causes the "zombie" cache to actually be destroyed.
>>> kmem_cache_free(foo);
>>
>> The free will be fine. But not cause the cache destruction, as that would
>> require checks on each free. But should be fine wrt safety if we only leak
>> some memory due to a wrong usage, no?
>
> Yes, technically that's safe, but we wouldn't prevent the leak, which still
> is not desirable (and not our ambition for a Rust API).
>
> From a C standpoint, both the warning and the cache leak could be solved by
> making kmem_cache_destroy() fallible as you mentioned previously.
>
> On the Rust side the cache would be represented with a struct KmemCache<T>
> (where T is the type that should be allocated by the cache).
>
> kmem_cache_destroy() would be called from KmemCache<T>::drop(), which is not
> fallible. But even if it were, we can't enforce that users keep the KmemCache
> instance alive as long as there are allocations.
>
> So, either we always keep the KmemCache<T> alive for the whole module lifetime
> (which depending on whether its built-in or not could be considered a memory
> leak as well). Or we ensure that the last kmem_cache_free() also frees the cache
> if kmem_cache_destroy() was called previously.

The rust wrapper side could do that so we don't have to add that check in
all kmem_cache_free() calls, maybe?
Also either way it could perhaps be difficult/expensive (memory barriers
etc) to properly handle a racing kmem_cache_free() and kmem_cache_destroy()
in a way that ensures the cache is always being destroyed, and not have the
kmem_cache_destroy() observe the destroy was premature, while the racing
kmem_cache_free() doesn't yet observe that destroy was attempted, and not
try to remove it.
> OOC, does the cache pointer remain valid if kmem_cache_destroy() is called while
> allocations still exist? I.e. is this, except for the WARN(), valid code?
>
> kmem_cache_destroy(cache);
> kmem_cache_free(foo);
> kmem_cache_destroy(cache);
Currently not, but making it valid hopefully should not be difficult,
without affecting fast path.

> At a quick glance it appears to me that things would go south in
> kmem_cache_release(). Anyways, I don't think it would help, if it would be the
> case.

It would help if it was safe and the rust wrapper side did this retry?