On 7/21/26 7:07 PM, Vlastimil Babka (SUSE) wrote: > On 7/20/26 14:44, Harry Yoo (Oracle) wrote: >> Teach kfree_rcu_sheaf() how to handle the !allow_spin case. Try to get >> an empty sheaf from pcs->spare or the barn even when spinning is not >> allowed. Unlike __pcs_replace_full_main(), try harder to allocate >> an empty sheaf because the fallback path will be more expensive than >> kfree_nolock(). >> >> Now that slab has internal alloc_flags to describe context, introduce >> free_flags analogously and convert free_flags to alloc_flags when >> allocating memory in the free path. alloc_empty_sheaf() now strips >> __GFP_RECLAIM when SLAB_ALLOC_NOLOCK is specified. >> >> When trylock fails or the kernel observes non-NULL pcs->rcu_free after >> lock acquisition, free the sheaf instead of putting it to the barn. >> This is rare and not worth complicating the code. >> >> Since call_rcu() cannot be called in an unknown context, >> kfree_rcu_sheaf() fails when the rcu sheaf becomes full. >> >> Link: >> https://lore.kernel.org/linux-mm/[email protected] >> Signed-off-by: Harry Yoo (Oracle) <[email protected]> > > LGTM. > > Reviewed-by: Vlastimil Babka (SUSE) <[email protected]>
Thanks a lot for reviewing, Vlastimil! > 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. >> 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 :) 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. -- Cheers, Harry / Hyeonggon
OpenPGP_signature.asc
Description: OpenPGP digital signature
