All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
@ 2026-09-07 16:19 Usama Arif
  2026-09-08  8:10 ` Yosry Ahmed
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Usama Arif @ 2026-09-07 16:19 UTC (permalink / raw)
  To: yosry, Andrew Morton, chengming.zhou, hannes, linux-kernel,
	linux-mm, nphamcs
  Cc: kernel-team, Usama Arif, stable, Alexandre Ghiti

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.

Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
Cc: stable@vger.kernel.org
Co-developed-by: Alexandre Ghiti <alex@ghiti.fr>
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 mm/zswap.c | 58 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 37f34e406c8e3..fd36ac38e9a1e 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1571,6 +1571,32 @@ bool zswap_store(struct folio *folio)
 	return ret;
 }
 
+/**
+ * zswap_is_present() - is any slot in [entry, entry + nr) in zswap?
+ * @entry: base swap entry of the range
+ * @nr: number of contiguous slots to check
+ *
+ * Context: The caller must keep the range pinned, otherwise the answer can
+ * change under it.
+ * Return: true if at least one slot in the range is in zswap.
+ */
+static bool zswap_is_present(swp_entry_t entry, unsigned int nr)
+{
+	pgoff_t offset = swp_offset(entry);
+	struct xarray *tree = swap_zswap_tree(entry);
+	unsigned long index = offset;
+
+	/*
+	 * A pinned range is at most SWAPFILE_CLUSTER slots and is aligned to
+	 * its own size, so one tree covers all of it and a single lookup is
+	 * enough. Scanning only part of the range would report a false
+	 * "absent" and let the caller read a stale copy from the device.
+	 */
+	BUILD_BUG_ON(SWAPFILE_CLUSTER > ZSWAP_ADDRESS_SPACE_PAGES);
+
+	return xa_find(tree, &index, offset + nr - 1, XA_PRESENT);
+}
+
 /**
  * zswap_load() - load a folio from zswap
  * @folio: folio to load
@@ -1578,15 +1604,12 @@ bool zswap_store(struct folio *folio)
  * Return: 0 on success, with the folio unlocked and marked up-to-date, or one
  * of the following error codes:
  *
- *  -EIO: if the swapped out content was in zswap, but could not be loaded
- *  into the page due to a decompression failure. 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 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).
+ *  -EIO: if the swapped out content was in zswap but could not be handed
+ *  back, either because decompression failed or because a slot in a
+ *  large-folio range is still in zswap and zswap cannot reconstruct a large
+ *  folio from per-page 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).
  *
  *  -ENOENT: if the swapped out content was not in zswap. The folio remains
  *  locked on return.
@@ -1605,13 +1628,18 @@ 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 can legitimately reach zswap_load() with its whole
+	 * range on the backing device, so scan the range rather than rejecting
+	 * it outright. The caller has pinned every slot, so zswap cannot start
+	 * a store or a writeback into the range while we look.
 	 */
-	if (WARN_ON_ONCE(folio_test_large(folio))) {
-		folio_unlock(folio);
-		return -EINVAL;
+	if (folio_test_large(folio)) {
+		if (WARN_ON_ONCE(zswap_is_present(swp,
+						  folio_nr_pages(folio)))) {
+			folio_unlock(folio);
+			return -EIO;
+		}
+		return -ENOENT;
 	}
 
 	entry = xa_load(tree, offset);
-- 
2.53.0-Meta



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
  2026-09-07 16:19 [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap Usama Arif
@ 2026-09-08  8:10 ` Yosry Ahmed
  2026-09-09 18:14 ` Nhat Pham
  2026-09-10  2:47 ` Andrew Morton
  2 siblings, 0 replies; 6+ messages in thread
From: Yosry Ahmed @ 2026-09-08  8:10 UTC (permalink / raw)
  To: Usama Arif
  Cc: Andrew Morton, chengming.zhou, hannes, linux-kernel, linux-mm,
	nphamcs, kernel-team, stable, Alexandre Ghiti, Longlong Xia

On Mon, Sep 7, 2026 at 9:20 AM Usama Arif <usama.arif@linux.dev> 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.
>
> Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
> Cc: stable@vger.kernel.org
> Co-developed-by: Alexandre Ghiti <alex@ghiti.fr>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

Acked-by: Yosry Ahmed <yosry@kernel.org>


