On 7/22/26 09:28, Harry Yoo wrote: >> Nits below: >> >>> --- >>> mm/slab.h | 18 +++++++++++++++++- >>> mm/slab_common.c | 2 +- >>> mm/slub.c | 36 ++++++++++++++++++++++++++++-------- >>> 3 files changed, 46 insertions(+), 10 deletions(-) >>> >>> diff --git a/mm/slab.h b/mm/slab.h >>> index 281a65233795..85ef2ebc9812 100644 >>> --- a/mm/slab.h >>> +++ b/mm/slab.h >>> @@ -429,7 +445,7 @@ static inline bool is_kmalloc_normal(struct kmem_cache >>> *s) >>> return !(s->flags & (SLAB_CACHE_DMA|SLAB_ACCOUNT|SLAB_RECLAIM_ACCOUNT)); >>> } >>> >>> -bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj); >>> +bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj, unsigned int >>> free_flags); >>> void flush_all_rcu_sheaves(void); >>> void flush_rcu_sheaves_on_cache(struct kmem_cache *s); >>> >>> diff --git a/mm/slab_common.c b/mm/slab_common.c >>> index b6426d7ceec9..e07b4e6d6679 100644 >>> --- a/mm/slab_common.c >>> +++ b/mm/slab_common.c >>> @@ -1605,7 +1605,7 @@ static bool kfree_rcu_sheaf(void *obj) >>> >>> s = slab->slab_cache; >>> if (likely(!IS_ENABLED(CONFIG_NUMA) || slab_nid(slab) == numa_mem_id())) >>> - return __kfree_rcu_sheaf(s, obj); >>> + return __kfree_rcu_sheaf(s, obj, SLAB_FREE_DEFAULT); >>> >>> return false; >>> } >>> diff --git a/mm/slub.c b/mm/slub.c >>> index e32a68677537..0c350274fbff 100644 >>> --- a/mm/slub.c >>> +++ b/mm/slub.c >>> @@ -2814,10 +2814,14 @@ static inline struct slab_sheaf >>> *alloc_empty_sheaf(struct kmem_cache *s, >>> >>> gfp &= ~OBJCGS_CLEAR_MASK; >>> >>> + if (alloc_flags & SLAB_ALLOC_NOLOCK) >>> + gfp &= ~__GFP_RECLAIM; >> >> So in general we expect gfp and alloc flags to be compatible and warn if >> they are not. This now performs an auto-adjustment, which makes it unusual. > > That's fair. > >> But AFAICS only one caller relies on it - __kfree_rcu_sheaf(). So maybe we >> could just do it there? > > Will do. I don't have strong preference on this.
Thanks! >>> return __alloc_empty_sheaf(s, gfp, alloc_flags, s->sheaf_capacity); >>> } >>> >>> -static void free_empty_sheaf(struct kmem_cache *s, struct slab_sheaf >>> *sheaf) >>> +static void __free_empty_sheaf(struct kmem_cache *s, struct slab_sheaf >>> *sheaf, >>> + bool allow_spin) >> >> Why not free_flags instead of allow_spin? Since you already introduced them. > > Indeed I tried that but gave up on doing that as part of series > after realizing free_empty_sheaf() alone has 12 callers :) Yeah but I meant __free_empty_sheaf() only. It's fine to leave the wrapper without the new parameter for the most common case, like you did. > But I think it's worth teaching those functions (including > free_empty_sheaf()) to handle SLAB_ALLOC_* and SLAB_FREE_* flags > rather than propagating allow_spin. It doesn't have to be now :)
