| Message ID | 20230418074639.1903197-1-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv3] zsmalloc: allow only one active pool compaction context | expand |
On Tue, Apr 18, 2023 at 12:46 AM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > zsmalloc pool can be compacted concurrently by many contexts, > e.g. > > cc1 handle_mm_fault() > do_anonymous_page() > __alloc_pages_slowpath() > try_to_free_pages() > do_try_to_free_pages( > lru_gen_shrink_node() > shrink_slab() > do_shrink_slab() > zs_shrinker_scan() > zs_compact() > > Pool compaction is currently (basically) single-threaded as > it is performed under pool->lock. Having multiple compaction > threads results in unnecessary contention, as each thread > competes for pool->lock. This, in turn, affects all zsmalloc > operations such as zs_malloc(), zs_map_object(), zs_free(), etc. > > Introduce the pool->compaction_in_progress atomic variable, > which ensures that only one compaction context can run at a > time. This reduces overall pool->lock contention in (corner) > cases when many contexts attempt to shrink zspool simultaneously. > > Fixes: c0547d0b6a4b ("zsmalloc: consolidate zs_pool's migrate_lock and size_class's locks") > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> Reviewed-by: Yosry Ahmed <yosryahmed@google.com> > --- > mm/zsmalloc.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index cc81dfba05a0..44ddaf5d601e 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -264,6 +264,7 @@ struct zs_pool { > struct work_struct free_work; > #endif > spinlock_t lock; > + atomic_t compaction_in_progress; > }; > > struct zspage { > @@ -2274,6 +2275,15 @@ unsigned long zs_compact(struct zs_pool *pool) > struct size_class *class; > unsigned long pages_freed = 0; > > + /* > + * Pool compaction is performed under pool->lock so it is basically > + * single-threaded. Having more than one thread in __zs_compact() > + * will increase pool->lock contention, which will impact other > + * zsmalloc operations that need pool->lock. > + */ > + if (atomic_xchg(&pool->compaction_in_progress, 1)) > + return 0; > + > for (i = ZS_SIZE_CLASSES - 1; i >= 0; i--) { > class = pool->size_class[i]; > if (class->index != i) > @@ -2281,6 +2291,7 @@ unsigned long zs_compact(struct zs_pool *pool) > pages_freed += __zs_compact(pool, class); > } > atomic_long_add(pages_freed, &pool->stats.pages_compacted); > + atomic_set(&pool->compaction_in_progress, 0); > > return pages_freed; > } > @@ -2388,6 +2399,7 @@ struct zs_pool *zs_create_pool(const char *name) > > init_deferred_free(pool); > spin_lock_init(&pool->lock); > + atomic_set(&pool->compaction_in_progress, 0); > > pool->name = kstrdup(name, GFP_KERNEL); > if (!pool->name) > -- > 2.40.0.634.g4ca3ef3211-goog >
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index cc81dfba05a0..44ddaf5d601e 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -264,6 +264,7 @@ struct zs_pool { struct work_struct free_work; #endif spinlock_t lock; + atomic_t compaction_in_progress; }; struct zspage { @@ -2274,6 +2275,15 @@ unsigned long zs_compact(struct zs_pool *pool) struct size_class *class; unsigned long pages_freed = 0; + /* + * Pool compaction is performed under pool->lock so it is basically + * single-threaded. Having more than one thread in __zs_compact() + * will increase pool->lock contention, which will impact other + * zsmalloc operations that need pool->lock. + */ + if (atomic_xchg(&pool->compaction_in_progress, 1)) + return 0; + for (i = ZS_SIZE_CLASSES - 1; i >= 0; i--) { class = pool->size_class[i]; if (class->index != i) @@ -2281,6 +2291,7 @@ unsigned long zs_compact(struct zs_pool *pool) pages_freed += __zs_compact(pool, class); } atomic_long_add(pages_freed, &pool->stats.pages_compacted); + atomic_set(&pool->compaction_in_progress, 0); return pages_freed; } @@ -2388,6 +2399,7 @@ struct zs_pool *zs_create_pool(const char *name) init_deferred_free(pool); spin_lock_init(&pool->lock); + atomic_set(&pool->compaction_in_progress, 0); pool->name = kstrdup(name, GFP_KERNEL); if (!pool->name)
zsmalloc pool can be compacted concurrently by many contexts, e.g. cc1 handle_mm_fault() do_anonymous_page() __alloc_pages_slowpath() try_to_free_pages() do_try_to_free_pages( lru_gen_shrink_node() shrink_slab() do_shrink_slab() zs_shrinker_scan() zs_compact() Pool compaction is currently (basically) single-threaded as it is performed under pool->lock. Having multiple compaction threads results in unnecessary contention, as each thread competes for pool->lock. This, in turn, affects all zsmalloc operations such as zs_malloc(), zs_map_object(), zs_free(), etc. Introduce the pool->compaction_in_progress atomic variable, which ensures that only one compaction context can run at a time. This reduces overall pool->lock contention in (corner) cases when many contexts attempt to shrink zspool simultaneously. Fixes: c0547d0b6a4b ("zsmalloc: consolidate zs_pool's migrate_lock and size_class's locks") Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> --- mm/zsmalloc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)