[PATCH v2 0/3] mm, swap: don't spin or flood the console on a bad swap entry

[PATCH v2 0/3] mm, swap: don't spin or flood the console on a bad swap entry

[Breno Leitao]

I've seen some machines at Meta fleet that show the following type of
problem:

1) It gets some weird warning:

  BUG: Bad page map in process khugepaged  pte:f000eef300000017 pmd:00000067
  addr:00007f57c0a01000 vm_flags:20200073 anon_vma:ffff88829af7c340 mapping:0000000000000000 index:7f57c0a01

The corruption is most likely the collapse/PT_RECLAIM race fixed by
commit 366a4532d96f ("mm: fix the race between collapse and PT_RECLAIM
under per-vma lock"). But this series is not about this one.

2) Then it floods all the monitoring of the fleet, sending the same
   message in the loop, crashing the our fleet kernel monitoring
   subsystem (which is the part that I am interested in protecting)

  get_swap_device: Bad swap offset entry 3ffffffc043c5

For instance, in a host today it logged 6M in a few hours, and it is still
going forever. Two things go wrong.

1) get_swap_device() prints unconditionally, unlike print_bad_pte() next
   door which suppresses itself with is_bad_page_map_ratelimited().

1) do_swap_page() returns 0 when get_swap_device() fails, so the
   fault is retried, reads the same entry and faults again.
   Nothing in the round trip changes the PTE.

Trying to fix it in a naive way:

Patch 1 is super simple, and rate limits the two prints.

Patch 2 makes get_swap_device() return ERR_PTR(-EINVAL) for an entry
that can never name a slot on any device, keeping NULL for a device
swapoff is taking away, and converts the callers. No functional change
expected.

Patch 3 uses that to return VM_FAULT_SIGBUS instead of retrying.

PS: Sashiko flagged several pre-existing issues, and get_swap_device()
returning an error opens the door to fixing some of them.  For this
series, I am focused in landing the basic cases first and build on top,
if needed.

---
Changes in v2:
- Rate limit swap_dup_entry_direct()'s print too (Andrew)
- Drop "in get_swap_device()" from patch 1's subject, it now covers all
  three prints
- Return ERR_PTR(-EIO) rather than ERR_PTR(-EINVAL) for a malformed
  entry; -EINVAL is too soft for a corrupt page table (David)
- Document the malformed entry case in get_swap_device()'s kerneldoc,
  in patch 2 instead of patch 3 (David)
- Reword patch 2's changelog, "an entry that can never name a slot on
  any device" was unclear (David)
- Link to v1: https://patch.msgid.link/20260810-swap-v1-0-375ef0767206@debian.org

To: Andrew Morton <akpm@linux-foundation.org>
To: Chris Li <chrisl@kernel.org>
To: Kairui Song <kasong@tencent.com>
To: Kemeng Shi <shikemeng@huaweicloud.com>
To: Nhat Pham <nphamcs@gmail.com>
To: Baoquan He <baoquan.he@linux.dev>
To: Barry Song <baohua@kernel.org>
To: Youngjun Park <youngjun.park@lge.com>
To: David Hildenbrand <david@kernel.org>
To: Lorenzo Stoakes <ljs@kernel.org>
To: "Liam R. Howlett" <liam@infradead.org>
To: Vlastimil Babka <vbabka@kernel.org>
To: Mike Rapoport <rppt@kernel.org>
To: Suren Baghdasaryan <surenb@google.com>
To: Michal Hocko <mhocko@suse.com>
To: Jann Horn <jannh@google.com>
To: Pedro Falcato <pfalcato@suse.de>
To: Hugh Dickins <hughd@google.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
To: Peter Xu <peterx@redhat.com>
To: Johannes Weiner <hannes@cmpxchg.org>
To: Yosry Ahmed <yosry@kernel.org>
To: Chengming Zhou <chengming.zhou@linux.dev>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org

