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

[Usama Arif]

On 27/07/2026 17:24, Nhat Pham wrote:
> On Fri, Jul 24, 2026 at 2:59 AM Usama Arif <usama.arif@xxxxxxxxx> wrote:
>>
>>
>>
>> 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);
>> +       }
> 
> If you move it to __swap_cache_add_check(), you can potentially
> backoff even before the folio allocation? :)


Thanks Nhat! Yes, this looks much much better as well:

@@ -190,6 +191,15 @@ static int __swap_cache_add_check(struct swap_cluster_info *ci,
        if (nr == 1)
                return 0;
 
+       /*
+        * The cluster lock serializes swap-cache insertion with zswap
+        * writeback. Reject mixed zswap/disk backing before allocating a
+        * large folio and recheck it before adding the folio to swap cache.
+        */
+       if (zswap_is_present(swp_entry(swp_type(targ_entry),
+                                      round_down(swp_offset(targ_entry), nr)), nr))
+               return -EBUSY;