Re: [PATCH v2] mm/slab: simplify SLAB_* flag handling

[Hyeonggon Yoo]

On Sat, Jan 25, 2025 at 1:49 AM Kevin Brodsky <kevin.brodsky@xxxxxxx> wrote:
>
> SLUB is the only remaining allocator. We can therefore get rid of
> the logic for allocator-specific flags:
>
> * Merge SLAB_CACHE_FLAGS into SLAB_CORE_FLAGS.
>
> * Remove CACHE_CREATE_MASK and instead mask out SLAB_DEBUG_FLAGS if
>   !CONFIG_SLUB_DEBUG. SLAB_DEBUG_FLAGS is now defined
>   unconditionally (no impact on existing code, which ignores it if
>   !CONFIG_SLUB_DEBUG).
>
> * Define SLAB_FLAGS_PERMITTED in terms of SLAB_CORE_FLAGS and
>   SLAB_DEBUG_FLAGS (no functional change).
>
> While at it also remove misleading comments that suggest that
> multiple allocators are available.
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@xxxxxxx>

Hi Kevin,
This patch in general looks fine to me.

However, there are subtle changes that were not documented by the changelog.
SLUB currently does not _completely_ ignore debug flags even when
CONFIG_SLUB_DEBUG=n. For example, calculate_sizes() relocate the free pointer
regardless of CONFIG_SLUB_DEBUG.

I believe completely ignoring debug flags should be acceptable
when CONFIG_SLUB_DEBUG=n, but this change should at least be documented
in the changelog.

Additionally, beyond what this patch currently does, we can remove several
CONFIG_SLUB_DEBUG #ifdefs in some functions (e.g., in calculate_sizes())
on top of this patch.

How does that sound to you?

Best,
Hyeonggon