---
Breno Leitao (3):
      mm, swap: ratelimit bad swap entry reports
      mm, swap: distinguish a malformed swap entry from a dying device
      mm: fail the fault on a malformed swap entry instead of retrying it

 mm/memory.c      |  9 +++++++--
 mm/mincore.c     |  2 +-
 mm/shmem.c       |  2 +-
 mm/swap_state.c  |  4 ++--
 mm/swapfile.c    | 20 ++++++++++++--------
 mm/userfaultfd.c |  3 ++-
 mm/zswap.c       |  2 +-
 7 files changed, 26 insertions(+), 16 deletions(-)
---
base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
change-id: 20260810-swap-25420f9c8ba9

Best regards,
--  
Breno Leitao <leitao@debian.org>

[PATCH v2 1/3] mm, swap: ratelimit bad swap entry reports

[Breno Leitao]

A corrupt page table hands the same bogus entry to get_swap_device() on
every access to the mapping, and every rejection is logged. One machine
logged 6185620 copies of the same line in a few hours.

swap_dup_entry_direct() prints the same message from the fork path, once
per call: the WARN_ON_ONCE() guarding it warns once, the pr_err() inside
does not.

Rate limit all three prints.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 mm/swapfile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 4d4e3e3059f6b..31c8a340606bb 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1899,11 +1899,11 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
 
 	return si;
 bad_nofile:
-	pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
+	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_file, entry.val);
 out:
 	return NULL;
 put_out:
-	pr_err("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
+	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
 	percpu_ref_put(&si->users);
 	return NULL;
 }
@@ -3876,7 +3876,7 @@ int swap_dup_entry_direct(swp_entry_t entry)
 
 	si = swap_entry_to_info(entry);
 	if (WARN_ON_ONCE(!si)) {
-		pr_err("%s%08lx\n", Bad_file, entry.val);
+		pr_err_ratelimited("%s%08lx\n", Bad_file, entry.val);
 		return -EINVAL;
 	}
 

-- 
2.53.0-Meta

[PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device

[Breno Leitao]

get_swap_device() returns NULL for two different things: an entry whose
type names no swap device or whose offset is past the end of one, and a
device that swapoff is taking away. The first never becomes valid, the
second does, and callers cannot tell them apart.

Return ERR_PTR(-EIO) for the two malformed cases and keep NULL for
swapoff. copy_nonpresent_pte() already reports -EIO for the same
corruption on the fork path.

Callers bail out on failure either way, so switch them to
IS_ERR_OR_NULL() and clear si where the cleanup path would otherwise
put an ERR_PTR. No functional change.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 mm/memory.c      |  6 ++++--
 mm/mincore.c     |  2 +-
 mm/shmem.c       |  2 +-
 mm/swap_state.c  |  4 ++--
 mm/swapfile.c    | 14 +++++++++-----
 mm/userfaultfd.c |  3 ++-
 mm/zswap.c       |  2 +-
 7 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index d9cf941967cf0..7201e848129a7 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4954,10 +4954,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		goto out;
 	}
 
-	/* Prevent swapoff from happening to us. */
+	/* Prevent swapoff from happening to us, and reject a bad entry. */
 	si = get_swap_device(entry);
-	if (unlikely(!si))
+	if (IS_ERR_OR_NULL(si)) {
+		si = NULL;
 		goto out;
+	}
 
 	folio = swap_cache_get_folio(entry);
 	if (folio)
diff --git a/mm/mincore.c b/mm/mincore.c
index ff4ac82817683..c086836bc4bcc 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -71,7 +71,7 @@ static unsigned char mincore_swap(swp_entry_t entry, bool shmem)
 	 */
 	if (shmem) {
 		si = get_swap_device(entry);
-		if (!si)
+		if (IS_ERR_OR_NULL(si))
 			return 0;
 	}
 	folio = swap_cache_get_folio(entry);
