* Re: [PATCH v3 6/7] mm, swap: remove swap slot cache [not found] ` <20250313165935.63303-7-ryncsn@gmail.com> @ 2025-04-28 13:52 ` Heiko Carstens 2025-04-28 15:31 ` Kairui Song 0 siblings, 1 reply; 4+ messages in thread From: Heiko Carstens @ 2025-04-28 13:52 UTC (permalink / raw) To: Kairui Song Cc: linux-mm, Andrew Morton, Chris Li, Barry Song, Hugh Dickins, Yosry Ahmed, Huang, Ying, Baoquan He, Nhat Pham, Johannes Weiner, Baolin Wang, Kalesh Singh, Matthew Wilcox, linux-kernel, linux-s390 Hi Kairui, On Fri, Mar 14, 2025 at 12:59:34AM +0800, Kairui Song wrote: > From: Kairui Song <kasong@tencent.com> > > Slot cache is no longer needed now, removing it and all related code. ... > Signed-off-by: Kairui Song <kasong@tencent.com> > Reviewed-by: Baoquan He <bhe@redhat.com> > --- > include/linux/swap.h | 3 - > include/linux/swap_slots.h | 28 ---- > mm/Makefile | 2 +- > mm/swap_slots.c | 295 ------------------------------------- > mm/swap_state.c | 8 +- > mm/swapfile.c | 194 ++++++++---------------- > 6 files changed, 67 insertions(+), 463 deletions(-) > delete mode 100644 include/linux/swap_slots.h > delete mode 100644 mm/swap_slots.c ... > diff --git a/mm/swapfile.c b/mm/swapfile.c ... > +swp_entry_t folio_alloc_swap(struct folio *folio) > { > - int order = swap_entry_order(entry_order); > - unsigned long size = 1 << order; > + unsigned int order = folio_order(folio); > + unsigned int size = 1 << order; > struct swap_info_struct *si, *next; > - int n_ret = 0; > + swp_entry_t entry = {}; > + unsigned long offset; > int node; > > + if (order) { > + /* > + * Should not even be attempting large allocations when huge > + * page swap is disabled. Warn and fail the allocation. > + */ > + if (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER) { > + VM_WARN_ON_ONCE(1); > + return entry; > + } > + } This warning triggers on s390. CONFIG_THP_SWAP is disabled and order is 8 when this triggers (reproduced with ltp's swapon01 test case): ------------[ cut here ]------------ WARNING: CPU: 1 PID: 895 at mm/swapfile.c:1227 folio_alloc_swap+0x438/0x440 Modules linked in: CPU: 1 UID: 0 PID: 895 Comm: swapon01 Not tainted 6.14.0-rc6-00227-g0ff67f990bd4-dirty #25 Hardware name: IBM 3931 A01 704 (z/VM 7.4.0) Krnl PSW : 0704d00180000000 000003ffe051210c (folio_alloc_swap+0x43c/0x440) R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:1 PM:0 RI:0 EA:3 Krnl GPRS: 0000000080000000 0000000000000001 0000000000000013 0000000000070000 0000000000000006 fffffef40e9da000 0000000000000000 0000037202fc4000 0000037f00000100 0000000000000100 0000037fe2e4b770 0000037202fc4000 0000000000000000 0000000000000000 000003ffe0512108 0000037fe2e4b3c8 Krnl Code: 000003ffe05120fe: b9160044 llgfr %r4,%r4 000003ffe0512102: c0e5ffdf8c0b brasl %r14,000003ffe0103918 #000003ffe0512108: af000000 mc 0,0 >000003ffe051210c: a7f4fe94 brc 15,000003ffe0511e34 000003ffe0512110: c0040069ce74 brcl 0,000003ffe124bdf8 000003ffe0512116: eb8ff0580024 stmg %r8,%r15,88(%r15) 000003ffe051211c: b90400ef lgr %r14,%r15 000003ffe0512120: e3f0ffb8ff71 lay %r15,-72(%r15) Call Trace: [<000003ffe051210c>] folio_alloc_swap+0x43c/0x440 [<000003ffe050afa6>] add_to_swap+0x56/0xf0 [<000003ffe045fdc0>] shrink_folio_list+0xe80/0x13b0 [<000003ffe0461946>] shrink_inactive_list+0x1a6/0x550 [<000003ffe04624a2>] shrink_lruvec+0x2b2/0x410 [<000003ffe0462840>] shrink_node_memcgs+0x240/0x2d0 [<000003ffe0462986>] shrink_node+0xb6/0x3e0 [<000003ffe046302a>] do_try_to_free_pages+0xda/0x610 [<000003ffe0464d2c>] try_to_free_mem_cgroup_pages+0x14c/0x2a0 [<000003ffe0568270>] try_charge_memcg+0x220/0x5d0 [<000003ffe056867a>] charge_memcg+0x5a/0x270 [<000003ffe056a484>] __mem_cgroup_charge+0x44/0x80 [<000003ffe04acf20>] alloc_anon_folio+0x280/0x610 [<000003ffe04ad45a>] do_anonymous_page+0x1aa/0x5e0 [<000003ffe04af4c4>] __handle_mm_fault+0x244/0x500 [<000003ffe04af820>] handle_mm_fault+0xa0/0x170 [<000003ffe01533f8>] do_exception+0x1d8/0x4a0 [<000003ffe11fb92a>] __do_pgm_check+0x13a/0x220 [<000003ffe120c3ce>] pgm_check_handler+0x11e/0x170 ---[ end trace 0000000000000000 ]--- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 6/7] mm, swap: remove swap slot cache 2025-04-28 13:52 ` [PATCH v3 6/7] mm, swap: remove swap slot cache Heiko Carstens @ 2025-04-28 15:31 ` Kairui Song 2025-04-29 7:31 ` Heiko Carstens 0 siblings, 1 reply; 4+ messages in thread From: Kairui Song @ 2025-04-28 15:31 UTC (permalink / raw) To: Heiko Carstens Cc: linux-mm, Andrew Morton, Chris Li, Barry Song, Hugh Dickins, Yosry Ahmed, Huang, Ying, Baoquan He, Nhat Pham, Johannes Weiner, Baolin Wang, Kalesh Singh, Matthew Wilcox, linux-kernel, linux-s390 On Mon, Apr 28, 2025 at 9:53 PM Heiko Carstens <hca@linux.ibm.com> wrote: > > Hi Kairui, > > On Fri, Mar 14, 2025 at 12:59:34AM +0800, Kairui Song wrote: > > From: Kairui Song <kasong@tencent.com> > > > > Slot cache is no longer needed now, removing it and all related code. > ... > > Signed-off-by: Kairui Song <kasong@tencent.com> > > Reviewed-by: Baoquan He <bhe@redhat.com> > > --- > > include/linux/swap.h | 3 - > > include/linux/swap_slots.h | 28 ---- > > mm/Makefile | 2 +- > > mm/swap_slots.c | 295 ------------------------------------- > > mm/swap_state.c | 8 +- > > mm/swapfile.c | 194 ++++++++---------------- > > 6 files changed, 67 insertions(+), 463 deletions(-) > > delete mode 100644 include/linux/swap_slots.h > > delete mode 100644 mm/swap_slots.c > ... > > diff --git a/mm/swapfile.c b/mm/swapfile.c > ... > > +swp_entry_t folio_alloc_swap(struct folio *folio) > > { > > - int order = swap_entry_order(entry_order); > > - unsigned long size = 1 << order; > > + unsigned int order = folio_order(folio); > > + unsigned int size = 1 << order; > > struct swap_info_struct *si, *next; > > - int n_ret = 0; > > + swp_entry_t entry = {}; > > + unsigned long offset; > > int node; > > > > + if (order) { > > + /* > > + * Should not even be attempting large allocations when huge > > + * page swap is disabled. Warn and fail the allocation. > > + */ > > + if (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER) { > > + VM_WARN_ON_ONCE(1); > > + return entry; > > + } > > + } > > This warning triggers on s390. CONFIG_THP_SWAP is disabled and order > is 8 when this triggers (reproduced with ltp's swapon01 test case): Hi Heiko, Thanks for the report. > > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 895 at mm/swapfile.c:1227 folio_alloc_swap+0x438/0x440 > Modules linked in: > CPU: 1 UID: 0 PID: 895 Comm: swapon01 Not tainted 6.14.0-rc6-00227-g0ff67f990bd4-dirty #25 > Hardware name: IBM 3931 A01 704 (z/VM 7.4.0) > Krnl PSW : 0704d00180000000 000003ffe051210c (folio_alloc_swap+0x43c/0x440) > R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:1 PM:0 RI:0 EA:3 > Krnl GPRS: 0000000080000000 0000000000000001 0000000000000013 0000000000070000 > 0000000000000006 fffffef40e9da000 0000000000000000 0000037202fc4000 > 0000037f00000100 0000000000000100 0000037fe2e4b770 0000037202fc4000 > 0000000000000000 0000000000000000 000003ffe0512108 0000037fe2e4b3c8 > Krnl Code: 000003ffe05120fe: b9160044 llgfr %r4,%r4 > 000003ffe0512102: c0e5ffdf8c0b brasl %r14,000003ffe0103918 > #000003ffe0512108: af000000 mc 0,0 > >000003ffe051210c: a7f4fe94 brc 15,000003ffe0511e34 > 000003ffe0512110: c0040069ce74 brcl 0,000003ffe124bdf8 > 000003ffe0512116: eb8ff0580024 stmg %r8,%r15,88(%r15) > 000003ffe051211c: b90400ef lgr %r14,%r15 > 000003ffe0512120: e3f0ffb8ff71 lay %r15,-72(%r15) > Call Trace: > [<000003ffe051210c>] folio_alloc_swap+0x43c/0x440 > [<000003ffe050afa6>] add_to_swap+0x56/0xf0 > [<000003ffe045fdc0>] shrink_folio_list+0xe80/0x13b0 > [<000003ffe0461946>] shrink_inactive_list+0x1a6/0x550 > [<000003ffe04624a2>] shrink_lruvec+0x2b2/0x410 > [<000003ffe0462840>] shrink_node_memcgs+0x240/0x2d0 > [<000003ffe0462986>] shrink_node+0xb6/0x3e0 > [<000003ffe046302a>] do_try_to_free_pages+0xda/0x610 > [<000003ffe0464d2c>] try_to_free_mem_cgroup_pages+0x14c/0x2a0 > [<000003ffe0568270>] try_charge_memcg+0x220/0x5d0 > [<000003ffe056867a>] charge_memcg+0x5a/0x270 > [<000003ffe056a484>] __mem_cgroup_charge+0x44/0x80 > [<000003ffe04acf20>] alloc_anon_folio+0x280/0x610 > [<000003ffe04ad45a>] do_anonymous_page+0x1aa/0x5e0 > [<000003ffe04af4c4>] __handle_mm_fault+0x244/0x500 > [<000003ffe04af820>] handle_mm_fault+0xa0/0x170 > [<000003ffe01533f8>] do_exception+0x1d8/0x4a0 > [<000003ffe11fb92a>] __do_pgm_check+0x13a/0x220 > [<000003ffe120c3ce>] pgm_check_handler+0x11e/0x170 > ---[ end trace 0000000000000000 ]--- > The !CONFIG_THP_SWAP check existed before because slot cache should reject high order allocation. But slot cache is gone, so large allocation will directly go to the allocator. It was not a meaningful WARN in the first place, and now the allocator should just fail silently for high order allocation, that's totally fine and expected and will just inform the caller to split the folio. I'll just change the WARN_ON condition to `if (order && size > SWAPFILE_CLUSTER)` then, this should silence the WARN. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 6/7] mm, swap: remove swap slot cache 2025-04-28 15:31 ` Kairui Song @ 2025-04-29 7:31 ` Heiko Carstens 2025-04-29 9:28 ` Kairui Song 0 siblings, 1 reply; 4+ messages in thread From: Heiko Carstens @ 2025-04-29 7:31 UTC (permalink / raw) To: Kairui Song Cc: linux-mm, Andrew Morton, Chris Li, Barry Song, Hugh Dickins, Yosry Ahmed, Huang, Ying, Baoquan He, Nhat Pham, Johannes Weiner, Baolin Wang, Kalesh Singh, Matthew Wilcox, linux-kernel, linux-s390 On Mon, Apr 28, 2025 at 11:31:59PM +0800, Kairui Song wrote: > On Mon, Apr 28, 2025 at 9:53 PM Heiko Carstens <hca@linux.ibm.com> wrote: > > > + if (order) { > > > + /* > > > + * Should not even be attempting large allocations when huge > > > + * page swap is disabled. Warn and fail the allocation. > > > + */ > > > + if (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER) { > > > + VM_WARN_ON_ONCE(1); > > > + return entry; > > > + } > > > + } > > The !CONFIG_THP_SWAP check existed before because slot cache should > reject high order allocation. But slot cache is gone, so large > allocation will directly go to the allocator. > > It was not a meaningful WARN in the first place, and now the allocator > should just fail silently for high order allocation, that's totally > fine and expected and will just inform the caller to split the folio. > > I'll just change the WARN_ON condition to `if (order && size > > SWAPFILE_CLUSTER)` then, this should silence the WARN. If I understand your suggestion correctly then this would be the resulting code: diff --git a/mm/swapfile.c b/mm/swapfile.c index 2eff8b51a945..5a7797143948 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1276,7 +1276,7 @@ int folio_alloc_swap(struct folio *folio, gfp_t gfp) * Should not even be attempting large allocations when huge * page swap is disabled. Warn and fail the allocation. */ - if (order && (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER)) { + if (order && size > SWAPFILE_CLUSTER) { VM_WARN_ON_ONCE(1); return -EINVAL; } However, with that change I get this splat (and a few more) instead: ------------[ cut here ]------------ DEBUG_LOCKS_WARN_ON(l->owner != current) WARNING: CPU: 4 PID: 934 at ./include/linux/local_lock_internal.h:52 folio_alloc_swap+0x22e/0x560 Modules linked in: CPU: 4 UID: 0 PID: 934 Comm: swapon01 Not tainted 6.15.0-rc4-00021-gca91b9500108-dirty #2 PREEMPT Hardware name: IBM 3931 A01 704 (z/VM 7.4.0) Krnl PSW : 0704f00180000000 000003ffe05231b2 (folio_alloc_swap+0x232/0x560) R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3 Krnl GPRS: 000003ff80000002 0000000000000003 0000000000000028 000003ffe2fe6c90 0000000000000003 000003ff80000003 000003ffe19fc538 000003720322c000 0000000000000000 000002f2ab38f000 000002f200000100 000003720322c000 0000000000000000 000002f3f0eda278 000003ffe05231ae 0000037febbdb3f8 Krnl Code: 000003ffe05231a2: c020008e69bc larl %r2,000003ffe16f051a 000003ffe05231a8: c0e5ffe2f4dc brasl %r14,000003ffe0181b60 #000003ffe05231ae: af000000 mc 0,0 >000003ffe05231b2: a7f4ff92 brc 15,000003ffe05230d6 000003ffe05231b6: a7080001 lhi %r0,1 000003ffe05231ba: a7a80001 lhi %r10,1 000003ffe05231be: a7980000 lhi %r9,0 000003ffe05231c2: a7f4ff04 brc 15,000003ffe0522fca Call Trace: [<000003ffe05231b2>] folio_alloc_swap+0x232/0x560 [<000003ffe046d2d2>] shrink_folio_list+0xe02/0x12d0 [<000003ffe046edc8>] shrink_inactive_list+0x188/0x5a0 [<000003ffe046f7d4>] shrink_lruvec+0x104/0x400 [<000003ffe046fd0c>] shrink_node_memcgs+0x23c/0x2c0 [<000003ffe046fe3a>] shrink_node+0xaa/0x420 [<000003ffe0470500>] do_try_to_free_pages+0xd0/0x5c0 [<000003ffe0472144>] try_to_free_mem_cgroup_pages+0x144/0x290 [<000003ffe057b33a>] try_charge_memcg+0x1ca/0x420 [<000003ffe057b5ea>] charge_memcg+0x5a/0x1a0 [<000003ffe057d5d4>] __mem_cgroup_charge+0x44/0x160 [<000003ffe04bb6ac>] alloc_anon_folio+0x27c/0x5d0 [<000003ffe04bbbac>] do_anonymous_page+0x1ac/0x7b0 [<000003ffe04bdec2>] __handle_mm_fault+0x212/0x4c0 [<000003ffe04be20c>] handle_mm_fault+0x9c/0x230 [<000003ffe015422c>] do_exception+0x1dc/0x540 [<000003ffe125c560>] __do_pgm_check+0x130/0x220 [<000003ffe126e3ae>] pgm_check_handler+0x11e/0x170 INFO: lockdep is turned off. Last Breaking-Event-Address: [<000003ffe0181c96>] __warn_printk+0x136/0x140 irq event stamp: 4120751 hardirqs last enabled at (4120751): [<000003ffe126d0ee>] _raw_spin_unlock_irq+0x3e/0x80 hardirqs last disabled at (4120750): [<000003ffe126cca8>] _raw_spin_lock_irq+0x98/0xd0 softirqs last enabled at (4104986): [<000003ffe018c1fc>] handle_softirqs+0x2fc/0x550 softirqs last disabled at (4104969): [<000003ffe018b9c6>] __irq_exit_rcu+0x126/0x140 ---[ end trace 0000000000000000 ]--- ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3 6/7] mm, swap: remove swap slot cache 2025-04-29 7:31 ` Heiko Carstens @ 2025-04-29 9:28 ` Kairui Song 0 siblings, 0 replies; 4+ messages in thread From: Kairui Song @ 2025-04-29 9:28 UTC (permalink / raw) To: Heiko Carstens Cc: linux-mm, Andrew Morton, Chris Li, Barry Song, Hugh Dickins, Yosry Ahmed, Huang, Ying, Baoquan He, Nhat Pham, Johannes Weiner, Baolin Wang, Kalesh Singh, Matthew Wilcox, linux-kernel, linux-s390 On Tue, Apr 29, 2025 at 3:31 PM Heiko Carstens <hca@linux.ibm.com> wrote: > > On Mon, Apr 28, 2025 at 11:31:59PM +0800, Kairui Song wrote: > > On Mon, Apr 28, 2025 at 9:53 PM Heiko Carstens <hca@linux.ibm.com> wrote: > > > > + if (order) { > > > > + /* > > > > + * Should not even be attempting large allocations when huge > > > > + * page swap is disabled. Warn and fail the allocation. > > > > + */ > > > > + if (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER) { > > > > + VM_WARN_ON_ONCE(1); > > > > + return entry; > > > > + } > > > > + } > > > > The !CONFIG_THP_SWAP check existed before because slot cache should > > reject high order allocation. But slot cache is gone, so large > > allocation will directly go to the allocator. > > > > It was not a meaningful WARN in the first place, and now the allocator > > should just fail silently for high order allocation, that's totally > > fine and expected and will just inform the caller to split the folio. > > > > I'll just change the WARN_ON condition to `if (order && size > > > SWAPFILE_CLUSTER)` then, this should silence the WARN. > > If I understand your suggestion correctly then this would be the > resulting code: > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 2eff8b51a945..5a7797143948 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1276,7 +1276,7 @@ int folio_alloc_swap(struct folio *folio, gfp_t gfp) > * Should not even be attempting large allocations when huge > * page swap is disabled. Warn and fail the allocation. > */ > - if (order && (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER)) { > + if (order && size > SWAPFILE_CLUSTER) { > VM_WARN_ON_ONCE(1); > return -EINVAL; > } > > However, with that change I get this splat (and a few more) instead: Sorry my bad, the allocator needs to fail silencely, not ignore and go on. So it should be: diff --git a/mm/swapfile.c b/mm/swapfile.c index e727021b8e2c..b86637cfb17a 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1272,13 +1272,22 @@ int folio_alloc_swap(struct folio *folio, gfp_t gfp) VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio); VM_BUG_ON_FOLIO(!folio_test_uptodate(folio), folio); - /* - * Should not even be attempting large allocations when huge - * page swap is disabled. Warn and fail the allocation. - */ - if (order && (!IS_ENABLED(CONFIG_THP_SWAP) || size > SWAPFILE_CLUSTER)) { - VM_WARN_ON_ONCE(1); - return -EINVAL; + if (order) { + /* + * Reject large allocation when THP_SWAP is disabled, + * the caller should split the folio and try again. + */ + if (!IS_ENABLED(CONFIG_THP_SWAP)) + return -EAGAIN; + + /* + * Allocation size should never exceed cluster size + * (HPAGE_PMD_SIZE). + */ + if (size > SWAPFILE_CLUSTER) { + VM_WARN_ON_ONCE(1); + return -EINVAL; + } } local_lock(&percpu_swap_cluster.lock); --- I've tested locally and it seems to work well, I'll send a patch to fix it. ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-29 9:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250313165935.63303-1-ryncsn@gmail.com>
[not found] ` <20250313165935.63303-7-ryncsn@gmail.com>
2025-04-28 13:52 ` [PATCH v3 6/7] mm, swap: remove swap slot cache Heiko Carstens
2025-04-28 15:31 ` Kairui Song
2025-04-29 7:31 ` Heiko Carstens
2025-04-29 9:28 ` Kairui Song
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox