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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2025/9/8 18:44, Kairui Song wrote:
> On Mon, Sep 8, 2025 at 11:52 AM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>>
>>
>> On 2025/9/6 03:13, Kairui Song wrote:
>>> From: Kairui Song <kasong@tencent.com>
>>>
>>> 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@tencent.com>
>>> ---
>>>    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@linux.alibaba.com>

> 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.