linux-kernel - Re: [PATCH 6/9] mm, swap: use the swap table for the swap cache and switch API

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: <CAMgjq7A5Gr2V9TDFg=_S+SjZ1r1gWXTtRGNofuzAgWNYLMW_DQ@mail.gmail.com>
Date: Tue, 2 Sep 2025 19:58:37 +0800
From: Kairui Song <ryncsn@...il.com>
To: Barry Song <21cnbao@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>, 
	Matthew Wilcox <willy@...radead.org>, Hugh Dickins <hughd@...gle.com>, Chris Li <chrisl@...nel.org>, 
	Baoquan He <bhe@...hat.com>, Nhat Pham <nphamcs@...il.com>, 
	Kemeng Shi <shikemeng@...weicloud.com>, Baolin Wang <baolin.wang@...ux.alibaba.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 6/9] mm, swap: use the swap table for the swap cache and
 switch API

On Tue, Sep 2, 2025 at 6:46 PM Barry Song <21cnbao@...il.com> wrote:
>
> > +
> > +/*
> > + * Helpers for accessing or modifying the swap table of a cluster,
> > + * the swap cluster must be locked.
> > + */
> > +static inline void __swap_table_set(struct swap_cluster_info *ci,
> > +                                   unsigned int off, unsigned long swp_tb)
> > +{
> > +       VM_WARN_ON_ONCE(off >= SWAPFILE_CLUSTER);
> > +       atomic_long_set(&ci->table[off], swp_tb);
> > +}
> > +
> > +static inline unsigned long __swap_table_get(struct swap_cluster_info *ci,
> > +                                            unsigned int off)
> > +{
> > +       VM_WARN_ON_ONCE(off >= SWAPFILE_CLUSTER);
> > +       return atomic_long_read(&ci->table[off]);
> > +}
> > +
>
> Why should this use atomic_long instead of just WRITE_ONCE and
> READ_ONCE?

Hi Barry,

That's a very good question. There are multiple reasons: I wanted to
wrap all access to the swap table to ensure there is no non-atomic
access, since it's almost always wrong to read a folio or shadow value
non-atomically from it. And users should never access swap tables
directly without the wrapper helpers. And in another reply, as Chris
suggested, we can use atomic operations to catch potential issues
easily too.

And most importantly, later phases can make use of things like
atomic_cmpxchg as a fast path to update the swap count of a swap
entry. That's a bit hard to explain for now, short summary is the swap
table will be using a single atomic for both count and folio tracking,
and we'll clean up the folio workflow with swap, so it should be
possible to get an final consistency of swap count by simply locking
the folio, and doing atomic_cmpxchg on swap table with folio locked
will be safe.

For now using atomic doesn't bring any overhead or complexity, only
make it easier to implement other code. So I think it should be good.

>
> Thanks
> Barry
>