Re: [PATCH v2 10/15] mm, swap: wrap swap cache replacement with a helper

[Baolin Wang]

On 2025/9/8 18:44, Kairui Song wrote:
> On Mon, Sep 8, 2025 at 11:52 AM Baolin Wang
> <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> > On 2025/9/6 03:13, Kairui Song wrote:
> > > From: Kairui Song <kasong@xxxxxxxxxxx>
> > >
> > > There are currently three swap cache users that are trying to replace an
> > > existing folio with a new one: huge memory splitting, migration, and
> > > shmem replacement. What they are doing is quite similar.
> > >
> > > Introduce a common helper for this. In later commits, they can be easily
> > > switched to use the swap table by updating this helper.
> > >
> > > The newly added helper also makes the swap cache API better defined, and
> > > debugging is easier.
> > >
> > > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> > > ---
> > >    mm/huge_memory.c |  5 ++---
> > >    mm/migrate.c     | 11 +++--------
> > >    mm/shmem.c       | 10 ++--------
> > >    mm/swap.h        |  3 +++
> > >    mm/swap_state.c  | 32 ++++++++++++++++++++++++++++++++
> > >    5 files changed, 42 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > > index 26cedfcd7418..a4d192c8d794 100644
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -3798,9 +3798,8 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> > >                         * NOTE: shmem in swap cache is not supported yet.
> > >                         */
> > >                        if (swap_cache) {
> > > -                             __xa_store(&swap_cache->i_pages,
> > > -                                        swap_cache_index(new_folio->swap),
> > > -                                        new_folio, 0);
> > > +                             __swap_cache_replace_folio(swap_cache, new_folio->swap,
> > > +                                                        folio, new_folio);
> > >                                continue;
> > >                        }
> >
> > IIUC, it doesn't seem like a simple function replacement here. It
> > appears that the original code has a bug: if the 'new_folio' is a large
> > folio after split, we need to iterate over each swap entry of the large
> > swapcache folio and then restore the new 'new_folio'.
>
> That should be OK. We have a check in uniform_split_supported and
> non_uniform_split_supported that swapcache folio can only be splitted
> into order0. And it seems there is no support for splitting pure
> swapcache folio now.

Ah, yes. Better to mention that in the commit message, otherwise, it 
will make people (at least for me) doubt whether this is a 
non-functional change.

With David's comments addressed, feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>

> Maybe we can try to enable and make use of higher order split
> after this series for swapcache. I just had a try to use some hackish
> code to split random folios in the swap cache to larger order, it seems
> fine after this series.