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

[Kairui Song <ryncsn@gmail.com>]

On Tue, Sep 2, 2025 at 6:46 PM Barry Song <21cnbao@gmail.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
>