Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap

[Usama Arif]

On 10/09/2026 03:47, Andrew Morton wrote:
> On Mon,  7 Sep 2026 09:19:38 -0700 Usama Arif <usama.arif@xxxxxxxxx> wrote:
> 
>> thp_swapin_suitable_orders() and shmem_swap_alloc_folio() sample
>> zswap_never_enabled() to decide whether a swapin may use a large folio.
>> zswap_load() samples the same one-way static key again once the read
>> reaches it.  Nothing serialises the two reads, and in between the task
>> allocates and pins a high-order folio, which can sleep.
>>
>> If zswap is enabled for the first time in that window, a large folio that
>> was correctly permitted reaches zswap_load(), which rejects every large
>> folio with -EINVAL.  swap_read_folio() treats anything other than -ENOENT
>> as "zswap handled it" and skips the backing-device read, so the folio
>> comes back unlocked and not uptodate: SIGBUS for an anonymous fault, -EIO
>> for shmem.  The data is intact on the swap device - it was written there
>> before zswap was ever enabled - and the not-uptodate folio stays in the
>> swap cache, so every retry of the fault fails the same way.  With
>> panic_on_warn the WARN takes the machine down rather than the task.
>>
>> Scan the range instead of rejecting the folio.  The caller has pinned
>> every slot before issuing the read, so zswap cannot start a store or a
>> writeback into the range and the scan is stable.  If nothing in the range
>> is in zswap it is all on the backing device: return -ENOENT and let
>> swap_read_folio() read it.
>>
>> A range that does have a slot in zswap is still refused, because zswap
>> stores large folios as order-0 entries and cannot reconstruct one.  That
>> stays reachable - a slot shared with another task can be stored inside
>> the same window - and refusing is correct, since the alternative is
>> returning the stale device copy.  Report it as -EIO rather than -EINVAL:
>> the request is valid, zswap just cannot serve it.  The only caller
>> distinguishes -ENOENT from everything else, so that part is a
>> documentation fix.
> 
> So to hit this bug the user needs to enable zswap system-wide during a
> teeny race window in the swapin code?
> 
> I suspect nobody has ever hit this and couldn't do so if they tried?


Yes, I think it would be very very difficult to hit this.

It was part of my PMD swap series, where its actually needed for the feature.

I think we can drop cc:stable, unless you think its needed Yosry?
> 
>> Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
>> Cc: stable@xxxxxxxxxxxxxxx
> 
> 
> Documentation/process/stable-kernel-rules.rst, with which I agree:
> 
> Rules on what kind of patches are accepted, and which ones are not, into the
> "-stable" tree:
> 
> - It or an equivalent fix must already exist in Linux mainline (upstream).
> - It must be obviously correct and tested.
> - It cannot be bigger than 100 lines, with context.
> - It must follow the
>   :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
>   rules.
> - It must either fix a real bug that bothers people or just add a device ID.
>   To elaborate on the former:
> 
>   - It fixes a problem like an oops, a hang, data corruption, a real security
>     issue, a hardware quirk, a build error (but not for things marked
>     CONFIG_BROKEN), or some "oh, that's not good" issue.
>   - Serious issues as reported by a user of a distribution kernel may also
>     be considered if they fix a notable performance or interactivity issue.
>     As these fixes are not as obvious and have a higher risk of a subtle
>     regression they should only be submitted by a distribution kernel
>     maintainer and include an addendum linking to a bugzilla entry if it
>     exists and additional information on the user-visible impact.
>   - No "This could be a problem..." type of things like a "theoretical race
>     condition", unless an explanation of how the bug can be exploited is also
>     provided.
>   - No "trivial" fixes without benefit for users (spelling changes, whitespace
>     cleanups, etc).
> 
> 
> If this patch meets the above then its changelog needs an update!