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

[David Hildenbrand (Arm)]

On 9/9/26 19:21, Tal Zussman wrote:
> 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"?
> 

I guess we should call it "page index" in the doc.

-- 
Cheers,

David