diff --git a/mm/shmem.c b/mm/shmem.c
index 65572cbf1bd3c..d0a9f52bfed71 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2276,7 +2276,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
 
 	si = get_swap_device(index_entry);
 	order = shmem_confirm_swap(mapping, index, index_entry);
-	if (unlikely(!si)) {
+	if (IS_ERR_OR_NULL(si)) {
 		if (order < 0)
 			return -EEXIST;
 		else
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 4b7a3303c463b..f2e86d6626ecc 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -715,7 +715,7 @@ struct folio *read_swap_cache_async(struct swap_io_ctx *ctx, swp_entry_t entry,
 	struct folio *folio;
 
 	si = get_swap_device(entry);
-	if (!si)
+	if (IS_ERR_OR_NULL(si))
 		return NULL;
 
 	mpol = get_vma_policy(vma, addr, 0, &ilx);
@@ -951,7 +951,7 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
 		 */
 		if (swp_type(entry) != swp_type(targ_entry)) {
 			si = get_swap_device(entry);
-			if (!si)
+			if (IS_ERR_OR_NULL(si))
 				continue;
 		}
 		folio = swap_cache_read_folio(&ctx, entry, gfp_mask, mpol, ilx,
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 31c8a340606bb..b96bc89815935 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1504,7 +1504,7 @@ int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp)
 	unsigned long offset = swp_offset(entry);
 
 	si = get_swap_device(entry);
-	if (!si)
+	if (IS_ERR_OR_NULL(si))
 		return 0;
 
 	ci = __swap_offset_to_cluster(si, offset);
@@ -1859,7 +1859,10 @@ void folio_put_swap(struct folio *folio, struct page *page)
  * Check whether swap entry is valid in the swap device.  If so,
  * return pointer to swap_info_struct, and keep the swap entry valid
  * via preventing the swap device from being swapoff, until
- * put_swap_device() is called.  Otherwise return NULL.
+ * put_swap_device() is called.  Return NULL for an empty entry or a
+ * device that is going away, and ERR_PTR(-EIO) if the entry's type
+ * names no swap device or its offset is past the end of one. These EIOs
+ * are preceded by pr_err().
  *
  * Notice that swapoff or swapoff+swapon can still happen before the
  * percpu_ref_tryget_live() in get_swap_device() or after the
@@ -1900,12 +1903,13 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
 	return si;
 bad_nofile:
 	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_file, entry.val);
+	return ERR_PTR(-EIO);
 out:
 	return NULL;
 put_out:
 	pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
 	percpu_ref_put(&si->users);
-	return NULL;
+	return ERR_PTR(-EIO);
 }
 
 /*
@@ -2001,7 +2005,7 @@ int swp_swapcount(swp_entry_t entry)
 	int count;
 
 	si = get_swap_device(entry);
-	if (!si)
+	if (IS_ERR_OR_NULL(si))
 		return 0;
 
 	ci = swap_cluster_lock(si, swp_offset(entry));
@@ -2127,7 +2131,7 @@ void swap_put_entries_direct(swp_entry_t entry, int nr)
 	struct swap_info_struct *si;
 
 	si = get_swap_device(entry);
-	if (WARN_ON_ONCE(!si))
+	if (WARN_ON_ONCE(IS_ERR_OR_NULL(si)))
 		return;
 	if (WARN_ON_ONCE(end_offset > si->max))
 		goto out;
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 24a4d92ffa3c2..bf7bc7fb1aa0f 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -1700,7 +1700,8 @@ static long move_pages_ptes(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd
 		}
 
 		si = get_swap_device(entry);
-		if (unlikely(!si)) {
+		if (IS_ERR_OR_NULL(si)) {
+			si = NULL;
 			ret = -EAGAIN;
 			goto out;
 		}
diff --git a/mm/zswap.c b/mm/zswap.c
index f7c9c89f6449c..bc9b931d6f447 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -997,7 +997,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
 
 	/* try to allocate swap cache folio */
 	si = get_swap_device(swpentry);
-	if (!si)
+	if (IS_ERR_OR_NULL(si))
 		return -EEXIST;
 
 	mpol = get_task_policy(current);

-- 
2.53.0-Meta

[PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Breno Leitao]

do_swap_page() returns 0 when get_swap_device() fails, which the fault
handler reads as "handled". For an entry that can never become valid
the retry takes the same fault again, so the thread spins forever,
retrying on the same fault.

Return VM_FAULT_SIGBUS (Bad access) for a malformed entry (pr_err() was
called at get_swap_device()).

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 mm/memory.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index 7201e848129a7..fa2b3d2ad3202 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4957,6 +4957,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	/* Prevent swapoff from happening to us, and reject a bad entry. */
 	si = get_swap_device(entry);
 	if (IS_ERR_OR_NULL(si)) {
+		/* A malformed entry never becomes valid, so don't retry it. */
+		if (IS_ERR(si))
+			ret = VM_FAULT_SIGBUS;
 		si = NULL;
 		goto out;
 	}

-- 
2.53.0-Meta

Re: [PATCH v2 0/3] mm, swap: don't spin or flood the console on a bad swap entry

[Andrew Morton]

On Thu, 13 Aug 2026 03:02:19 -0700 Breno Leitao <leitao@debian.org> wrote:

> I've seen some machines at Meta fleet that show the following type of
> problem:
> 
> 1) It gets some weird warning:
> 
>   BUG: Bad page map in process khugepaged  pte:f000eef300000017 pmd:00000067
>   addr:00007f57c0a01000 vm_flags:20200073 anon_vma:ffff88829af7c340 mapping:0000000000000000 index:7f57c0a01
> 
> The corruption is most likely the collapse/PT_RECLAIM race fixed by
> commit 366a4532d96f ("mm: fix the race between collapse and PT_RECLAIM
> under per-vma lock"). But this series is not about this one.
> 
> 2) Then it floods all the monitoring of the fleet, sending the same
>    message in the loop, crashing the our fleet kernel monitoring
>    subsystem (which is the part that I am interested in protecting)
> 
>   get_swap_device: Bad swap offset entry 3ffffffc043c5
> 
> For instance, in a host today it logged 6M in a few hours, and it is still
> going forever. Two things go wrong.
> 
> 1) get_swap_device() prints unconditionally, unlike print_bad_pte() next
>    door which suppresses itself with is_bad_page_map_ratelimited().
> 
> 1) do_swap_page() returns 0 when get_swap_device() fails, so the
>    fault is retried, reads the same entry and faults again.
>    Nothing in the round trip changes the PTE.
> 
> Trying to fix it in a naive way:

