Re: [PATCH v2 1/8] mm/swap: add folio_swap_entry() and folio_page_swap_entry()

[Tal Zussman]

On 9/9/26 5:00 PM, David Hildenbrand (Arm) wrote:
> On 9/8/26 17:16, Tal Zussman wrote:
>> A folio in the swap cache occupies folio_nr_pages() contiguous swap
>> entries starting at folio->swap, so a page's swap entry is just
>> folio->swap plus the page's index in the folio. page_swap_entry() hides
>> this behind a compound_head() call, and callers that already have the
>> folio sometimes open-code the arithmetic instead.
>> 
>> Add folio_swap_entry(), which takes a folio and a page index, and
>> folio_page_swap_entry() for callers that have the page.
>> 
>> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
>> ---
>>  include/linux/swap.h | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>> 
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index fc290e29e4a9..1d979e76e78a 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -272,6 +272,39 @@ struct swap_info_struct {
>>  	const struct swap_ops *ops;
>>  };
>>  
>> +/**
>> + * folio_swap_entry - Return the swap entry for a page within a folio.
>> + * @folio: The folio.
>> + * @idx: The index of the page within the folio.
>> + *
>> + * A folio in the swap cache occupies folio_nr_pages() contiguous swap
>> + * entries starting at folio->swap. The caller must ensure the folio is
>> + * in the swap cache and that @idx is within the folio.
>> + */
>> +static inline
>> +swp_entry_t folio_swap_entry(const struct folio *folio, unsigned long idx)
>> +{
>> +	swp_entry_t entry = folio->swap;
>> +
>> +	VM_WARN_ON_ONCE_FOLIO(idx >= folio_nr_pages(folio), folio);
>> +	entry.val += idx;
>> +	return entry;
>> +}
>> +
>> +/**
>> + * folio_page_swap_entry - Return the swap entry of a page in a folio.
>> + * @folio: The folio containing @page.
>> + * @page: A page within @folio.
>> + *
>> + * The caller must ensure the folio is in the swap cache and that @page
>> + * is part of @folio.
>> + */
>> +static inline swp_entry_t folio_page_swap_entry(const struct folio *folio,
>> +		const struct page *page)
>> +{
>> +	return folio_swap_entry(folio, folio_page_idx(folio, page));
>> +}
>> +
>>  static inline swp_entry_t page_swap_entry(struct page *page)
>>  {
>>  	struct folio *folio = page_folio(page);
>> 
> 
> "Return the swap entry for a page within a folio" vs. "Return the swap entry of
> a page in a folio."
> 
> yet only of the variants has a "page" in the name :)
> 
> A bit confusing.
> 
> Not immediately sure how it could be done cleaner. The minority of cases seem to
> use folio_swap_entry.
> 

Yeah... I considered naming them folio_swap_entry() and folio_swap_entry_idx()
(or a variant of that) but I liked the page version more. I can update the
comments to try to differentiate between the two a little more if that would
help. How's "Return the swap entry at an index within a folio"?

> -- 
> Cheers,
> 
> David
>