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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

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)