Cool.

These behaviors sound pretty obnoxious.  And the patches are quite
simple so hopefully the swap maintainers will make quick work of them.

I'm assuming that users of earlier kernels will want these things fixed
so please let's work on identifying suitable Fixes: targets and
deciding which of them should get a cc:stable.



In a spirit of experimentation I asked Gemini to identify suitable Fixes:
targets and it said

[1/3]: Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")

[2/3]: Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")
	(and it complained that this patch doesn't fix anything)

[3/3] Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")

And I cannot find such a commit anywhere, so wtf.

chatgpt didn't give me anything useful.



[2/3] is "no functional change" so ideally it simply wouldn't be
present in the series - we should aim for minimal changes when fixing
bugs, then leave the cleanups for later.


> PS: Sashiko flagged several pre-existing issues, and get_swap_device()
> returning an error opens the door to fixing some of them.  For this
> series, I am focused in landing the basic cases first and build on top,
> if needed.

Yeah. probably these are the same issues:
	https://sashiko.dev/#/patchset/20260813-swap-v2-0-4a625ccabdae@debian.org


As usual, they're all mishandled error-path things.  It's axiomatic,
really - nobody hits error-path bugs, so they never get reported so
they never get fixed.

otoh, now that these bugs are out there and known about, it's possible
that a Black Hat can find a way of exploiting them, which increases the
pressure to get these bugs addressed.

Re: [PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device

[Barry Song]

On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
>
> get_swap_device() returns NULL for two different things: an entry whose
> type names no swap device or whose offset is past the end of one, and a
> device that swapoff is taking away. The first never becomes valid, the
> second does, and callers cannot tell them apart.
>
> Return ERR_PTR(-EIO) for the two malformed cases and keep NULL for
> swapoff. copy_nonpresent_pte() already reports -EIO for the same
> corruption on the fork path.
>
> Callers bail out on failure either way, so switch them to
> IS_ERR_OR_NULL() and clear si where the cleanup path would otherwise
> put an ERR_PTR. No functional change.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
[...]

> @@ -1859,7 +1859,10 @@ void folio_put_swap(struct folio *folio, struct page *page)
>   * Check whether swap entry is valid in the swap device.  If so,
>   * return pointer to swap_info_struct, and keep the swap entry valid
>   * via preventing the swap device from being swapoff, until
> - * put_swap_device() is called.  Otherwise return NULL.
> + * put_swap_device() is called.  Return NULL for an empty entry or a
> + * device that is going away, and ERR_PTR(-EIO) if the entry's type
> + * names no swap device or its offset is past the end of one. These EIOs
> + * are preceded by pr_err().
>   *
>   * Notice that swapoff or swapoff+swapon can still happen before the
>   * percpu_ref_tryget_live() in get_swap_device() or after the
> @@ -1900,12 +1903,13 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry)
>         return si;
>  bad_nofile:
>         pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_file, entry.val);
> +       return ERR_PTR(-EIO);
>  out:
>         return NULL;
>  put_out:
>         pr_err_ratelimited("%s: %s%08lx\n", __func__, Bad_offset, entry.val);
>         percpu_ref_put(&si->users);
> -       return NULL;
> +       return ERR_PTR(-EIO);
>  }

