On 12/3/23 11:15, Chengming Zhou wrote: > On 2023/12/3 14:53, Hyeonggon Yoo wrote: >> On Thu, Nov 2, 2023 at 12:25 PM <chengming.zhou@xxxxxxxxx> wrote: >>> >>> From: Chengming Zhou <zhouchengming@xxxxxxxxxxxxx> >>> >>> Now we will freeze slabs when moving them out of node partial list to >>> cpu partial list, this method needs two cmpxchg_double operations: >>> >>> 1. freeze slab (acquire_slab()) under the node list_lock >>> 2. get_freelist() when pick used in ___slab_alloc() >>> >>> Actually we don't need to freeze when moving slabs out of node partial >>> list, we can delay freezing to when use slab freelist in ___slab_alloc(), >>> so we can save one cmpxchg_double(). >>> >>> And there are other good points: >>> - The moving of slabs between node partial list and cpu partial list >>> becomes simpler, since we don't need to freeze or unfreeze at all. >>> >>> - The node list_lock contention would be less, since we don't need to >>> freeze any slab under the node list_lock. >>> >>> We can achieve this because there is no concurrent path would manipulate >>> the partial slab list except the __slab_free() path, which is now >>> serialized by slab_test_node_partial() under the list_lock. >>> >>> Since the slab returned by get_partial() interfaces is not frozen anymore >>> and no freelist is returned in the partial_context, so we need to use the >>> introduced freeze_slab() to freeze it and get its freelist. >>> >>> Similarly, the slabs on the CPU partial list are not frozen anymore, >>> we need to freeze_slab() on it before use. >>> >>> We can now delete acquire_slab() as it became unused. >>> >>> Signed-off-by: Chengming Zhou <zhouchengming@xxxxxxxxxxxxx> >>> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx> >>> Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> >>> --- >>> mm/slub.c | 113 +++++++++++------------------------------------------- >>> 1 file changed, 23 insertions(+), 90 deletions(-) >>> >>> diff --git a/mm/slub.c b/mm/slub.c >>> index edf567971679..bcb5b2c4e213 100644 >>> --- a/mm/slub.c >>> +++ b/mm/slub.c >>> @@ -2234,51 +2234,6 @@ static void *alloc_single_from_new_slab(struct kmem_cache *s, >>> return object; >>> } >>> >>> -/* >>> - * Remove slab from the partial list, freeze it and >>> - * return the pointer to the freelist. >>> - * >>> - * Returns a list of objects or NULL if it fails. >>> - */ >>> -static inline void *acquire_slab(struct kmem_cache *s, >>> - struct kmem_cache_node *n, struct slab *slab, >>> - int mode) >> >> Nit: alloc_single_from_partial()'s comment still refers to acquire_slab(). >> > > Ah, right! It should be changed to remove_partial(). > > diff --git a/mm/slub.c b/mm/slub.c > index 437485a2408d..623c17a4cdd6 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2463,7 +2463,7 @@ static inline void remove_partial(struct kmem_cache_node *n, > } > > /* > - * Called only for kmem_cache_debug() caches instead of acquire_slab(), with a > + * Called only for kmem_cache_debug() caches instead of remove_partial(), with a > * slab from the n->partial list. Remove only a single object from the slab, do > * the alloc_debug_processing() checks and leave the slab on the list, or move > * it to full list if it was the last free object. > > Hi Vlastimil, could you please help to fold it? Done, thanks.
