Re: [PATCH v2 5/6] mm: move folio_index to mm/swap.h and remove no longer needed helper

[David Hildenbrand <david@redhat.com>]

On 29.04.25 13:49, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> There are no remaining users of folio_index() outside the mm subsystem.
> Move it to mm/swap.h to co-locate it with swap_cache_index(), eliminating
> a forward declaration, and a function call overhead.
> 
> Also remove the helper that was used to fix circular header dependency
> issue.
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>   include/linux/pagemap.h | 20 --------------------
>   mm/memfd.c              |  1 +
>   mm/migrate.c            |  1 +
>   mm/swap.h               | 23 +++++++++++++++++++++++
>   mm/swapfile.c           |  6 ------
>   5 files changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 1dc3416a9c0d..47b5746d5a65 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -884,26 +884,6 @@ static inline struct page *grab_cache_page_nowait(struct address_space *mapping,
>   			mapping_gfp_mask(mapping));
>   }
>   
> -extern pgoff_t __folio_swap_cache_index(struct folio *folio);
> -
> -/**
> - * folio_index - File index of a folio.
> - * @folio: The folio.
> - *
> - * For a folio which is either in the page cache or the swap cache,
> - * return its index within the address_space it belongs to.  If you know
> - * the page is definitely in the page cache, you can look at the folio's
> - * index directly.
> - *
> - * Return: The index (offset in units of pages) of a folio in its file.
> - */
> -static inline pgoff_t folio_index(struct folio *folio)
> -{
> -	if (unlikely(folio_test_swapcache(folio)))
> -		return __folio_swap_cache_index(folio);
> -	return folio->index;
> -}
> -
>   /**
>    * folio_next_index - Get the index of the next folio.
>    * @folio: The current folio.
> diff --git a/mm/memfd.c b/mm/memfd.c
> index c64df1343059..ab367e61553d 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -20,6 +20,7 @@
>   #include <linux/memfd.h>
>   #include <linux/pid_namespace.h>
>   #include <uapi/linux/memfd.h>
> +#include "swap.h"
>   
>   /*
>    * We need a tag: a new tag would expand every xa_node by 8 bytes,
> diff --git a/mm/migrate.c b/mm/migrate.c
> index f3ee6d8d5e2e..662e5dc44b33 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -50,6 +50,7 @@
>   #include <trace/events/migrate.h>
>   
>   #include "internal.h"
> +#include "swap.h"
>   
>   bool isolate_movable_page(struct page *page, isolate_mode_t mode)
>   {
> diff --git a/mm/swap.h b/mm/swap.h
> index 6f4a3f927edb..91439d0ce50b 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -50,6 +50,24 @@ static inline pgoff_t swap_cache_index(swp_entry_t entry)
>   	return swp_offset(entry) & SWAP_ADDRESS_SPACE_MASK;
>   }
>   
> +/**
> + * folio_index - File index of a folio.
> + * @folio: The folio.
> + *
> + * For a folio which is either in the page cache or the swap cache,
> + * return its index within the address_space it belongs to.  If you know
> + * the folio is definitely in the page cache, you can look at the folio's
> + * index directly.
> + *
> + * Return: The index (offset in units of pages) of a folio in its file.
> + */
> +static inline pgoff_t folio_index(struct folio *folio)
> +{
> +	if (unlikely(folio_test_swapcache(folio)))
> +		return swap_cache_index(folio->swap);
> +	return folio->index;
> +}
> +
>   void show_swap_cache_info(void);
>   void *get_shadow_from_swap_cache(swp_entry_t entry);
>   int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
> @@ -125,6 +143,11 @@ static inline pgoff_t swap_cache_index(swp_entry_t entry)
>   	return 0;
>   }
>   
> +static inline pgoff_t folio_index(struct folio *folio)
> +{
> +	return folio->index;
> +}
> +

Could we just have a single generic function that does something like:

static inline pgoff_t folio_index(struct folio *folio)
{
	if (unlikely(IS_ENABLED(CONFIG_SWAP) &&
		     folio_test_swapcache(folio)))
		return swap_cache_index(folio->swap);
	return folio->index;
}


-- 
Cheers,

David / dhildenb