Hi Breno,

Do you know why we’re seeing corrupted PTEs with `bad_nofile`?
Does this issue still exist in mainline?

Thanks
Barry

Re: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Barry Song]

On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
>
> do_swap_page() returns 0 when get_swap_device() fails, which the fault
> handler reads as "handled". For an entry that can never become valid
> the retry takes the same fault again, so the thread spins forever,
> retrying on the same fault.
>
> Return VM_FAULT_SIGBUS (Bad access) for a malformed entry (pr_err() was
> called at get_swap_device()).
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  mm/memory.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 7201e848129a7..fa2b3d2ad3202 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4957,6 +4957,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>         /* Prevent swapoff from happening to us, and reject a bad entry. */
>         si = get_swap_device(entry);
>         if (IS_ERR_OR_NULL(si)) {
> +               /* A malformed entry never becomes valid, so don't retry it. */
> +               if (IS_ERR(si))
> +                       ret = VM_FAULT_SIGBUS;


Hi Breno,

Since you now return VM_FAULT_SIGBUS, the page fault should no
longer retry repeatedly. Do we still need patch 1/3, which adds the
rate-limited printk?

Best Regards
Barry

Re: [PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device

[Breno Leitao]

Hello Barry,

On Mon, Aug 17, 2026 at 06:20:22AM +0800, Barry Song wrote:
> On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
> 
> Do you know why we’re seeing corrupted PTEs with `bad_nofile`?
>
> Does this issue still exist in mainline?

The problem seems related to the problem fixed by commit 366a4532d96f
("mm: fix the race between collapse and PT_RECLAIM under per-vma lock"),
but, I am less interested in that problem and more on the behaviour of
this code path when problems like this exist.

I've got this into this issue due to an overflow in the kernel
monitoring subsytem at Meta due to 7 machines getting into this state.
:-|

Re: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Breno Leitao]

On Mon, Aug 17, 2026 at 06:22:41AM +0800, Barry Song wrote:
> On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
> >
> > do_swap_page() returns 0 when get_swap_device() fails, which the fault
> > handler reads as "handled". For an entry that can never become valid
> > the retry takes the same fault again, so the thread spins forever,
> > retrying on the same fault.
> >
> > Return VM_FAULT_SIGBUS (Bad access) for a malformed entry (pr_err() was
> > called at get_swap_device()).
> >
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> >  mm/memory.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 7201e848129a7..fa2b3d2ad3202 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4957,6 +4957,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >         /* Prevent swapoff from happening to us, and reject a bad entry. */
> >         si = get_swap_device(entry);
> >         if (IS_ERR_OR_NULL(si)) {
> > +               /* A malformed entry never becomes valid, so don't retry it. */
> > +               if (IS_ERR(si))
> > +                       ret = VM_FAULT_SIGBUS;
> 
> 
> Hi Breno,
> 
> Since you now return VM_FAULT_SIGBUS, the page fault should no
> longer retry repeatedly. Do we still need patch 1/3, which adds the
> rate-limited printk?

Yes, I still think we need it, for a few reasons:

1) A different bug could just as easily trigger the same message
   flood again.
2) I don't see a case where flooding the log with this message
   would help. If it keeps firing, something else is already
   broken, and the repeated message itself adds nothing useful.
