mm, swap: don't spin or flood the console on a bad swap entry
Subsystems (7)
v2 cover mm, swap: don't spin or flood the console on a bad swap entry
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/[email protected]
To: Andrew Morton <[email protected]>
To: Chris Li <[email protected]>
To: Kairui Song <[email protected]>
To: Kemeng Shi <[email protected]>
To: Nhat Pham <[email protected]>
To: Baoquan He <[email protected]>
To: Barry Song <[email protected]>
To: Youngjun Park <[email protected]>
To: David Hildenbrand <[email protected]>
To: Lorenzo Stoakes <[email protected]>
To: "Liam R. Howlett" <[email protected]>
To: Vlastimil Babka <[email protected]>
To: Mike Rapoport <[email protected]>
To: Suren Baghdasaryan <[email protected]>
To: Michal Hocko <[email protected]>
To: Jann Horn <[email protected]>
To: Pedro Falcato <[email protected]>
To: Hugh Dickins <[email protected]>
To: Baolin Wang <[email protected]>
To: Peter Xu <[email protected]>
To: Johannes Weiner <[email protected]>
To: Yosry Ahmed <[email protected]>
To: Chengming Zhou <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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,
Signature · 1 line
Breno Leitao <[email protected]>
PATCH v2 1/3 mm, swap: ratelimit bad swap entry reports
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.
--- mm/swapfile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
mm/swapfile.c +3 -3
@@ -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 ; }
Signature · 1 line
2.53.0-Meta
Re: [PATCH v2 1/3] mm, swap: ratelimit bad swap entry reports
Quoted 5 lines · 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.
Quoted 8 lines
swap_dup_entry_direct()prints the same message from the fork path, once per call: theWARN_ON_ONCE()guarding it warns once, thepr_err()inside does not.Rate limit all three prints.
Signed-off-by Breno Leitao <[email protected]>---
Re: [PATCH v2 1/3] mm, swap: ratelimit bad swap entry reports
In reply to Breno LeitaoQuoted 5 lines · 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.
Quoted 7 lines
swap_dup_entry_direct()prints the same message from the fork path, once per call: theWARN_ON_ONCE()guarding it warns once, thepr_err()inside does not.Rate limit all three prints.
Signed-off-by Breno Leitao <[email protected]>
Re: [PATCH v2 1/3] mm, swap: ratelimit bad swap entry reports
In reply to Breno LeitaoQuoted 34 lines · 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: theWARN_ON_ONCE()guarding it warns once, thepr_err()inside does not.Rate limit all three prints.
Signed-off-by Breno Leitao <[email protected]>--- mm/swapfile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)mm/swapfile.c +2
-2@@ -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 ; }
mm/swapfile.c int swap_dup_entry_direct(swp_entry_t entry) · line 3882
Quoted 8 lines
mm/swapfile.c +1
-1@@ -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 ; }
Looks good to me, thanks!
PATCH v2 2/3 mm, swap: distinguish a malformed swap entry from a dying device
In reply to Breno Leitaoget_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.
--- 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(-)
mm/memory.c +4 -2
@@ -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)
mm/mincore.c +1 -1
@@ -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);
mm/shmem.c +1 -1
@@ -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
mm/swap_state.c +2 -2
@@ -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,
mm/swapfile.c +9 -5
@@ -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;
mm/userfaultfd.c +2 -1
@@ -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; }
mm/zswap.c +1 -1
@@ -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);
Signature · 1 line
2.53.0-Meta
Barry Song
reviewerRe: [PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device
Quoted 16 lines · 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-EIOfor 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 anERR_PTR. No functional change.Signed-off-by Breno Leitao <[email protected]>---
[...]
Quoted 15 lines
mm/swapfile.c +4
-1@@ -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:
mm/swapfile.c struct swap_info_struct *get_swap_device(swp_entry_t entry) · line 1913
Quoted 10 lines
mm/swapfile.c +2
-1pr_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 2/3] mm, swap: distinguish a malformed swap entry from a dying device
Hello Barry,
Quoted 5 lines · Barry Song
On Thu, Aug 13, 2026 at 6:02 PM Breno Leitao <[email protected]> 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 2/3] mm, swap: distinguish a malformed swap entry from a dying device
In reply to Breno LeitaoQuoted 10 lines · 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-EIOfor the same corruption on the fork path.
Quoted 5 lines
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 anERR_PTR. No functional change.Signed-off-by Breno Leitao <[email protected]>
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.
Re: [PATCH v2 2/3] mm, swap: distinguish a malformed swap entry from a dying device
In reply to Breno LeitaoQuoted 14 lines · 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-EIOfor 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 anERR_PTR. No functional change.Signed-off-by Breno Leitao <[email protected]>
Quoted 10 lines
--- 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(-)
Looks good, thanks!
PATCH v2 3/3 mm: fail the fault on a malformed swap entry instead of retrying it
In reply to Breno Leitaodo_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()).
--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)
mm/memory.c +3 -0
@@ -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; }
Signature · 1 line
2.53.0-Meta
Barry Song
reviewerRe: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it
Quoted 19 lines · Breno Leitao
do_swap_page()returns 0 whenget_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 atget_swap_device()).Signed-off-by Breno Leitao <[email protected]>--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)mm/memory.c +0
-0
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4962
Quoted 6 lines
mm/memory.c +3
-0@@ -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 3/3] mm: fail the fault on a malformed swap entry instead of retrying it
Quoted 30 lines · Barry Song
Quoted 19 lines · Breno Leitao
do_swap_page()returns 0 whenget_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 atget_swap_device()).Signed-off-by Breno Leitao <[email protected]>--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)mm/memory.c +0
-0mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4962
Quoted 6 lines
mm/memory.c +3
-0@@ -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,
Quoted 3 lines
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
Barry Song
reviewerRe: [PATCH v2 3/3] mm: fail the fault on a malformed swap entry instead of retrying it
Quoted 47 lines · Breno Leitao
Quoted 30 lines · Barry Song
Quoted 19 lines · Breno Leitao
do_swap_page()returns 0 whenget_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 atget_swap_device()).Signed-off-by Breno Leitao <[email protected]>--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)mm/memory.c +0
-0mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4962
Quoted 6 lines
mm/memory.c +3
-0@@ -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,
Quoted 3 lines
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.
Quoted 1 line
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
Quoted 15 lines · Barry Song
Quoted 15 lines
Quoted 3 lines
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.
Quoted 5 lines
Quoted 1 line
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.
Quoted 2 lines
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 3/3] mm: fail the fault on a malformed swap entry instead of retrying it
In reply to Breno LeitaoQuoted 10 lines · Breno Leitao
do_swap_page()returns 0 whenget_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 atget_swap_device()).Signed-off-by Breno Leitao <[email protected]>
With a few minor nits below,
Quoted 9 lines
--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)mm/memory.c +0
-0
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4963
Quoted 7 lines
mm/memory.c +3
-0@@ -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
mm/memory.c +1 -1
@@ -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 3/3] mm: fail the fault on a malformed swap entry instead of retrying it
In reply to Breno LeitaoQuoted 31 lines · Breno Leitao
do_swap_page()returns 0 whenget_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 atget_swap_device()).Signed-off-by Breno Leitao <[email protected]>--- mm/memory.c | 3 +++ 1 file changed, 3 insertions(+)mm/memory.c +3
-0@@ -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; }Signature · 2 lines
-- 2.53.0-Meta
Looks good to me but interesting b4 shazam will apply a Fixed tag to this, maybe it picked up the "Fixes" in discussion.
I checked the discussion and the history behind this, 2799e77529c2 caused the loop, and 23b230ba8ac3 caused the pr_err spam. But before 2799e77529c2 it wasn't correct either, it was OOM kill.
So anyway, the fix is good, we might omit the fixes tag on this I think, the behavior was never correct.
Andrew Morton
maintainerQuoted 21 lines · On Thu, 13 Aug 2026 03:02:19 -0700 Breno Leitao <[email protected]> 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:7f57c0a01The corruption is most likely the collapse/PT_RECLAIM race fixed by commit
366a4532d96f("mm: fix the race between collapse andPT_RECLAIMunder 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 entry3ffffffc043c5For instance, in a host today it logged 6M in a few hours, and it is still going forever. Two things go wrong.
Quoted 8 lines
1)
get_swap_device()prints unconditionally, unlikeprint_bad_pte()next door which suppresses itself withis_bad_page_map_ratelimited().1)
do_swap_page()returns 0 whenget_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.
Quoted 4 lines
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/[email protected]
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.
Hello Andrew,
Quoted 46 lines · Andrew Morton
Quoted 21 lines · On Thu, 13 Aug 2026 03:02:19 -0700 Breno Leitao <[email protected]> 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:7f57c0a01The corruption is most likely the collapse/PT_RECLAIM race fixed by commit
366a4532d96f("mm: fix the race between collapse andPT_RECLAIMunder 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 entry3ffffffc043c5For instance, in a host today it logged 6M in a few hours, and it is still going forever. Two things go wrong.
Quoted 8 lines
1)
get_swap_device()prints unconditionally, unlikeprint_bad_pte()next door which suppresses itself withis_bad_page_map_ratelimited().1)
do_swap_page()returns 0 whenget_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
Quoted 8 lines
[1/3]: Fixes:
122e201211e4("mm, swap:get_swap_device()to get reference count ofswap_info_struct")[2/3]: Fixes:
122e201211e4("mm, swap:get_swap_device()to get reference count ofswap_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 ofswap_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:
Quoted 3 lines
[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?
Andrew Morton
maintainerQuoted 5 lines · On Mon, 17 Aug 2026 05:21:32 -0700 Breno Leitao <[email protected]> wrote:
I think only 1/3 should be getting a Fixes: in v3. The message I am drowning in is the
Bad_offsetone:
get_swap_device: Bad swap offset entry3ffffffc043c5
Quoted 8 lines
63d8620ecf93b5("mm/swapfile: usepercpu_refto serialize against concurrent swapoff") added theput_out: label with just thepercpu_ref_put(), so that arm was silent. Thepr_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 <[email protected]>
OK, so you think that only [1/3] should have cc:stable?
Quoted 9 lines
Quoted 3 lines
[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.
Quoted 7 lines
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?
If I'm correct above then please send along [1/3] as a separate thing and I can queue it as a backportable hotfix. Then [2/3] and [3/3] as a separate two-patch series for 7.3-rcX.
Quoted 11 lines · Andrew Morton
Quoted 9 lines · On Mon, 17 Aug 2026 05:21:32 -0700 Breno Leitao <[email protected]> wrote:
I think only 1/3 should be getting a Fixes: in v3. The message I am drowning in is the
Bad_offsetone:
get_swap_device: Bad swap offset entry3ffffffc043c5
63d8620ecf93b5("mm/swapfile: usepercpu_refto serialize against concurrent swapoff") added theput_out: label with just thepercpu_ref_put(), so that arm was silent. Thepr_err()landed in v5.19:
Quoted 6 lines
Quoted 4 lines
So, if I need to update it, I will include:
Fixes 23b230ba8ac3 ("mm/swap: print bad swap offset entry in get_swap_device")Cc <[email protected]>OK, so you think that only [1/3] should have cc:stable?
Correct, that is my suggestion. The other patches are more improvements than a proper fix, I would say.
Quoted 12 lines
Quoted 12 lines
Quoted 3 lines
[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.
Quoted 8 lines
Quoted 4 lines
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?
If I'm correct above then please send along [1/3] as a separate thing and I can queue it as a backportable hotfix. Then [2/3] and [3/3] as a separate two-patch series for 7.3-rcX.
ack, I will send [1/3] with the Fixes: tag, and then [2/3] and [3/3] as a new version of this series.
Thanks,
--breno