linux-kernel - Re: [PATCH v2 3/4] mm/shmem, swap: improve mthp swapin process

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMgjq7DWMPOy-1z0gCs6DcPo39e95rO10gQwLUtYcZhoqCOL9w@mail.gmail.com>
Date: Mon, 23 Jun 2025 02:48:45 +0800
From: Kairui Song <ryncsn@...il.com>
To: YoungJun Park <youngjun.park@....com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>, 
	Hugh Dickins <hughd@...gle.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>, 
	Matthew Wilcox <willy@...radead.org>, Kemeng Shi <shikemeng@...weicloud.com>, 
	Chris Li <chrisl@...nel.org>, Nhat Pham <nphamcs@...il.com>, Baoquan He <bhe@...hat.com>, 
	Barry Song <baohua@...nel.org>, linux-kernel@...r.kernel.org, gunho.lee@....com, 
	taejoon.song@....com
Subject: Re: [PATCH v2 3/4] mm/shmem, swap: improve mthp swapin process

On Fri, Jun 20, 2025 at 3:53 PM YoungJun Park <youngjun.park@....com> wrote:
> On Fri, Jun 20, 2025 at 01:55:37AM +0800, Kairui Song wrote:
> > +     if (skip_swapcache) {
> > +             swapcache_clear(si, folio->swap, folio_nr_pages(folio));
> > +             folio->swap.val = 0;
> >       }
> > +     if (folio)
> > +             folio_put(folio);
> >       put_swap_device(si);
>
> I really appreciate the patch. great work!

Hi YoungJun,

Thanks for the suggestions.

>
> It's a rather trivial point, but I was wondering if the following change
> might make sense:
> ...
>         if (skip_swapcache)
>                 swapcache_clear(si, folio->swap, folio_nr_pages(folio));
>
>         put_swap_device(si);
>
>         if (folio)
>                 folio_put(folio);
>
>         return error;
> ...
>
> My intention here is to minimize the reference to si,

The si reference is only used to prevent swapoff from releasing the
underlying swap data structures, which is trivial as the overhead is
tiny, and releasing the folio first might help reduce memory pressure
(even more trivial though).

> and from what I understand, this folio has already been allocated and would
> soon disappear. Is it possible to to reduce the clear operation?
> (folio->swap.val = 0)

Right, that might be not needed, but leaving a dangling swap entry in
the folio->private seems not a very good practice to me, so while at,
I added this clearing (folio->private is always cleared for anon
swapin that bypass swap cache). The chance of a failed
"skip_swapcache" swapin is quite low here, so I think it should be OK.

> Just a small suggestion.
> Thank you again for your work!

Thanks for the review!

> Regards,
> Youngjun Park
>