3) From a monitoring perspective, I'd guess 95% of our log
   messages should be rate limited anyway, and this one would fall into
   this category.

You think this one shouldn't be ratelimited?

Thanks for the review,
--breno

Re: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Barry Song]

On Mon, Aug 17, 2026 at 5:30 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Mon, Aug 17, 2026 at 06:22:41AM +0800, Barry Song wrote:
> > On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
> > >
> > > do_swap_page() returns 0 when get_swap_device() fails, which the fault
> > > handler reads as "handled". For an entry that can never become valid
> > > the retry takes the same fault again, so the thread spins forever,
> > > retrying on the same fault.
> > >
> > > Return VM_FAULT_SIGBUS (Bad access) for a malformed entry (pr_err() was
> > > called at get_swap_device()).
> > >
> > > Signed-off-by: Breno Leitao <leitao@debian.org>
> > > ---
> > >  mm/memory.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 7201e848129a7..fa2b3d2ad3202 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -4957,6 +4957,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> > >         /* Prevent swapoff from happening to us, and reject a bad entry. */
> > >         si = get_swap_device(entry);
> > >         if (IS_ERR_OR_NULL(si)) {
> > > +               /* A malformed entry never becomes valid, so don't retry it. */
> > > +               if (IS_ERR(si))
> > > +                       ret = VM_FAULT_SIGBUS;
> >
> >
> > Hi Breno,
> >
> > Since you now return VM_FAULT_SIGBUS, the page fault should no
> > longer retry repeatedly. Do we still need patch 1/3, which adds the
> > rate-limited printk?
>
> Yes, I still think we need it, for a few reasons:
>
> 1) A different bug could just as easily trigger the same message
>    flood again.
> 2) I don't see a case where flooding the log with this message
>    would help. If it keeps firing, something else is already
>    broken, and the repeated message itself adds nothing useful.
> 3) From a monitoring perspective, I'd guess 95% of our log
>    messages should be rate limited anyway, and this one would fall into
>    this category.
>
> You think this one shouldn't be ratelimited?

I’m fine with rate limiting. I’m just curious: now that you return
`SIGBUS`, the PF won’t retry, so you shouldn’t get flooded with
printk messages, right?
Or are there still cases where returning SIGBUS won’t prevent the
printk flooding?

Best Regards
Barry

Re: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Breno Leitao]