> ---
>  mm/zswap.c | 58 ++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 37f34e406c8e3..fd36ac38e9a1e 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1571,6 +1571,32 @@ bool zswap_store(struct folio *folio)
>         return ret;
>  }
>
> +/**
> + * zswap_is_present() - is any slot in [entry, entry + nr) in zswap?
> + * @entry: base swap entry of the range
> + * @nr: number of contiguous slots to check
> + *
> + * Context: The caller must keep the range pinned, otherwise the answer can
> + * change under it.
> + * Return: true if at least one slot in the range is in zswap.
> + */
> +static bool zswap_is_present(swp_entry_t entry, unsigned int nr)
> +{
> +       pgoff_t offset = swp_offset(entry);
> +       struct xarray *tree = swap_zswap_tree(entry);
> +       unsigned long index = offset;
> +
> +       /*
> +        * A pinned range is at most SWAPFILE_CLUSTER slots and is aligned to
> +        * its own size, so one tree covers all of it and a single lookup is
> +        * enough. Scanning only part of the range would report a false
> +        * "absent" and let the caller read a stale copy from the device.
> +        */
> +       BUILD_BUG_ON(SWAPFILE_CLUSTER > ZSWAP_ADDRESS_SPACE_PAGES);
> +
> +       return xa_find(tree, &index, offset + nr - 1, XA_PRESENT);
> +}
> +
>  /**
>   * zswap_load() - load a folio from zswap
>   * @folio: folio to load
> @@ -1578,15 +1604,12 @@ bool zswap_store(struct folio *folio)
>   * Return: 0 on success, with the folio unlocked and marked up-to-date, or one
>   * of the following error codes:
>   *
> - *  -EIO: if the swapped out content was in zswap, but could not be loaded
> - *  into the page due to a decompression failure. 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 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).
> + *  -EIO: if the swapped out content was in zswap but could not be handed
> + *  back, either because decompression failed or because a slot in a
> + *  large-folio range is still in zswap and zswap cannot reconstruct a large
> + *  folio from per-page 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).
>   *
>   *  -ENOENT: if the swapped out content was not in zswap. The folio remains
>   *  locked on return.
> @@ -1605,13 +1628,18 @@ 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 can legitimately reach zswap_load() with its whole
> +        * range on the backing device, so scan the range rather than rejecting
> +        * it outright. The caller has pinned every slot, so zswap cannot start
> +        * a store or a writeback into the range while we look.
>          */
> -       if (WARN_ON_ONCE(folio_test_large(folio))) {
> -               folio_unlock(folio);
> -               return -EINVAL;
> +       if (folio_test_large(folio)) {
> +               if (WARN_ON_ONCE(zswap_is_present(swp,
> +                                                 folio_nr_pages(folio)))) {
> +                       folio_unlock(folio);
> +                       return -EIO;
> +               }
> +               return -ENOENT;
>         }
>
>         entry = xa_load(tree, offset);
> --
> 2.53.0-Meta
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
  2026-09-07 16:19 [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap Usama Arif
  2026-09-08  8:10 ` Yosry Ahmed
@ 2026-09-09 18:14 ` Nhat Pham
  2026-09-10  2:47 ` Andrew Morton
  2 siblings, 0 replies; 6+ messages in thread
From: Nhat Pham @ 2026-09-09 18:14 UTC (permalink / raw)
  To: Usama Arif
  Cc: yosry, Andrew Morton, chengming.zhou, hannes, linux-kernel,
	linux-mm, kernel-team, stable, Alexandre Ghiti

On Mon, Sep 7, 2026 at 9:20 AM Usama Arif <usama.arif@linux.dev> 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.
>
> Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
> Cc: stable@vger.kernel.org
> Co-developed-by: Alexandre Ghiti <alex@ghiti.fr>
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>

Acked-by: Nhat Pham <nphamcs@gmail.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
  2026-09-07 16:19 [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap Usama Arif
  2026-09-08  8:10 ` Yosry Ahmed
  2026-09-09 18:14 ` Nhat Pham
@ 2026-09-10  2:47 ` Andrew Morton
  2026-09-10 10:11   ` Usama Arif
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2026-09-10  2:47 UTC (permalink / raw)
  To: Usama Arif
  Cc: yosry, chengming.zhou, hannes, linux-kernel, linux-mm, nphamcs,
	kernel-team, stable, Alexandre Ghiti

On Mon,  7 Sep 2026 09:19:38 -0700 Usama Arif <usama.arif@linux.dev> 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?

> Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
> Cc: stable@vger.kernel.org


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!



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
  2026-09-10  2:47 ` Andrew Morton
@ 2026-09-10 10:11   ` Usama Arif
  2026-09-10 10:20     ` Yosry Ahmed
  0 siblings, 1 reply; 6+ messages in thread
From: Usama Arif @ 2026-09-10 10:11 UTC (permalink / raw)
  To: Andrew Morton
  Cc: yosry, chengming.zhou, hannes, linux-kernel, linux-mm, nphamcs,
	kernel-team, stable, Alexandre Ghiti



On 10/09/2026 03:47, Andrew Morton wrote:
> On Mon,  7 Sep 2026 09:19:38 -0700 Usama Arif <usama.arif@linux.dev> 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@vger.kernel.org
> 
> 
> 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!



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap
  2026-09-10 10:11   ` Usama Arif
@ 2026-09-10 10:20     ` Yosry Ahmed
  0 siblings, 0 replies; 6+ messages in thread
From: Yosry Ahmed @ 2026-09-10 10:20 UTC (permalink / raw)
  To: Usama Arif
  Cc: Andrew Morton, chengming.zhou, hannes, linux-kernel, linux-mm,
	nphamcs, kernel-team, stable, Alexandre Ghiti

On Thu, Sep 10, 2026 at 3:11 AM Usama Arif <usama.arif@linux.dev> wrote:
>
>
>
> On 10/09/2026 03:47, Andrew Morton wrote:
> > On Mon,  7 Sep 2026 09:19:38 -0700 Usama Arif <usama.arif@linux.dev> 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?

I am fine either way.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-10 10:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 16:19 [PATCH] mm: zswap: don't fail a large-folio swapin whose range is not in zswap Usama Arif
2026-09-08  8:10 ` Yosry Ahmed
2026-09-09 18:14 ` Nhat Pham
2026-09-10  2:47 ` Andrew Morton
2026-09-10 10:11   ` Usama Arif
2026-09-10 10:20     ` Yosry Ahmed

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.