Re: [PATCH 02/10] slab: Rename slab->__page_flags to slab->flags

Vlastimil Babka
2025-06-09T13:12:40Z

On 6/7/25 00:22, Matthew Wilcox (Oracle) wrote:
> Slab has its own reasons for using flag bits; they aren't just
> the page bits. Maybe this won't be the ultimate solution, but
> we should be clear that these bits are in use.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> mm/slab.h | 16 ++++++++++++++--
> mm/slub.c | 6 +++---
> 2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index 05a21dc796e0..a25f12244b6c 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -50,7 +50,7 @@ typedef union {
>
> /* Reuses the bits in struct page */
> struct slab {
> - unsigned long __page_flags;
> + unsigned long flags;
>
> struct kmem_cache *slab_cache;
> union {
> @@ -99,7 +99,7 @@ struct slab {
>
> #define SLAB_MATCH(pg, sl) \
> static_assert(offsetof(struct page, pg) == offsetof(struct slab, sl))
> -SLAB_MATCH(flags, __page_flags);
> +SLAB_MATCH(flags, flags);
> SLAB_MATCH(compound_head, slab_cache); /* Ensure bit 0 is clear */
> SLAB_MATCH(_refcount, __page_refcount);
> #ifdef CONFIG_MEMCG
> @@ -113,6 +113,18 @@ static_assert(sizeof(struct slab) <= sizeof(struct page));
> static_assert(IS_ALIGNED(offsetof(struct slab, freelist), sizeof(freelist_aba_t)));
> #endif
>
> +/**
> + * enum slab_flags - How the slab flags bits are used.
> + * @SL_locked: Is locked with slab_lock()
> + *
> + * The slab flags share space with the page flags but some bits have
> + * different interpretations. The high bits are used for information
> + * like zone/node/section.
> + */
> +enum slab_flags {
> + SL_locked,

Given how in 3 you do SL_partial = PG_workingset, we could just use
PG_locked here too, just as a flag known to be safe. I've read your
discussion with Harry but I'd simply do that for now?

Also I think the whole enum could be moved to mm/slub.c ? Patch 4 adds
SL_pfmemalloc but also moves all its uses to mm/slub.c

> +};
> +
> /**
> * folio_slab - Converts from folio to slab.
> * @folio: The folio.
> diff --git a/mm/slub.c b/mm/slub.c
> index 31e11ef256f9..e9cbacee406d 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -639,12 +639,12 @@ static inline unsigned int slub_get_cpu_partial(struct kmem_cache *s)
> */
> static __always_inline void slab_lock(struct slab *slab)
> {
> - bit_spin_lock(PG_locked, &slab->__page_flags);
> + bit_spin_lock(SL_locked, &slab->flags);
> }
>
> static __always_inline void slab_unlock(struct slab *slab)
> {
> - bit_spin_unlock(PG_locked, &slab->__page_flags);
> + bit_spin_unlock(SL_locked, &slab->flags);
> }
>
> static inline bool
> @@ -1010,7 +1010,7 @@ static void print_slab_info(const struct slab *slab)
> {
> pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%pGp\n",
> slab, slab->objects, slab->inuse, slab->freelist,
> - &slab->__page_flags);
> + &slab->flags);
> }
>
> void skip_orig_size_check(struct kmem_cache *s, const void *object)