On Mon, Aug 17, 2026 at 05:40:59PM +0800, Barry Song wrote:
> > > Since you now return VM_FAULT_SIGBUS, the page fault should no
> > > longer retry repeatedly. Do we still need patch 1/3, which adds the
> > > rate-limited printk?
> >
> > Yes, I still think we need it, for a few reasons:
> >
> > 1) A different bug could just as easily trigger the same message
> >    flood again.
> > 2) I don't see a case where flooding the log with this message
> >    would help. If it keeps firing, something else is already
> >    broken, and the repeated message itself adds nothing useful.
> > 3) From a monitoring perspective, I'd guess 95% of our log
> >    messages should be rate limited anyway, and this one would fall into
> >    this category.
> >
> > You think this one shouldn't be ratelimited?
> 
> I’m fine with rate limiting. I’m just curious: now that you return
> `SIGBUS`, the PF won’t retry, so you shouldn’t get flooded with
> printk messages, right?

Oh, do_swap_page() is only one of the get_swap_device() call sites, and
I am only returning SIGBUS from do_swap_page(). The other callers are
unchanged, and whether any of them can loop on the same entry and flood
needs a closer look.

> Or are there still cases where returning SIGBUS won’t prevent the
> printk flooding?

For the path I hit, do_swap_page(), SIGBUS does prevent the flood.
I cannot say the same for the other callers yet.

That is also why I would like to keep patch 1 standing on its own: it is
a cheap backstop no matter which caller is spinning, and it can go to
stable independently of patches 2 and 3.

Thanks for the solid questions,
--breno

Re: [PATCH v2 1/3] mm, swap: ratelimit bad swap entry reports

[Barry Song]

On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
>
> A corrupt page table hands the same bogus entry to get_swap_device() on
> every access to the mapping, and every rejection is logged. One machine
> logged 6185620 copies of the same line in a few hours.
>
> swap_dup_entry_direct() prints the same message from the fork path, once
> per call: the WARN_ON_ONCE() guarding it warns once, the pr_err() inside
> does not.
>
> Rate limit all three prints.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---

Reviewed-by: Barry Song <baohua@kernel.org>

