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

Kairui Song posted 19 patches 2 months ago
There is a newer version of this series
[PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper
Posted by Kairui Song 2 months ago
From: Kairui Song <kasong@tencent.com>

No feature change, split the common logic into a stand alone helper to
be reused later.

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
Re: [PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper
Posted by Baoquan He 1 month, 3 weeks ago
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
>
Re: [PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper
Posted by Kairui Song 1 month, 3 weeks ago
On Wed, Dec 17, 2025 at 7:22 PM Baoquan He <bhe@redhat.com> wrote:
>
> 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?
>

Ah you are right, this patch belongs to phase 3 I think, I can drop it
for now, it's just a code rearrangement. Thanks!
Re: [PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper
Posted by Kairui Song 1 month, 3 weeks ago
On Thu, Dec 18, 2025 at 2:37 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Wed, Dec 17, 2025 at 7:22 PM Baoquan He <bhe@redhat.com> wrote:
> >
> > 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?
> >
>
> Ah you are right, this patch belongs to phase 3 I think, I can drop it
> for now, it's just a code rearrangement. Thanks!

Hi Baoquan

While working on V5 I noticed we do need this patch, the patch "mm,
swap: use swap cache as the swap in synchronize layer" needs this
splitted helper as an intermediate step. That usage will be dropped by
the end of this series but I think we can live with this splitted out
helper. The splitted helper and original help will all be gone in
phase 3, and there is no behavior change at all.
Re: [PATCH v4 11/19] mm, swap: split locked entry duplicating into a standalone helper
Posted by Baoquan He 1 month, 3 weeks ago
On 12/20/25 at 01:26am, Kairui Song wrote:
> On Thu, Dec 18, 2025 at 2:37 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > On Wed, Dec 17, 2025 at 7:22 PM Baoquan He <bhe@redhat.com> wrote:
> > >
> > > 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?
> > >
> >
> > Ah you are right, this patch belongs to phase 3 I think, I can drop it
> > for now, it's just a code rearrangement. Thanks!
> 
> Hi Baoquan
> 
> While working on V5 I noticed we do need this patch, the patch "mm,
> swap: use swap cache as the swap in synchronize layer" needs this
> splitted helper as an intermediate step. That usage will be dropped by
> the end of this series but I think we can live with this splitted out
> helper. The splitted helper and original help will all be gone in
> phase 3, and there is no behavior change at all.

That's fine to me, thanks for telling. I have some minor concers in
other patches, will add in v5.