Re: [PATCH v5 04/11] mm: zswap: add range lookup for large-folio swapin

[Usama Arif]

On 23/07/2026 17:42, Yosry Ahmed wrote:
>>>> @@ -1595,13 +1611,19 @@ int zswap_load(struct folio *folio)
>>>>              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.
>>>> +     * A large folio reaches zswap_load() only when its whole range is
>>>> +     * expected to be on disk: PMD swap-entry consumers split before
>>>> +     * calling into PMD-order swapin whenever any slot is still in zswap.
>>>> +     * Confirm the range is entirely absent from zswap and return -ENOENT
>>>> +     * so the caller reads it from disk; if a slot is unexpectedly still in
>>>> +     * zswap, fail the read rather than return partially-initialized data.
>>>>       */
>>>> -    if (WARN_ON_ONCE(folio_test_large(folio))) {
>>>> -            folio_unlock(folio);
>>>> -            return -EINVAL;
>>>> +    if (folio_test_large(folio)) {
>>>> +            if (zswap_is_present(swp, folio_nr_pages(folio))) {
>>>
>>> Is dropping the warning here intentional (for the folio_test_large() &&
>>> zswap_is_present() case)?
>>>
>>
>>
>> Yes, so we can end up in a race, which should be handled gracefully.
>>
>> For example, lets say we have zswap writeback enabled, which means we
>> can end up in a state where we have a PMD swap entry and 511 of the 512
>> slots have been written to disk, but 1 slot (slot X) is still in zswap.
>>
>> We can then have the following race:
>>
>>
>>   CPU A: PMD swap-in                  CPU B: split-PTE swap-in
>>   ------------------                 ------------------------
>>
>>   Checks swap cache: empty
>>
>>                                        Faults slot X
>>                                        Adds order-0 folio F
>>                                        zswap_load(F):
>>                                          removes X from zswap
>>                                          marks F dirty
>>
>>   Checks zswap range: empty
>>   A is preempted
>>
>>                                        Unmaps/reclaims F
>>                                        zswap_store(F):
>>                                          puts X back in zswap
>>                                        Removes F from swap cache
>>
>>   A resumes
>>   Allocates large swap-cache folio G
>>   zswap_load(G) finds X in zswap
> 
> Why don't we check the zswap range after allocating a folio in the
> swap cache? I am assuming at this point we have the folio locked and
> the result should be stable?
> 

Yes that makes sense. The folio is locked and the result will be stable.
How about something like below?

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 1f08fb522036..41f168377cb1 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -12,6 +12,7 @@
 #include <linux/kernel_stat.h>
 #include <linux/mempolicy.h>
 #include <linux/swap.h>
+#include <linux/zswap.h>
 #include <linux/leafops.h>
 #include <linux/init.h>
 #include <linux/pagemap.h>
@@ -500,6 +501,17 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
        __folio_set_locked(folio);
        __folio_set_swapbacked(folio);
        __swap_cache_do_add_folio(ci, folio, entry);
+       /*
+        * Reject mixed zswap/disk backing before starting the read.
+        */
+       if (order && zswap_is_present(entry, nr_pages)) {
+               __swap_cache_do_del_folio(ci, folio, entry, shadow);
+               spin_unlock(&ci->lock);
+               folio_unlock(folio);
+               /* nr_pages refs from swap cache, 1 from allocation */
+               folio_put_refs(folio, nr_pages + 1);
+               return ERR_PTR(-EBUSY);
+       }
        spin_unlock(&ci->lock);
 
        if (mem_cgroup_swapin_charge_folio(folio, memcg_id,


>>
>> The correct action would be to reject the PMD order read and fall back
>> to per-page loading.
>>
>> I am bit torn about what to do for zswap here. The series is quite big
>> already. Alexandre is looking at adding support for PMD swap, but will
>> send patches once this series gets merged. My initial versions 1
>> and 2, basically stopped installing PMD swap entry if zswap was ever enabled.
>> From v3, I used Alexandre's suggestion to do zswap_is_present() test
>> so that PMD swap entry can keep on working.
>>
>> Both of these paths are temporary till Alex sends his series. I
>> do feel my initial version was simpler but will basically stop
>> working if zswap is ever enabled. Do you have any suggestions on
>> what your preference is for zswap?
> 
> Even with PMD zswap load support, we still have to deal with the case
> where some order-0 pages are in zswap and some are on disk, right? I
> don't see how that would eliminate the case you mentioned above.

Ack, yeah writeback will still mess with things. I will keep the current
implementation.