On 7/22/26 4:37 PM, Vlastimil Babka (SUSE) wrote: > 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.
Ah, I see! Right, that's reasonable and will do. >> 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 :) Yeah :) Thanks! -- Cheers, Harry / Hyeonggon
OpenPGP_signature.asc
Description: OpenPGP digital signature
