* [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() @ 2026-09-09 12:35 Kefeng Wang 2026-09-09 12:53 ` Kefeng Wang 2026-09-09 14:52 ` Johannes Weiner 0 siblings, 2 replies; 4+ messages in thread From: Kefeng Wang @ 2026-09-09 12:35 UTC (permalink / raw) To: Andrew Morton Cc: linux-mm, Kefeng Wang, Chengming Zhou, Johannes Weiner, Kairui Song, Nhat Pham, Yosry Ahmed zswap_store() falls through to check_old and walks the swap xarray even when zswap is disabled. Add a zswap_never_enabled() early return matching zswap_load(), and reuse zswap_invalidate() whose xa_empty() check skips empty per-area trees to avoid unnecessary xarry lockup. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Cc: Chengming Zhou <chengming.zhou@linux.dev> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Kairui Song <kasong@tencent.com> Cc: Nhat Pham <nphamcs@gmail.com> Cc: Yosry Ahmed <yosryahmed@google.com> Cc: Andrew Morton <akpm@linux-foundation.org> --- mm/zswap.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/mm/zswap.c b/mm/zswap.c index 5d0d8bd72193..fc6c5e0db5e4 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1488,6 +1488,9 @@ bool zswap_store(struct folio *folio) VM_WARN_ON_ONCE(!folio_test_locked(folio)); VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); + if (zswap_never_enabled()) + return false; + if (!zswap_enabled) goto check_old; @@ -1545,15 +1548,10 @@ bool zswap_store(struct folio *folio) if (!ret) { unsigned type = swp_type(swp); pgoff_t offset = swp_offset(swp); - struct zswap_entry *entry; - struct xarray *tree; - - for (index = 0; index < nr_pages; ++index) { - tree = swap_zswap_tree(swp_entry(type, offset + index)); - entry = xa_erase(tree, offset + index); - if (entry) - zswap_entry_free(entry); - } + + for (index = 0; index < nr_pages; ++index) + zswap_invalidate(swp_entry(type, offset + index)); + } return ret; -- 2.55.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() 2026-09-09 12:35 [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() Kefeng Wang @ 2026-09-09 12:53 ` Kefeng Wang 2026-09-09 14:52 ` Johannes Weiner 1 sibling, 0 replies; 4+ messages in thread From: Kefeng Wang @ 2026-09-09 12:53 UTC (permalink / raw) To: Andrew Morton Cc: linux-mm, Chengming Zhou, Johannes Weiner, Kairui Song, Nhat Pham, Yosry Ahmed On 9/9/2026 8:35 PM, Kefeng Wang wrote: > zswap_store() falls through to check_old and walks the swap xarray > even when zswap is disabled. Add a zswap_never_enabled() early return > matching zswap_load(), and reuse zswap_invalidate() whose xa_empty() > check skips empty per-area trees to avoid unnecessary xarry lockup. Sorry for the typo, should be "xarray lookup". > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Cc: Chengming Zhou <chengming.zhou@linux.dev> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Kairui Song <kasong@tencent.com> > Cc: Nhat Pham <nphamcs@gmail.com> > Cc: Yosry Ahmed <yosryahmed@google.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > --- > mm/zswap.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 5d0d8bd72193..fc6c5e0db5e4 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1488,6 +1488,9 @@ bool zswap_store(struct folio *folio) > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); > > + if (zswap_never_enabled()) > + return false; > + > if (!zswap_enabled) > goto check_old; > > @@ -1545,15 +1548,10 @@ bool zswap_store(struct folio *folio) > if (!ret) { > unsigned type = swp_type(swp); > pgoff_t offset = swp_offset(swp); > - struct zswap_entry *entry; > - struct xarray *tree; > - > - for (index = 0; index < nr_pages; ++index) { > - tree = swap_zswap_tree(swp_entry(type, offset + index)); > - entry = xa_erase(tree, offset + index); > - if (entry) > - zswap_entry_free(entry); > - } > + > + for (index = 0; index < nr_pages; ++index) > + zswap_invalidate(swp_entry(type, offset + index)); > + > } > > return ret; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() 2026-09-09 12:35 [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() Kefeng Wang 2026-09-09 12:53 ` Kefeng Wang @ 2026-09-09 14:52 ` Johannes Weiner 2026-09-10 1:16 ` Kefeng Wang 1 sibling, 1 reply; 4+ messages in thread From: Johannes Weiner @ 2026-09-09 14:52 UTC (permalink / raw) To: Kefeng Wang Cc: Andrew Morton, linux-mm, Chengming Zhou, Kairui Song, Nhat Pham, Yosry Ahmed On Wed, Sep 09, 2026 at 08:35:47PM +0800, Kefeng Wang wrote: > zswap_store() falls through to check_old and walks the swap xarray > even when zswap is disabled. Add a zswap_never_enabled() early return > matching zswap_load(), and reuse zswap_invalidate() whose xa_empty() > check skips empty per-area trees to avoid unnecessary xarry lockup. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Cc: Chengming Zhou <chengming.zhou@linux.dev> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Kairui Song <kasong@tencent.com> > Cc: Nhat Pham <nphamcs@gmail.com> > Cc: Yosry Ahmed <yosryahmed@google.com> > Cc: Andrew Morton <akpm@linux-foundation.org> These are really two separate changes. Can you please split them out? > --- > mm/zswap.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 5d0d8bd72193..fc6c5e0db5e4 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1488,6 +1488,9 @@ bool zswap_store(struct folio *folio) > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); > > + if (zswap_never_enabled()) > + return false; Reviewed-by: Johannes Weiner <hannes@cmpxchg.org> > @@ -1545,15 +1548,10 @@ bool zswap_store(struct folio *folio) > if (!ret) { > unsigned type = swp_type(swp); > pgoff_t offset = swp_offset(swp); > - struct zswap_entry *entry; > - struct xarray *tree; > - > - for (index = 0; index < nr_pages; ++index) { > - tree = swap_zswap_tree(swp_entry(type, offset + index)); > - entry = xa_erase(tree, offset + index); > - if (entry) > - zswap_entry_free(entry); > - } > + > + for (index = 0; index < nr_pages; ++index) > + zswap_invalidate(swp_entry(type, offset + index)); That dance through a swp_entry_t was already kind of awful. This would be a good opportunity to refactor things to avoid that: diff --git a/mm/zswap.c b/mm/zswap.c index e6ec3295bdb0..df18fdaae703 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -228,10 +228,15 @@ static bool zswap_has_pool; /* One swap address space for each 64M swap space */ #define ZSWAP_ADDRESS_SPACE_SHIFT 14 #define ZSWAP_ADDRESS_SPACE_PAGES (1 << ZSWAP_ADDRESS_SPACE_SHIFT) + +static inline struct xarray *zswap_tree(int type, pgoff_t offset) +{ + return &zswap_trees[type][offset >> ZSWAP_ADDRESS_SPACE_SHIFT]; +} + static inline struct xarray *swap_zswap_tree(swp_entry_t swp) { - return &zswap_trees[swp_type(swp)][swp_offset(swp) - >> ZSWAP_ADDRESS_SPACE_SHIFT]; + return zswap_tree(swp_type(swp), swp_offset(swp)); } #define zswap_pool_debug(msg, p) \ @@ -729,6 +734,19 @@ static void zswap_entry_free(struct zswap_entry *entry) atomic_long_dec(&zswap_stored_pages); } +static void __zswap_invalidate(int type, pgoff_t offset) +{ + struct xarray *tree = zswap_tree(type, offset); + struct zswap_entry *entry; + + if (xa_empty(tree)) + return; + + entry = xa_erase(tree, offset); + if (entry) + zswap_entry_free(entry); +} + /********************************* * compressed storage functions **********************************/ @@ -1554,12 +1572,8 @@ bool zswap_store(struct folio *folio) struct zswap_entry *entry; struct xarray *tree; - for (index = 0; index < nr_pages; ++index) { - tree = swap_zswap_tree(swp_entry(type, offset + index)); - entry = xa_erase(tree, offset + index); - if (entry) - zswap_entry_free(entry); - } + for (index = 0; index < nr_pages; ++index) + __zswap_invalidate(type, offset + index); } return ret; @@ -1647,16 +1661,7 @@ int zswap_load(struct folio *folio) void zswap_invalidate(swp_entry_t swp) { - pgoff_t offset = swp_offset(swp); - struct xarray *tree = swap_zswap_tree(swp); - struct zswap_entry *entry; - - if (xa_empty(tree)) - return; - - entry = xa_erase(tree, offset); - if (entry) - zswap_entry_free(entry); + __zswap_invalidate(swp_type(swp), swp_offset(swp)); } int zswap_swapon(int type, unsigned long nr_pages) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() 2026-09-09 14:52 ` Johannes Weiner @ 2026-09-10 1:16 ` Kefeng Wang 0 siblings, 0 replies; 4+ messages in thread From: Kefeng Wang @ 2026-09-10 1:16 UTC (permalink / raw) To: Johannes Weiner Cc: Andrew Morton, linux-mm, Chengming Zhou, Kairui Song, Nhat Pham, Yosry Ahmed On 9/9/2026 10:52 PM, Johannes Weiner wrote: > On Wed, Sep 09, 2026 at 08:35:47PM +0800, Kefeng Wang wrote: >> zswap_store() falls through to check_old and walks the swap xarray >> even when zswap is disabled. Add a zswap_never_enabled() early return >> matching zswap_load(), and reuse zswap_invalidate() whose xa_empty() >> check skips empty per-area trees to avoid unnecessary xarry lockup. >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> Cc: Chengming Zhou <chengming.zhou@linux.dev> >> Cc: Johannes Weiner <hannes@cmpxchg.org> >> Cc: Kairui Song <kasong@tencent.com> >> Cc: Nhat Pham <nphamcs@gmail.com> >> Cc: Yosry Ahmed <yosryahmed@google.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> > > These are really two separate changes. Can you please split them out? Sure. > >> --- >> mm/zswap.c | 16 +++++++--------- >> 1 file changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index 5d0d8bd72193..fc6c5e0db5e4 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -1488,6 +1488,9 @@ bool zswap_store(struct folio *folio) >> VM_WARN_ON_ONCE(!folio_test_locked(folio)); >> VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); >> >> + if (zswap_never_enabled()) >> + return false; > > Reviewed-by: Johannes Weiner <hannes@cmpxchg.org> > Thanks. >> @@ -1545,15 +1548,10 @@ bool zswap_store(struct folio *folio) >> if (!ret) { >> unsigned type = swp_type(swp); >> pgoff_t offset = swp_offset(swp); >> - struct zswap_entry *entry; >> - struct xarray *tree; >> - >> - for (index = 0; index < nr_pages; ++index) { >> - tree = swap_zswap_tree(swp_entry(type, offset + index)); >> - entry = xa_erase(tree, offset + index); >> - if (entry) >> - zswap_entry_free(entry); >> - } >> + >> + for (index = 0; index < nr_pages; ++index) >> + zswap_invalidate(swp_entry(type, offset + index)); > > That dance through a swp_entry_t was already kind of awful. This would > be a good opportunity to refactor things to avoid that: It looks better, thanks for your review. will update. > > diff --git a/mm/zswap.c b/mm/zswap.c > index e6ec3295bdb0..df18fdaae703 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -228,10 +228,15 @@ static bool zswap_has_pool; > /* One swap address space for each 64M swap space */ > #define ZSWAP_ADDRESS_SPACE_SHIFT 14 > #define ZSWAP_ADDRESS_SPACE_PAGES (1 << ZSWAP_ADDRESS_SPACE_SHIFT) > + > +static inline struct xarray *zswap_tree(int type, pgoff_t offset) > +{ > + return &zswap_trees[type][offset >> ZSWAP_ADDRESS_SPACE_SHIFT]; > +} > + > static inline struct xarray *swap_zswap_tree(swp_entry_t swp) > { > - return &zswap_trees[swp_type(swp)][swp_offset(swp) > - >> ZSWAP_ADDRESS_SPACE_SHIFT]; > + return zswap_tree(swp_type(swp), swp_offset(swp)); > } > > #define zswap_pool_debug(msg, p) \ > @@ -729,6 +734,19 @@ static void zswap_entry_free(struct zswap_entry *entry) > atomic_long_dec(&zswap_stored_pages); > } > > +static void __zswap_invalidate(int type, pgoff_t offset) > +{ > + struct xarray *tree = zswap_tree(type, offset); > + struct zswap_entry *entry; > + > + if (xa_empty(tree)) > + return; > + > + entry = xa_erase(tree, offset); > + if (entry) > + zswap_entry_free(entry); > +} > + > /********************************* > * compressed storage functions > **********************************/ > @@ -1554,12 +1572,8 @@ bool zswap_store(struct folio *folio) > struct zswap_entry *entry; > struct xarray *tree; > > - for (index = 0; index < nr_pages; ++index) { > - tree = swap_zswap_tree(swp_entry(type, offset + index)); > - entry = xa_erase(tree, offset + index); > - if (entry) > - zswap_entry_free(entry); > - } > + for (index = 0; index < nr_pages; ++index) > + __zswap_invalidate(type, offset + index); > } > > return ret; > @@ -1647,16 +1661,7 @@ int zswap_load(struct folio *folio) > > void zswap_invalidate(swp_entry_t swp) > { > - pgoff_t offset = swp_offset(swp); > - struct xarray *tree = swap_zswap_tree(swp); > - struct zswap_entry *entry; > - > - if (xa_empty(tree)) > - return; > - > - entry = xa_erase(tree, offset); > - if (entry) > - zswap_entry_free(entry); > + __zswap_invalidate(swp_type(swp), swp_offset(swp)); > } > > int zswap_swapon(int type, unsigned long nr_pages) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-10 1:16 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-09 12:35 [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store() Kefeng Wang 2026-09-09 12:53 ` Kefeng Wang 2026-09-09 14:52 ` Johannes Weiner 2026-09-10 1:16 ` Kefeng Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.