Re: [PATCH v4 08/12] mm/swapfile: Add batched version of folio_put_swap

[Dev Jain]

On 16/06/26 7:24 pm, David Hildenbrand (Arm) wrote:
> On 5/26/26 08:36, Dev Jain wrote:
>> Add folio_put_swap_pages 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: the caller invokes folio_put_swap_pages() if it wants to
>> operate on a range of pages in the folio (i.e nr_pages may be anything
>> between 1 till folio_nr_pages()), and invokes folio_put_swap() if it
> 
> "between 1 and folio_nr_pages()" ?

Ok.

> 
>> wants to operate on the entire folio.
>>
>> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
>> ---
> 
> 
> [...]
> 
>> +++ b/mm/swap.h
>> @@ -208,7 +208,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
>>  int folio_alloc_swap(struct folio *folio);
>>  int folio_dup_swap_pages(struct folio *folio, struct page *page,
>>  			 unsigned long nr_pages);
>> -void folio_put_swap(struct folio *folio, struct page *page);
>> +void folio_put_swap_pages(struct folio *folio, struct page *page,
>> +			  unsigned long nr_pages);
> 
> Nit \tab\tab (same below)
> 
>>  
>>  /* For internal use */
>>  extern void __swap_cluster_free_entries(struct swap_info_struct *si,
>> @@ -397,7 +398,8 @@ static inline int folio_dup_swap_pages(struct folio *folio, struct page *page,
>>  	return -EINVAL;
>>  }
>>  
> 
> 
> [...]
> 
>>  
>>  	swap_put_entries_cluster(si, swp_offset(entry), nr_pages, false);
>>  }
>> @@ -2499,7 +2497,8 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
>>  		new_pte = pte_mkuffd_wp(new_pte);
>>  setpte:
>>  	set_pte_at(vma->vm_mm, addr, pte, new_pte);
>> -	folio_put_swap(swapcache, folio_file_page(swapcache, swp_offset(entry)));
>> +	folio_put_swap_pages(swapcache,
>> +			     folio_file_page(swapcache, swp_offset(entry)), 1);
>>  out:
>>  	if (pte)
>>  		pte_unmap_unlock(pte, ptl);
> 
> 
> We could consider having a simple folio_put_swap_page() helper. But maybe most
> of these will be converted to batching in the long run either way.

Yep.

> 
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks!

>