Re: [PATCH] mm: zswap: avoid unnecessary xarray lookup in zswap_store()

[Johannes Weiner <hannes@cmpxchg.org>]

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)