Re: [PATCH 03/10] slab: Add SL_private flag

Harry Yoo
2025-06-09T02:25:27Z

On Fri, Jun 06, 2025 at 11:22:05PM +0100, Matthew Wilcox (Oracle) wrote:
> Give slab its own name for this flag. Keep the PG_workingset alias
> information in one place.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

nit: subject should be "slab: Add SL_partial flag"

Otherwise looks good to me.

Acked-by: Harry Yoo <harry.yoo@oracle.com>

--
Cheers,
Harry / Hyeonggon

> mm/slab.h | 2 ++
> mm/slub.c | 20 ++++++++------------
> 2 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/mm/slab.h b/mm/slab.h
> index a25f12244b6c..fca818011f7d 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -116,6 +116,7 @@ static_assert(IS_ALIGNED(offsetof(struct slab, freelist), sizeof(freelist_aba_t)
> /**
> * enum slab_flags - How the slab flags bits are used.
> * @SL_locked: Is locked with slab_lock()
> + * @SL_partial: On the per-node partial list
> *
> * The slab flags share space with the page flags but some bits have
> * different interpretations. The high bits are used for information
> @@ -123,6 +124,7 @@ static_assert(IS_ALIGNED(offsetof(struct slab, freelist), sizeof(freelist_aba_t)
> */
> enum slab_flags {
> SL_locked,
> + SL_partial = PG_workingset, /* Historical reasons for this bit */
> };
>
> /**
> diff --git a/mm/slub.c b/mm/slub.c
> index e9cbacee406d..804b39d06fa0 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -91,14 +91,14 @@
> * The partially empty slabs cached on the CPU partial list are used
> * for performance reasons, which speeds up the allocation process.
> * These slabs are not frozen, but are also exempt from list management,
> - * by clearing the PG_workingset flag when moving out of the node
> + * by clearing the SL_partial flag when moving out of the node
> * partial list. Please see __slab_free() for more details.
> *
> * To sum up, the current scheme is:
> - * - node partial slab: PG_Workingset && !frozen
> - * - cpu partial slab: !PG_Workingset && !frozen
> - * - cpu slab: !PG_Workingset && frozen
> - * - full slab: !PG_Workingset && !frozen
> + * - node partial slab: SL_partial && !frozen
> + * - cpu partial slab: !SL_partial && !frozen
> + * - cpu slab: !SL_partial && frozen
> + * - full slab: !SL_partial && !frozen
> *
> * list_lock
> *
> @@ -2717,23 +2717,19 @@ static void discard_slab(struct kmem_cache *s, struct slab *slab)
> free_slab(s, slab);
> }
>
> -/*
> - * SLUB reuses PG_workingset bit to keep track of whether it's on
> - * the per-node partial list.
> - */
> static inline bool slab_test_node_partial(const struct slab *slab)
> {
> - return folio_test_workingset(slab_folio(slab));
> + return test_bit(SL_partial, &slab->flags);
> }
>
> static inline void slab_set_node_partial(struct slab *slab)
> {
> - set_bit(PG_workingset, folio_flags(slab_folio(slab), 0));
> + set_bit(SL_partial, &slab->flags);
> }
>
> static inline void slab_clear_node_partial(struct slab *slab)
> {
> - clear_bit(PG_workingset, folio_flags(slab_folio(slab), 0));
> + clear_bit(SL_partial, &slab->flags);
> }
>
> /*
> --
> 2.47.2
>
>