mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Fujunjie <fujunjie1@qq.com>
To: Nhat Pham <nphamcs@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Alexandre Ghiti <alexghiti@meta.com>,
	Kairui Song <kasong@tencent.com>,
	Usama Arif <usamaarif642@gmail.com>, Chris Li <chrisl@kernel.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Yosry Ahmed <yosry@kernel.org>,
	David Hildenbrand <david@kernel.org>,
	Hugh Dickins <hughd@google.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	linux-kernel@vger.kernel.org, cgroups@vger.kernel.org
Subject: Re: [RFC PATCH v2 3/9] mm/zswap: support fully zswap-backed large folio loads
Date: Mon, 1 Jun 2026 04:03:24 +0800	[thread overview]
Message-ID: <tencent_A78A28F31A1ECA231A375216585C77CFF308@qq.com> (raw)
In-Reply-To: <CAKEwX=Or6forBoArv1b=MZuhOuF+MTuLLZWPKgUmkBVaoBoYSQ@mail.gmail.com>



On 5/30/2026 2:25 AM, Nhat Pham wrote:
> On Fri, May 29, 2026 at 5:19 AM fujunjie <fujunjie1@qq.com> wrote:
>>
>> zswap currently refuses large swapcache folios. That is correct for mixed
>> backend ranges, but it also prevents the common swapin path from loading a
>> range that is still fully backed by zswap.
>>
>> Teach zswap_load() to fill a locked large swapcache folio by decompressing
>> each base-page entry into the matching folio offset, then flushing the
>> folio once. A missing entry after zswap data has been seen is reported as
>> -EAGAIN so the caller can drop the speculative large folio and retry
>> order-0.
>>
>> The large load keeps the zswap entries in place. It is a clean speculative
>> fill: until the swap slots are freed, zswap remains the backing copy if
>> reclaim drops the large folio before PTEs are installed.
>>
>> Signed-off-by: fujunjie <fujunjie1@qq.com>
>> ---
>>  mm/zswap.c | 105 ++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 87 insertions(+), 18 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index da5297f7bd69..94ba112a2982 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -15,6 +15,8 @@
>>
>>  #include <linux/module.h>
>>  #include <linux/cpu.h>
>> +#include <linux/mm.h>
>> +#include <linux/huge_mm.h>
>>  #include <linux/highmem.h>
>>  #include <linux/slab.h>
>>  #include <linux/spinlock.h>
>> @@ -934,7 +936,8 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
>>         return comp_ret == 0 && alloc_ret == 0;
>>  }
>>
>> -static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>> +static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio,
>> +                            unsigned int page_idx, bool flush_dcache)
>>  {
>>         struct zswap_pool *pool = entry->pool;
>>         struct scatterlist input[2]; /* zsmalloc returns an SG list 1-2 entries */
>> @@ -952,14 +955,15 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>>
>>                 WARN_ON_ONCE(input->length != PAGE_SIZE);
>>
>> -               dst = kmap_local_folio(folio, 0);
>> +               dst = kmap_local_folio(folio, page_idx * PAGE_SIZE);
>>                 memcpy_from_sglist(dst, input, 0, PAGE_SIZE);
>>                 dlen = PAGE_SIZE;
>>                 kunmap_local(dst);
>> -               flush_dcache_folio(folio);
>> +               if (flush_dcache)
>> +                       flush_dcache_folio(folio);
>>         } else {
>>                 sg_init_table(&output, 1);
>> -               sg_set_folio(&output, folio, PAGE_SIZE, 0);
>> +               sg_set_folio(&output, folio, PAGE_SIZE, page_idx * PAGE_SIZE);
>>                 acomp_request_set_params(acomp_ctx->req, input, &output,
>>                                          entry->length, PAGE_SIZE);
>>                 ret = crypto_acomp_decompress(acomp_ctx->req);
>> @@ -1042,7 +1046,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>>                 goto out;
>>         }
>>
>> -       if (!zswap_decompress(entry, folio)) {
>> +       if (!zswap_decompress(entry, folio, 0, true)) {
>>                 ret = -EIO;
>>                 goto out;
>>         }
>> @@ -1615,10 +1619,9 @@ enum zswap_range_state zswap_probe_range(swp_entry_t swp,
>>   *  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).
>> + *  -EAGAIN: if the swapped out content belongs to a large folio, but the
>> + *  range is mixed or raced with writeback. The folio remains locked so the
>> + *  caller can drop the large swapcache folio and retry order-0.
>>   *
>>   *  -ENOENT: if the swapped out content was not in zswap. The folio remains
>>   *  locked on return.
>> @@ -1626,9 +1629,12 @@ enum zswap_range_state zswap_probe_range(swp_entry_t swp,
>>  int zswap_load(struct folio *folio)
>>  {
>>         swp_entry_t swp = folio->swap;
>> +       unsigned int nr_pages = folio_nr_pages(folio);
>> +       unsigned int type = swp_type(swp);
>>         pgoff_t offset = swp_offset(swp);
>> -       struct xarray *tree = swap_zswap_tree(swp);
>> +       struct xarray *tree;
>>         struct zswap_entry *entry;
>> +       unsigned int i;
>>
>>         VM_WARN_ON_ONCE(!folio_test_locked(folio));
>>         VM_WARN_ON_ONCE(!folio_test_swapcache(folio));
>> @@ -1636,21 +1642,84 @@ 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 (folio_test_large(folio)) {
>> +               struct obj_cgroup *first_objcg = NULL;
>> +               bool same_objcg = true;
>> +               bool saw_zswap = false;
>> +               bool saw_non_zswap = false;
>> +
>> +               /*
>> +                * The locked large swapcache folio now covers the range and
>> +                * conflicts with zswap writeback's order-0 swapcache allocation.
>> +                * If the range is mixed or an entry disappears, retry order-0.
>> +                */
>> +               for (i = 0; i < nr_pages; i++) {
>> +                       tree = swap_zswap_tree(swp_entry(type, offset + i));
>> +                       entry = xa_load(tree, offset + i);
>> +                       if (!entry) {
>> +                               if (saw_zswap)
>> +                                       return -EAGAIN;
>> +                               saw_non_zswap = true;
>> +                               continue;
>> +                       }
> 
> Can we use xas_load API here instead of traversing down the tree again
> and again?

I'll rework it to use xas_load(), while handling zswap tree boundaries correctly.

> 
>> +                       if (saw_non_zswap)
>> +                               return -EAGAIN;
>> +
>> +                       if (!saw_zswap)
>> +                               first_objcg = entry->objcg;
>> +                       else if (entry->objcg != first_objcg)
>> +                               same_objcg = false;
> 
> Can we get different objcg at this point?

The objcg pointers can be different in principle, for example if
the range is assembled from entries that came from different per-node objcgs
of the same memcg.

But for this accounting path, count_objcg_events() ultimately charges the
event to obj_cgroup_memcg(entry->objcg). Since the large swapcache allocation
has already checked compatible swap ownership for the range, the final memcg
accounting target should be the same even if the objcg pointers differ.

I will simplify this in v3 and avoid the extra objcg equality pass.

> 
>> +                       saw_zswap = true;
>> +               }
>> +               if (!saw_zswap)
>> +                       return -ENOENT;
>> +
>> +               for (i = 0; i < nr_pages; i++) {
>> +                       tree = swap_zswap_tree(swp_entry(type, offset + i));
>> +                       entry = xa_load(tree, offset + i);
>> +                       if (!entry)
>> +                               return -EAGAIN;
>> +
>> +                       if (!zswap_decompress(entry, folio, i, false)) {
>> +                               folio_unlock(folio);
>> +                               return -EIO;
>> +                       }
>> +               }
>> +
>> +               flush_dcache_folio(folio);
>> +               /*
>> +                * Keep zswap entries until swap slots are freed. This is a clean
>> +                * speculative fill; zswap remains the backing copy if reclaim
>> +                * drops the large folio before PTEs are installed.
>> +                */
>> +               folio_mark_uptodate(folio);
>> +               count_vm_events(ZSWPIN, nr_pages);
>> +               count_mthp_stat(folio_order(folio), MTHP_STAT_SWPIN);
>> +
>> +               if (same_objcg) {
>> +                       if (first_objcg)
>> +                               count_objcg_events(first_objcg, ZSWPIN, nr_pages);
>> +               } else {
>> +                       for (i = 0; i < nr_pages; i++) {
>> +                               tree = swap_zswap_tree(swp_entry(type, offset + i));
>> +                               entry = xa_load(tree, offset + i);
>> +                               if (WARN_ON_ONCE(!entry))
>> +                                       continue;
>> +                               if (entry->objcg)
>> +                                       count_objcg_events(entry->objcg, ZSWPIN, 1);
> 
> xas_load() here too?

Yes, same issue here. 

> 
> 
>> +                       }
>> +               }
>> +
>>                 folio_unlock(folio);
>> -               return -EINVAL;
>> +               return 0;
>>         }
> 
>>
>> +       tree = swap_zswap_tree(swp);
>>         entry = xa_load(tree, offset);
>>         if (!entry)
>>                 return -ENOENT;
>>
>> -       if (!zswap_decompress(entry, folio)) {
>> +       if (!zswap_decompress(entry, folio, 0, true)) {
>>                 folio_unlock(folio);
>>                 return -EIO;
>>         }
> 
> I wonder how much of these two paths (order 0 and larger order) can be
> unified...

I think more of this can be unified than this version does.

I split the paths this way because I treated the large-folio load as a
speculative fill and kept the zswap entries as the backing copy. But with
your point that an installed large swapcache folio should block zswap
writeback from turning the range mixed, I should revisit that completion rule
instead of baking it into a separate path.

For the v3 version I will try to collapse the common load path. If the large-folio
case still needs different entry lifetime rules, I will make that distinction
explicit.

> 
>> --
>> 2.34.1
>>



  reply	other threads:[~2026-05-31 20:03 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-29 12:17 [RFC PATCH v2 0/9] mm: support zswap-backed large folio swapin fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 1/9] mm/zswap: expose range state for swapin policy fujunjie
2026-05-29 18:35   ` Nhat Pham
2026-05-31 13:47     ` Fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 2/9] mm: let swap_read_folio() report retryable zswap races fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 3/9] mm/zswap: support fully zswap-backed large folio loads fujunjie
2026-05-29 18:25   ` Nhat Pham
2026-05-31 20:03     ` Fujunjie [this message]
2026-05-29 12:19 ` [RFC PATCH v2 4/9] mm: admit large swapin by backend range in swapin_sync() fujunjie
2026-05-29 14:43   ` Kairui Song
2026-05-29 14:45     ` Kairui Song
2026-05-31 12:34       ` Fujunjie
2026-05-31 12:21     ` Fujunjie
2026-05-29 18:34   ` Nhat Pham
2026-05-31 13:15     ` Fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 5/9] mm: add common locality admission for zswap large swapin fujunjie
2026-05-29 19:00   ` Nhat Pham
2026-06-01  9:47     ` Fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 6/9] mm: provide anon locality evidence " fujunjie
2026-05-29 19:22   ` Nhat Pham
2026-05-31 13:11     ` Fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 7/9] mm/shmem: provide VMA-hint locality " fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 8/9] mm: try all-zswap large swapin within swap readahead windows fujunjie
2026-05-29 12:19 ` [RFC PATCH v2 9/9] docs: mm: update THP swapin counter descriptions fujunjie
2026-05-29 18:37   ` Nhat Pham
2026-05-31 13:21     ` Fujunjie
2026-05-29 18:06 ` [RFC PATCH v2 0/9] mm: support zswap-backed large folio swapin Nhat Pham
2026-05-31 12:32   ` Fujunjie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_A78A28F31A1ECA231A375216585C77CFF308@qq.com \
    --to=fujunjie1@qq.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexghiti@meta.com \
    --cc=cgroups@vger.kernel.org \
    --cc=chrisl@kernel.org \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=usamaarif642@gmail.com \
    --cc=yosry@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®