linux-kernel - Re: [PATCH v3 6/7] mm, swap: remove swap slot cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMgjq7B2JsnSHgaBEnxxN+9RAexyrNkurYk9w+LG=gDVpJB6Qw@mail.gmail.com>
Date: Tue, 29 Apr 2025 17:28:44 +0800
From: Kairui Song <ryncsn@...il.com>
To: Heiko Carstens <hca@...ux.ibm.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>, 
	Chris Li <chrisl@...nel.org>, Barry Song <v-songbaohua@...o.com>, 
	Hugh Dickins <hughd@...gle.com>, Yosry Ahmed <yosryahmed@...gle.com>, 
	"Huang, Ying" <ying.huang@...ux.alibaba.com>, Baoquan He <bhe@...hat.com>, 
	Nhat Pham <nphamcs@...il.com>, Johannes Weiner <hannes@...xchg.org>, 
	Baolin Wang <baolin.wang@...ux.alibaba.com>, Kalesh Singh <kaleshsingh@...gle.com>, 
	Matthew Wilcox <willy@...radead.org>, linux-kernel@...r.kernel.org, 
	linux-s390@...r.kernel.org
Subject: Re: [PATCH v3 6/7] mm, swap: remove swap slot cache

On Tue, Apr 29, 2025 at 3:31 PM Heiko Carstens <hca@...ux.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@...ux.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.