Restore swap_space attr aviod krn panic

From: "robin.kuo" <[email protected]>

Restore swap_space attr avoid krn panic

Commit 8b47299a411a ('mm, swap: mark swap address space ro and add context debug check') made the swap address space read-only. It may lead to kernel panic if arch_prepare_to_swap returns a failure under heavy memory pressure as follows,

el1_abort+0x40/0x64
el1h_64_sync_handler+0x48/0xcc
el1h_64_sync+0x84/0x88
errseq_set+0x4c/0xb8 (P)
__filemap_set_wb_err+0x20/0xd0
shrink_folio_list+0xc20/0x11cc
evict_folios+0x1520/0x1be4
try_to_shrink_lruvec+0x27c/0x3dc
shrink_one+0x9c/0x228
shrink_node+0xb3c/0xeac
do_try_to_free_pages+0x170/0x4f0
try_to_free_pages+0x334/0x534
__alloc_pages_direct_reclaim+0x90/0x158
__alloc_pages_slowpath+0x334/0x588
__alloc_frozen_pages_noprof+0x224/0x2fc
__folio_alloc_noprof+0x14/0x64
vma_alloc_zeroed_movable_folio+0x34/0x44
do_pte_missing+0xad4/0x1040
handle_mm_fault+0x4a4/0x790
do_page_fault+0x288/0x5f8
do_translation_fault+0x38/0x54
do_mem_abort+0x54/0xa8

Restore swap address space as not ro to avoid the panic.

---
 mm/swap.h       | 2 +-
 mm/swap_state.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

index d034c13d8dd2..1bd466da3039 100644
@@ -198,7 +198,7 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug);
 void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug);
 
 /* linux/mm/swap_state.c */
-extern struct address_space swap_space __ro_after_init;
+extern struct address_space swap_space __read_mostly;
 static inline struct address_space *swap_address_space(swp_entry_t entry)
 {
 	return &swap_space;

index 5f97c6ae70a2..44d228982521 100644
@@ -37,8 +37,7 @@ static const struct address_space_operations swap_aops = {
 #endif
 };
 
-/* Set swap_space as read only as swap cache is handled by swap table */
-struct address_space swap_space __ro_after_init = {
+struct address_space swap_space __read_mostly = {
 	.a_ops = &swap_aops,
 };
 

2.45.2

Thanks, and welcome to Linux ;)

Well damn. We have checking tools which should be able to detect this:
we referenced something which was dropped from kernel memory in the calling function's context. i don't know who I should report this to.

I shall add cc:stable to this patch, to help ensure that it is backported into 6.18.x.

Hi Robin

Thanks very much for the patch! I'm fine with it with some suggestions:

Marking it as RO was intentional to avoid silent unexpected usages of the swap space, the swap space is hollow now, as all content is now in the swap table.

For example the one you posted, calling filemap_set_wb_err for the swap space is meaningless, the error info will be ignored without any notice.
In the comment for mapping_set_error (wrapper for filemap_set_wb_err):
* When a writeback error occurs, most filesystems will want to call
* mapping_set_error to record the error in the mapping so that it can be
* reported when the application calls fsync(2).

So far there is no fsync for swap or other ways to report mapping errors to userspace. For example, __end_swap_bio_write will just print an error log if writetback failed. I went through the code, currently only arch_prepare_to_swap will cause a call to filemap_set_wb_err. Swap is not a persistent storage like normal FS, so simply bouncing the writeback folio back as active is good enough for almost all cases, like the one in end_swap_bio_write.

Catching potential issues with RO/panic is overkill indeed. So I'm fine with dropping the RO by default. But perhaps we can keep the RO attribute at least for VM_DEBUG build? Like this:

index eba4c04ad039..307cc71d3635 100644
@@ -220,7 +220,12 @@ int swap_writeout(struct folio *folio, struct

swap_iocb **swap_plug);
void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug);

/* linux/mm/swap_state.c */
-extern struct address_space swap_space __ro_after_init; +#ifdef CONFIG_DEBUG_VM
+#define __swap_space_attr __ro_after_init
+#else
+#define __swap_space_attr __read_mostly
+#endif
+extern struct address_space swap_space __swap_space_attr;
static inline struct address_space *swap_address_space(swp_entry_t entry)

 {
        return &swap_space;

index 3916d144eddc..11273e0ec811 100644
@@ -38,7 +38,7 @@ static const struct address_space_operations swap_aops = {
 };
 
 /* Set swap_space as read only as swap cache is handled by swap table */
-struct address_space swap_space __ro_after_init = {
+struct address_space swap_space __swap_space_attr = {
        .a_ops = &swap_aops,
 };
 

---

And as for this particular error marking issue you posted, maybe we should just drop the handle_write_error I think. That helper was useful when the filesystems are still using the .writepage interface, but now all filesystem have switched to .writepages and only swap may call that helper, which is no longer helpful, as the error info is completely ignored with no report back to userspace. Maybe we need something like this:

index 39f0336d9c29..62fe433aac0b 100644
@@ -477,27 +477,6 @@ static int reclaimer_offset(struct scan_control *sc)
        return PGSTEAL_DIRECT - PGSTEAL_KSWAPD;
 }
 
-/*
- * We detected a synchronous write error writing a folio out.  Probably
- * -ENOSPC.  We need to propagate that into the address_space for a subsequent
- * fsync(), msync() or close().
- *
- * The tricky part is that after writepage we cannot touch the mapping: nothing
- * prevents it from being freed up.  But we have a ref on the folio and once
- * that folio is locked, the mapping is pinned.
- *
- * We're allowed to run sleeping folio_lock() here because we know

the caller has
- * __GFP_FS.
- */
-static void handle_write_error(struct address_space *mapping,
- struct folio *folio, int error)
-{
- folio_lock(folio);
- if (folio_mapping(folio) == mapping)
- mapping_set_error(mapping, error);
- folio_unlock(folio);
-}
-
static bool skip_throttle_noprogress(pg_data_t *pgdat)

 {
        int reclaimable = 0, write_pending = 0;
@@ -650,9 +629,10 @@ static pageout_t writeout(struct folio *folio,
struct address_space *mapping,
        else
                res = swap_writeout(folio, plug);

- if (res < 0)
- handle_write_error(mapping, folio, res);
- if (res == AOP_WRITEPAGE_ACTIVATE) {
+ if (res < 0) {
+ pr_alert_ratelimited("Swap failure of folio (order:
%d, error: %d)\n",
+ folio_order(folio), res);

+       } else if (res == AOP_WRITEPAGE_ACTIVATE) {
                folio_clear_reclaim(folio);
                return PAGE_ACTIVATE;
        }

Also a nitpick, you may want to add "mm, swap:" prefix to the patch summary.

For a stable backport simply making it not RO is simpler and better. We can do the cleanup and add debug attr as I mentioned in another reply later.

So for this change with Cc stable:

From: "robin.kuo" <[email protected]>

Hi Kairui, Andrew,

Thanks for your review and suggestions!
I update the patch summary to "mm, swap:" as suggested. Regarding the debug attribute, I will also try the version you suggested locally (using __swap_space_attr).

Thanks again for your help!

Best regards,
Robin

robin.kuo (1):
mm, swap: Restore swap_space attr aviod krn panic

 mm/swap.h       | 2 +-
 mm/swap_state.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

2.45.2

From: "robin.kuo" <[email protected]>

commit 8b47299a411a ("mm, swap: mark swap address space ro and add context debug check") made the swap address space read-only. It may lead to kernel panic if arch_prepare_to_swap returns a failure under heavy memory pressure as follows,

el1_abort+0x40/0x64
el1h_64_sync_handler+0x48/0xcc
el1h_64_sync+0x84/0x88
errseq_set+0x4c/0xb8 (P)
__filemap_set_wb_err+0x20/0xd0
shrink_folio_list+0xc20/0x11cc
evict_folios+0x1520/0x1be4
try_to_shrink_lruvec+0x27c/0x3dc
shrink_one+0x9c/0x228
shrink_node+0xb3c/0xeac
do_try_to_free_pages+0x170/0x4f0
try_to_free_pages+0x334/0x534
__alloc_pages_direct_reclaim+0x90/0x158
__alloc_pages_slowpath+0x334/0x588
__alloc_frozen_pages_noprof+0x224/0x2fc
__folio_alloc_noprof+0x14/0x64
vma_alloc_zeroed_movable_folio+0x34/0x44
do_pte_missing+0xad4/0x1040
handle_mm_fault+0x4a4/0x790
do_page_fault+0x288/0x5f8
do_translation_fault+0x38/0x54
do_mem_abort+0x54/0xa8

Restore swap address space as not ro to avoid the panic.

---
 mm/swap.h       | 2 +-
 mm/swap_state.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

index d034c13d8dd2..1bd466da3039 100644
@@ -198,7 +198,7 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug);
 void __swap_writepage(struct folio *folio, struct swap_iocb **swap_plug);
 
 /* linux/mm/swap_state.c */
-extern struct address_space swap_space __ro_after_init;
+extern struct address_space swap_space __read_mostly;
 static inline struct address_space *swap_address_space(swp_entry_t entry)
 {
 	return &swap_space;

index 5f97c6ae70a2..44d228982521 100644
@@ -37,8 +37,7 @@ static const struct address_space_operations swap_aops = {
 #endif
 };
 
-/* Set swap_space as read only as swap cache is handled by swap table */
-struct address_space swap_space __ro_after_init = {
+struct address_space swap_space __read_mostly = {
 	.a_ops = &swap_aops,
 };
 

2.45.2