Re: [PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device

[Barry Song]

On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
>
> get_swap_device() returns NULL for two different things: an entry whose
> type names no swap device or whose offset is past the end of one, and a
> device that swapoff is taking away. The first never becomes valid, the
> second does, and callers cannot tell them apart.
>
> Return ERR_PTR(-EIO) for the two malformed cases and keep NULL for
> swapoff. copy_nonpresent_pte() already reports -EIO for the same
> corruption on the fork path.
>
> Callers bail out on failure either way, so switch them to
> IS_ERR_OR_NULL() and clear si where the cleanup path would otherwise
> put an ERR_PTR. No functional change.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>

My gut feeling is that a corrupted PTE entry could also result in
type > 0 && type < MAX_SWAPFILES and offset < si->max.
However, that would likely lead to other serious problems later.

For this patch, I think it is reasonable to report the two obvious
corrupted PTE entries as -EIO.

Reviewed-by: Barry Song <baohua@kernel.org>

Re: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it

[Barry Song]

On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <leitao@debian.org> wrote:
>
> do_swap_page() returns 0 when get_swap_device() fails, which the fault
> handler reads as "handled". For an entry that can never become valid
> the retry takes the same fault again, so the thread spins forever,
> retrying on the same fault.
>
> Return VM_FAULT_SIGBUS (Bad access) for a malformed entry (pr_err() was
> called at get_swap_device()).
>
> Signed-off-by: Breno Leitao <leitao@debian.org>

With a few minor nits below,

Reviewed-by: Barry Song <baohua@kernel.org>

> ---
>  mm/memory.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 7201e848129a7..fa2b3d2ad3202 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4957,6 +4957,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>         /* Prevent swapoff from happening to us, and reject a bad entry. */
>         si = get_swap_device(entry);
>         if (IS_ERR_OR_NULL(si)) {
> +               /* A malformed entry never becomes valid, so don't retry it. */
> +               if (IS_ERR(si))
> +                       ret = VM_FAULT_SIGBUS;
>                 si = NULL;

Rather than resetting si to NULL, a more natural approach might be:

index efdf82b3c418..0286b7635bcd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5272,7 +5272,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
        if (vmf->pte)
                pte_unmap_unlock(vmf->pte, vmf->ptl);
 out:
-       if (si)
+       if (!IS_ERR_OR_NULL(si))
                put_swap_device(si);
        return ret;
 out_nomap:


Thanks
Barry

Re: [PATCH v2 0/3] mm, swap: don't spin or flood the console on a bad swap entry

[Breno Leitao]

Hello Andrew,

On Thu, Aug 13, 2026 at 01:34:55PM -0700, Andrew Morton wrote:
> On Thu, 13 Aug 2026 03:02:19 -0700 Breno Leitao <leitao@debian.org> wrote:
> 
> > I've seen some machines at Meta fleet that show the following type of
> > problem:
> > 
> > 1) It gets some weird warning:
> > 
> >   BUG: Bad page map in process khugepaged  pte:f000eef300000017 pmd:00000067
> >   addr:00007f57c0a01000 vm_flags:20200073 anon_vma:ffff88829af7c340 mapping:0000000000000000 index:7f57c0a01
> > 
> > The corruption is most likely the collapse/PT_RECLAIM race fixed by
> > commit 366a4532d96f ("mm: fix the race between collapse and PT_RECLAIM
> > under per-vma lock"). But this series is not about this one.
> > 
> > 2) Then it floods all the monitoring of the fleet, sending the same
> >    message in the loop, crashing the our fleet kernel monitoring
> >    subsystem (which is the part that I am interested in protecting)
> > 
> >   get_swap_device: Bad swap offset entry 3ffffffc043c5
> > 
> > For instance, in a host today it logged 6M in a few hours, and it is still
> > going forever. Two things go wrong.
> > 
> > 1) get_swap_device() prints unconditionally, unlike print_bad_pte() next
> >    door which suppresses itself with is_bad_page_map_ratelimited().
> > 
> > 1) do_swap_page() returns 0 when get_swap_device() fails, so the
> >    fault is retried, reads the same entry and faults again.
> >    Nothing in the round trip changes the PTE.
> > 
> > Trying to fix it in a naive way:
> 
> Cool.
> 
> These behaviors sound pretty obnoxious.  And the patches are quite
> simple so hopefully the swap maintainers will make quick work of them.
> 
> I'm assuming that users of earlier kernels will want these things fixed
> so please let's work on identifying suitable Fixes: targets and
> deciding which of them should get a cc:stable.
> 
> 
> 
> In a spirit of experimentation I asked Gemini to identify suitable Fixes:
> targets and it said
> 
> [1/3]: Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")
> 
> [2/3]: Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")
> 	(and it complained that this patch doesn't fix anything)
> 
> [3/3] Fixes: 122e201211e4 ("mm, swap: get_swap_device() to get reference count of swap_info_struct")
> 
> And I cannot find such a commit anywhere, so wtf.

I think only 1/3 should be getting a Fixes: in v3. The message I am
drowning in is the Bad_offset one:

  get_swap_device: Bad swap offset entry 3ffffffc043c5

63d8620ecf93b5 ("mm/swapfile: use percpu_ref to serialize against
concurrent swapoff") added the put_out: label with just the
percpu_ref_put(), so that arm was silent. The pr_err() landed in v5.19:

So, if I need to update it, I will include:

Fixes: 23b230ba8ac3 ("mm/swap: print bad swap offset entry in get_swap_device")
Cc: <stable@vger.kernel.org>

> [2/3] is "no functional change" so ideally it simply wouldn't be
> present in the series - we should aim for minimal changes when fixing
> bugs, then leave the cleanups for later.

I need 2/3 to expose the difference in the first place.
get_swap_device() returns NULL both for a malformed entry and for
a device swapoff is taking away, so no caller can tell whether the
failure is worth retrying. 

2/3 adds that distinction and converts the callers, but none of them act
on it yet, so it is no functional change on its own. 

Then 3/3 is the actual fix, now that do_swap_page() can differentiate
a retry from give up.

Do you want me to squash them?