Re: [PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper

[Baoquan He <bhe@redhat.com>]

On 12/05/25 at 03:29am, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> No feature change, split the common logic into a stand alone helper to
                                                   ~~~~~~~~~~~
                                                    standalone, typo?
> be reused later.

In phase 2, I saw the newly added swap_dup_entries() is only called by
__swap_duplicate(). The 'reused later' means?

> 
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/swapfile.c | 62 +++++++++++++++++++++++++++++------------------------------
>  1 file changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 2703dfafc632..d9d943fc7b8d 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3666,26 +3666,14 @@ void si_swapinfo(struct sysinfo *val)
>   * - swap-cache reference is requested but the entry is not used. -> ENOENT
>   * - swap-mapped reference requested but needs continued swap count. -> ENOMEM
>   */
> -static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
> +static int swap_dup_entries(struct swap_info_struct *si,
> +			    struct swap_cluster_info *ci,
> +			    unsigned long offset,
> +			    unsigned char usage, int nr)
>  {
> -	struct swap_info_struct *si;
> -	struct swap_cluster_info *ci;
> -	unsigned long offset;
> -	unsigned char count;
> -	unsigned char has_cache;
> -	int err, i;
> -
> -	si = swap_entry_to_info(entry);
> -	if (WARN_ON_ONCE(!si)) {
> -		pr_err("%s%08lx\n", Bad_file, entry.val);
> -		return -EINVAL;
> -	}
> -
> -	offset = swp_offset(entry);
> -	VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTER);
> -	ci = swap_cluster_lock(si, offset);
> +	int i;
> +	unsigned char count, has_cache;
>  
> -	err = 0;
>  	for (i = 0; i < nr; i++) {
>  		count = si->swap_map[offset + i];
>  
> @@ -3693,25 +3681,20 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
>  		 * Allocator never allocates bad slots, and readahead is guarded
>  		 * by swap_entry_swapped.
>  		 */
> -		if (WARN_ON(swap_count(count) == SWAP_MAP_BAD)) {
> -			err = -ENOENT;
> -			goto unlock_out;
> -		}
> +		if (WARN_ON(swap_count(count) == SWAP_MAP_BAD))
> +			return -ENOENT;
>  
>  		has_cache = count & SWAP_HAS_CACHE;
>  		count &= ~SWAP_HAS_CACHE;
>  
>  		if (!count && !has_cache) {
> -			err = -ENOENT;
> +			return -ENOENT;
>  		} else if (usage == SWAP_HAS_CACHE) {
>  			if (has_cache)
> -				err = -EEXIST;
> +				return -EEXIST;
>  		} else if ((count & ~COUNT_CONTINUED) > SWAP_MAP_MAX) {
> -			err = -EINVAL;
> +			return -EINVAL;
>  		}
> -
> -		if (err)
> -			goto unlock_out;
>  	}
>  
>  	for (i = 0; i < nr; i++) {
> @@ -3730,14 +3713,31 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
>  			 * Don't need to rollback changes, because if
>  			 * usage == 1, there must be nr == 1.
>  			 */
> -			err = -ENOMEM;
> -			goto unlock_out;
> +			return -ENOMEM;
>  		}
>  
>  		WRITE_ONCE(si->swap_map[offset + i], count | has_cache);
>  	}
>  
> -unlock_out:
> +	return 0;
> +}
> +
> +static int __swap_duplicate(swp_entry_t entry, unsigned char usage, int nr)
> +{
> +	int err;
> +	struct swap_info_struct *si;
> +	struct swap_cluster_info *ci;
> +	unsigned long offset = swp_offset(entry);
> +
> +	si = swap_entry_to_info(entry);
> +	if (WARN_ON_ONCE(!si)) {
> +		pr_err("%s%08lx\n", Bad_file, entry.val);
> +		return -EINVAL;
> +	}
> +
> +	VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset % SWAPFILE_CLUSTER);
> +	ci = swap_cluster_lock(si, offset);
> +	err = swap_dup_entries(si, ci, offset, usage, nr);
>  	swap_cluster_unlock(ci);
>  	return err;
>  }
> 
> -- 
> 2.52.0
>