Re: [PATCH 7/9] mm/swapfile: Make folio_put_swap batchable

[Dev Jain]

On 10/03/26 1:59 pm, Kairui Song wrote:
> On Tue, Mar 10, 2026 at 3:47 PM Dev Jain <dev.jain@xxxxxxx> wrote:
>>
>> Teach folio_put_swap to handle a batch of consecutive pages. Note that
>> folio_put_swap already can handle a subset of this: nr_pages == 1 and
>> nr_pages == folio_nr_pages(folio). Generalize this to any nr_pages.
>>
>> Currently we have a not-so-nice logic of passing in subpage == NULL if
>> we mean to exercise the logic on the entire folio, and subpage != NULL if
>> we want to exercise the logic on only that subpage. Remove this
>> indirection, and explicitly pass subpage != NULL, and the number of
>> pages required.
>>
>> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
>> ---
>>  mm/memory.c   |  6 +++---
>>  mm/rmap.c     |  4 ++--
>>  mm/shmem.c    |  6 +++---
>>  mm/swap.h     |  5 +++--
>>  mm/swapfile.c | 13 +++++--------
>>  5 files changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 768646c0b3b6a..8249a9b7083ab 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -5002,7 +5002,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>         if (unlikely(folio != swapcache)) {
>>                 folio_add_new_anon_rmap(folio, vma, address, RMAP_EXCLUSIVE);
>>                 folio_add_lru_vma(folio, vma);
>> -               folio_put_swap(swapcache, NULL);
>> +               folio_put_swap(swapcache, folio_page(swapcache, 0), folio_nr_pages(swapcache));
>>         } else if (!folio_test_anon(folio)) {
>>                 /*
>>                  * We currently only expect !anon folios that are fully
>> @@ -5011,12 +5011,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>                 VM_WARN_ON_ONCE_FOLIO(folio_nr_pages(folio) != nr_pages, folio);
>>                 VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);
>>                 folio_add_new_anon_rmap(folio, vma, address, rmap_flags);
>> -               folio_put_swap(folio, NULL);
>> +               folio_put_swap(folio, folio_page(folio, 0), folio_nr_pages(folio));
>>         } else {
>>                 VM_WARN_ON_ONCE(nr_pages != 1 && nr_pages != folio_nr_pages(folio));
>>                 folio_add_anon_rmap_ptes(folio, page, nr_pages, vma, address,
>>                                          rmap_flags);
>> -               folio_put_swap(folio, nr_pages == 1 ? page : NULL);
>> +               folio_put_swap(folio, page, nr_pages);
>>         }
>>
>>         VM_BUG_ON(!folio_test_anon(folio) ||
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index f6d5b187cf09b..42f6b00cced01 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2293,7 +2293,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>                          * so we'll not check/care.
>>                          */
>>                         if (arch_unmap_one(mm, vma, address, pteval) < 0) {
>> -                               folio_put_swap(folio, subpage);
>> +                               folio_put_swap(folio, subpage, 1);
>>                                 set_pte_at(mm, address, pvmw.pte, pteval);
>>                                 goto walk_abort;
>>                         }
>> @@ -2301,7 +2301,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>                         /* See folio_try_share_anon_rmap(): clear PTE first. */
>>                         if (anon_exclusive &&
>>                             folio_try_share_anon_rmap_pte(folio, subpage)) {
>> -                               folio_put_swap(folio, subpage);
>> +                               folio_put_swap(folio, subpage, 1);
>>                                 set_pte_at(mm, address, pvmw.pte, pteval);
>>                                 goto walk_abort;
>>                         }
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 86ee34c9b40b3..d9d216ea28ecb 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -1716,7 +1716,7 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug,
>>                 /* Swap entry might be erased by racing shmem_free_swap() */
>>                 if (!error) {
>>                         shmem_recalc_inode(inode, 0, -nr_pages);
>> -                       folio_put_swap(folio, NULL);
>> +                       folio_put_swap(folio, folio_page(folio, 0), folio_nr_pages(folio));
> 
> I just realized that we already have a nr_pages variable available
> here, maybe you can just use that?
> 
> Feel free to ignore this if it might touch more code.
> 
>>                 }
>>
>>                 /*
>> @@ -2196,7 +2196,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
>>
>>         nr_pages = folio_nr_pages(folio);
>>         folio_wait_writeback(folio);
>> -       folio_put_swap(folio, NULL);
>> +       folio_put_swap(folio, folio_page(folio, 0), folio_nr_pages(folio));
>>         swap_cache_del_folio(folio);
>>         /*
>>          * Don't treat swapin error folio as alloced. Otherwise inode->i_blocks
>> @@ -2426,7 +2426,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
>>         if (sgp == SGP_WRITE)
>>                 folio_mark_accessed(folio);
>>
>> -       folio_put_swap(folio, NULL);
>> +       folio_put_swap(folio, folio_page(folio, 0), folio_nr_pages(folio));
> 
> Same here, nr_pages seems good enough?

Yup, thanks for your observation.