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: <CAMgjq7CRWeozoMbkJV43B+06UjakJw6bc0Wn1=2ai=E70=WE3Q@mail.gmail.com>
Date: Fri, 29 Aug 2025 02:05:19 +0800
From: Kairui Song <ryncsn@...il.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: Chris Li <chrisl@...nel.org>, linux-mm@...ck.org, 
	Andrew Morton <akpm@...ux-foundation.org>, Matthew Wilcox <willy@...radead.org>, 
	Hugh Dickins <hughd@...gle.com>, Barry Song <baohua@...nel.org>, Baoquan He <bhe@...hat.com>, 
	Nhat Pham <nphamcs@...il.com>, Kemeng Shi <shikemeng@...weicloud.com>, 
	Ying Huang <ying.huang@...ux.alibaba.com>, Johannes Weiner <hannes@...xchg.org>, 
	David Hildenbrand <david@...hat.com>, Yosry Ahmed <yosryahmed@...gle.com>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Zi Yan <ziy@...dia.com>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/9] mm, swap: always lock and check the swap cache folio
 before use

On Thu, Aug 28, 2025 at 11:41 AM Baolin Wang
<baolin.wang@...ux.alibaba.com> wrote:
>
>
>
> On 2025/8/27 22:35, Kairui Song wrote:
> > On Wed, Aug 27, 2025 at 4:21 PM Chris Li <chrisl@...nel.org> wrote:
> >>
> >> On Fri, Aug 22, 2025 at 12:21 PM Kairui Song <ryncsn@...il.com> wrote:
> >>
> >>> diff --git a/mm/shmem.c b/mm/shmem.c
> >>> index e9d0d2784cd5..b4d39f2a1e0a 100644
> >>> --- a/mm/shmem.c
> >>> +++ b/mm/shmem.c
> >>> @@ -2379,8 +2379,6 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> >>>                          count_vm_event(PGMAJFAULT);
> >>>                          count_memcg_event_mm(fault_mm, PGMAJFAULT);
> >>>                  }
> >>> -       } else {
> >>> -               swap_update_readahead(folio, NULL, 0);
> >>
> >> Also this update readahead move to later might have a similar problem.
> >> All the bail out in the move will lose the readahead status update.
> >>
> >> The readahead deed is already done. Missing the status update seems
> >> incorrect.
> >
> > Thanks for the detailed review.
> >
> > The only change I wanted here is that swap readahead update should be
> > done after checking the folio still corresponds to the swap entry
> > triggering the swapin. That should have slight to none effect compared
> > to before considering the extremely tiny time window. We are only
> > following the convention more strictly.
> >
> > In theory it might even help to reduce false updates: if the folio no
> > longer corresponds to the swap entry, we are hitting an unrelated
> > folio, doing a readahead update will either mislead vma readahead's
> > address hint, or could clean up the readahead flag of an unrelated
> > folio without actually using it. If the folio does get hit in the
> > future, due to the missing readahead flag, the statistic will go
> > wrong.
>
> Yes, that’s what I thought as well.
>
> By the way, can we do it right all at once in patch 1 (I mean the shmem
> changes)?

Hi Baolin,

Yeah it's OK to do so but it's kind of a very slight behaviour change.
Currently patch 1 has zero behaviour change, so maybe just leave it in
this patch where we sanitize all swap cache conventions all at once.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