Re: [PATCH 20/28] mm, swap: check swap table directly for checking cache

[Baoquan He <bhe@redhat.com>]

On 05/15/25 at 04:17am, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> Instead of looking at the swap map, check swap table directly to tell if
> a swap entry has cache. Prepare for remove SWAP_HAS_CACHE.

But you actually check both the swap table entry and swap map entry in
this patch, or do I miss anything? 

E.g

if (!swap_count(si->swap_map[offset]) && swp_te_is_folio(swp_te))


> 
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/memory.c     | 12 +++++------
>  mm/swap.h       |  6 ++++++
>  mm/swap_state.c | 11 ++++++++++
>  mm/swapfile.c   | 54 +++++++++++++++++++++++--------------------------
>  4 files changed, 48 insertions(+), 35 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index a70624a55aa2..a9a548575e72 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4314,15 +4314,15 @@ static struct folio *__alloc_swap_folio(struct vm_fault *vmf)
>  }
>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
> +static inline int non_swapcache_batch(swp_entry_t entry, unsigned int max_nr)
>  {
> -	struct swap_info_struct *si = swp_info(entry);
> -	pgoff_t offset = swp_offset(entry);
> -	int i;
> +	unsigned int i;
>  
>  	for (i = 0; i < max_nr; i++) {
> -		if ((si->swap_map[offset + i] & SWAP_HAS_CACHE))
> -			return i;
> +		/* Page table lock pins the swap entries / swap device */
> +		if (swap_cache_check_folio(entry))
> +			break;
> +		entry.val++;
>  	}
>  
>  	return i;
> diff --git a/mm/swap.h b/mm/swap.h
> index 467996dafbae..2ae4624a0e48 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -186,6 +186,7 @@ static inline struct address_space *swap_address_space(swp_entry_t entry)
>  extern struct folio *swap_cache_get_folio(swp_entry_t entry);
>  extern struct folio *swap_cache_add_folio(swp_entry_t entry, struct folio *folio,
>  					  void **shadow, bool swapin);
> +extern bool swap_cache_check_folio(swp_entry_t entry);
>  extern void *swap_cache_get_shadow(swp_entry_t entry);
>  /* Below helpers requires the caller to lock the swap cluster. */
>  extern void __swap_cache_del_folio(swp_entry_t entry,
> @@ -395,6 +396,11 @@ static inline void *swap_cache_get_shadow(swp_entry_t end)
>  	return NULL;
>  }
>  
> +static inline bool swap_cache_check_folio(swp_entry_t entry)
> +{
> +	return false;
> +}
> +
>  static inline unsigned int folio_swap_flags(struct folio *folio)
>  {
>  	return 0;
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index c8bb16835612..ea6a1741db5c 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -266,6 +266,17 @@ struct folio *swap_cache_get_folio(swp_entry_t entry)
>  	return folio;
>  }
>  
> +/*
> + * Check if a swap entry has folio cached, may return false positive.
> + * Caller must hold a reference of the swap device or pin it in other ways.
> + */
> +bool swap_cache_check_folio(swp_entry_t entry)
> +{
> +	swp_te_t swp_te;
> +	swp_te = __swap_table_get(swp_cluster(entry), swp_offset(entry));
> +	return swp_te_is_folio(swp_te);
> +}
> +
>  /*
>   * If we are the only user, then try to free up the swap cache.
>   *
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index ef233466725e..0f2a499ff2c9 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -181,15 +181,19 @@ static long swap_usage_in_pages(struct swap_info_struct *si)
>  #define TTRS_FULL		0x4
>  
>  static bool swap_only_has_cache(struct swap_info_struct *si,
> -			      unsigned long offset, int nr_pages)
> +				struct swap_cluster_info *ci,
> +				unsigned long offset, int nr_pages)
>  {
>  	unsigned char *map = si->swap_map + offset;
>  	unsigned char *map_end = map + nr_pages;
> +	swp_te_t entry;
>  
>  	do {
> +		entry = __swap_table_get(ci, offset);

entry is not used in swap_only_has_cache() in this patch.

>  		VM_BUG_ON(!(*map & SWAP_HAS_CACHE));
> -		if (*map != SWAP_HAS_CACHE)
> +		if (*map)
>  			return false;
> +		offset++;
>  	} while (++map < map_end);
>  
>  	return true;
......snip...