Re: [RFC PATCH 3/5] mm: zswap: load fully stored large folios

[Yosry Ahmed <yosry@kernel.org>]

On Fri, May 08, 2026 at 08:20:31PM +0000, fujunjie wrote:
> zswap_store() already stores every base page of a large folio as a
> separate zswap entry and tears the whole folio back down on store
> failure. The load side still rejects any large folio, which forces the
> swapin path to avoid mTHP swapin once zswap has ever been enabled.
> 
> Use zswap_entry_batch() to distinguish three cases: the whole range is
> absent from zswap and should fall through to the disk backend, the whole
> range is present and can be decompressed one base page at a time, or the
> range is mixed and must be treated as an invalid large-folio backend
> selection.
> 
> After all entries decompress successfully, mark the folio uptodate and
> dirty, account the mTHP swpin stat once for the folio, account one ZSWPIN
> event per base page, and invalidate each zswap entry because the
> swapcache folio becomes authoritative.
> 
> Signed-off-by: fujunjie <fujunjie1@qq.com>
> ---
>  Documentation/admin-guide/mm/transhuge.rst |  4 +-
>  mm/zswap.c                                 | 65 ++++++++++++++--------
>  2 files changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/transhuge.rst b/Documentation/admin-guide/mm/transhuge.rst
> index 5fbc3d89bb07..05456906aff6 100644
> --- a/Documentation/admin-guide/mm/transhuge.rst
> +++ b/Documentation/admin-guide/mm/transhuge.rst
> @@ -644,8 +644,8 @@ zswpout
>  	piece without splitting.
>  
>  swpin
> -	is incremented every time a huge page is swapped in from a non-zswap
> -	swap device in one piece.
> +	is incremented every time a huge page is swapped in from swap I/O or
> +	zswap in one piece.
>  
>  swpin_fallback
>  	is incremented if swapin fails to allocate or charge a huge page
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 27c14b8edd15..863ca1e896ed 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -28,6 +28,7 @@
>  #include <crypto/acompress.h>
>  #include <crypto/scatterwalk.h>
>  #include <linux/zswap.h>
> +#include <linux/huge_mm.h>
>  #include <linux/mm_types.h>
>  #include <linux/page-flags.h>
>  #include <linux/swapops.h>
> @@ -1614,20 +1615,23 @@ bool zswap_store(struct folio *folio)
>   *  NOT marked up-to-date, so that an IO error is emitted (e.g. do_swap_page()
>   *  will SIGBUS).
>   *
> - *  -EINVAL: if the swapped out content was in zswap, but the page belongs
> - *  to a large folio, which is not supported by zswap. The folio is unlocked,
> - *  but NOT marked up-to-date, so that an IO error is emitted (e.g.
> - *  do_swap_page() will SIGBUS).
> + *  -EINVAL: if the folio spans a mix of zswap and non-zswap entries. The
> + *  folio is unlocked, but NOT marked up-to-date, so that an IO error is
> + *  emitted (e.g. do_swap_page() will SIGBUS). Large folio swapin should
> + *  reject such ranges before calling zswap_load().
>   *
> - *  -ENOENT: if the swapped out content was not in zswap. The folio remains
> + *  -ENOENT: if the swapped out content was not in zswap. For a large folio,
> + *  this means the whole folio range was not in zswap. The folio remains
>   *  locked on return.
>   */
>  int zswap_load(struct folio *folio)
>  {
>  	swp_entry_t swp = folio->swap;
>  	pgoff_t offset = swp_offset(swp);
> -	struct xarray *tree = swap_zswap_tree(swp);
>  	struct zswap_entry *entry;
> +	int nr_pages = folio_nr_pages(folio);
> +	bool is_zswap;
> +	int index;
>  
>  	VM_WARN_ON_ONCE(!folio_test_locked(folio));
>  	VM_WARN_ON_ONCE(!folio_test_swapcache(folio));
> @@ -1635,30 +1639,36 @@ int zswap_load(struct folio *folio)
>  	if (zswap_never_enabled())
>  		return -ENOENT;
>  
> -	/*
> -	 * Large folios should not be swapped in while zswap is being used, as
> -	 * they are not properly handled. Zswap does not properly load large
> -	 * folios, and a large folio may only be partially in zswap.
> -	 */
> -	if (WARN_ON_ONCE(folio_test_large(folio))) {
> +	if (zswap_entry_batch(swp, nr_pages, &is_zswap) != nr_pages) {
> +		WARN_ON_ONCE(folio_test_large(folio));

IIUC the condition can only be true for large folios, so I think just
WARN_ON_ONCE() in the if statement itself?

Taking a step back, this is doing xa_load() lookups that are repeated
again below. Maybe we should drop this check here and integrate the
check into the loop below (either all entries exist or don't)?

We might end up with a case where we decompress some parts of the folio
then return a failure, but this possibility already exists (e.g.
decompression failures).

>  		folio_unlock(folio);
>  		return -EINVAL;
>  	}
>  
> -	entry = xa_load(tree, offset);
> -	if (!entry)
> +	if (!is_zswap)
>  		return -ENOENT;
>  
> -	if (!zswap_decompress(entry, folio, 0)) {
> -		folio_unlock(folio);
> -		return -EIO;
> +	for (index = 0; index < nr_pages; index++) {
> +		swp_entry_t entry_swp = swp_entry(swp_type(swp),
> +						  offset + index);
> +		struct xarray *tree = swap_zswap_tree(entry_swp);
> +
> +		entry = xa_load(tree, offset + index);
> +		if (WARN_ON_ONCE(!entry)) {
> +			folio_unlock(folio);
> +			return -EINVAL;
> +		}
> +
> +		if (!zswap_decompress(entry, folio, index)) {
> +			folio_unlock(folio);
> +			return -EIO;
> +		}
>  	}
>  
>  	folio_mark_uptodate(folio);
>  
> -	count_vm_event(ZSWPIN);
> -	if (entry->objcg)
> -		count_objcg_events(entry->objcg, ZSWPIN, 1);
> +	count_mthp_stat(folio_order(folio), MTHP_STAT_SWPIN);

Not a problem with this patch, but I wonder why different swapin
implementations are making this call instead of putting it in a
higher-level (e.g. alloc_swap_folio()).

> +	count_vm_events(ZSWPIN, nr_pages);
>  
>  	/*
>  	 * We are reading into the swapcache, invalidate zswap entry.
> @@ -1668,8 +1678,19 @@ int zswap_load(struct folio *folio)
>  	 * compression work.
>  	 */
>  	folio_mark_dirty(folio);
> -	xa_erase(tree, offset);
> -	zswap_entry_free(entry);
> +
> +	for (index = 0; index < nr_pages; index++) {
> +		swp_entry_t entry_swp = swp_entry(swp_type(swp),
> +						  offset + index);
> +		struct xarray *tree = swap_zswap_tree(entry_swp);
> +
> +		entry = xa_erase(tree, offset + index);
> +		if (WARN_ON_ONCE(!entry))
> +			continue;
> +		if (entry->objcg)
> +			count_objcg_events(entry->objcg, ZSWPIN, 1);

Hmm I was wondering how the stat update should be handled here. It is
possible that a folio was swapped out from one memcg and is being
swapped in by a process in a different memcg. For order-0 folios, the
folio gets charged to the swapout memcg.

However, looking at alloc_swap_folio() ->
mem_cgroup_swapin_charge_folio(), it seems like we charge mTHP folios to
the swapout memcg of the first swap entry. This seems a bit arbitrary.

Focusing at the code here, seems like we'll count the swapin against the
swapout memcg, which is good in terms of keeping ZSWPIN and ZSWPOUT
balanced. However, it seems like it's possible that the folio is being
charged to a different memcg here than the swapout memcg, so we may end
up counting ZSWPIN in one memcg but charging the folio in another.

I am not sure if this is practically a problem, but something doesn't
sound right. Johannes (and other memcg folks), were these the intended
charging semantics for mTHP swapin? Is it reasonable to count ZSWPIN in
one memcg and charge the folio in another?

> +		zswap_entry_free(entry);
> +	}
>  
>  	folio_unlock(folio);
>  	return 0;
> -- 
> 2.34.1
> 
>