[PATCH 0/3] Cleanup scan_swap_map_slots() a little

[PATCH 0/3] Cleanup scan_swap_map_slots() a little

[Wei Yang]

Function scan_swap_map_slots() is used to iterate swap_map[] array for an
available swap entry. While after several optimizations, e.g. for ssd case,
the logic of this function is a little not easy to catch.

This patch set tries to clean up the logic a little:

  * shows the ssd/non-ssd case is handled mutually exclusively
  * remove some unnecessary goto for ssd case

Wei Yang (3):
  mm/swapfile.c: offset is only used when there is more slots
  mm/swapfile.c: explicitly show ssd/non-ssd is handled mutually
    exclusive
  mm/swapfile.c: remove the unnecessary goto for SSD case

 mm/swapfile.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

-- 
2.23.0

[PATCH 1/3] mm/swapfile.c: offset is only used when there is more slots

[Wei Yang]

When si->cluster_nr is zero, function would reach done and return. The
increased offset would not be used any more. This means we can move the
offset increment into the if clause.

This brings a further code cleanup possibility.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/swapfile.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 6b6e41967bf3..52afb74fc3d1 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -871,11 +871,9 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 		else
 			goto done;
 	}
-	/* non-ssd case */
-	++offset;
 
 	/* non-ssd case, still more slots in cluster? */
-	if (si->cluster_nr && !si->swap_map[offset]) {
+	if (si->cluster_nr && !si->swap_map[++offset]) {
 		--si->cluster_nr;
 		goto checks;
 	}
-- 
2.23.0

[PATCH 2/3] mm/swapfile.c: explicitly show ssd/non-ssd is handled mutually exclusive

[Wei Yang]

The code shows if this is ssd, it will jump to specific tag and skip the
following code for non-ssd.

Let's use "else if" to explicitly show the mutually exclusion for
ssd/non-ssd to reduce ambiguity.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/swapfile.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index 52afb74fc3d1..adf48d4b1b63 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -760,9 +760,7 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 			goto checks;
 		else
 			goto scan;
-	}
-
-	if (unlikely(!si->cluster_nr--)) {
+	} else if (unlikely(!si->cluster_nr--)) {
 		if (si->pages - si->inuse_pages < SWAPFILE_CLUSTER) {
 			si->cluster_nr = SWAPFILE_CLUSTER - 1;
 			goto checks;
@@ -870,10 +868,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 			goto checks;
 		else
 			goto done;
-	}
-
-	/* non-ssd case, still more slots in cluster? */
-	if (si->cluster_nr && !si->swap_map[++offset]) {
+	} else if (si->cluster_nr && !si->swap_map[++offset]) {
+		/* non-ssd case, still more slots in cluster? */
 		--si->cluster_nr;
 		goto checks;
 	}
-- 
2.23.0

[PATCH 3/3] mm/swapfile.c: remove the unnecessary goto for SSD case

[Wei Yang]

Now we can see there is redundant goto for SSD case. In these two
places, we can just let the code walk through to the correct tag instead
of explicitly jump to it.

Let's remove them for better readability.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/swapfile.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/mm/swapfile.c b/mm/swapfile.c
index adf48d4b1b63..f903e5a165d5 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -756,9 +756,7 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 
 	/* SSD algorithm */
 	if (si->cluster_info) {
-		if (scan_swap_map_try_ssd_cluster(si, &offset, &scan_base))
-			goto checks;
-		else
+		if (!scan_swap_map_try_ssd_cluster(si, &offset, &scan_base))
 			goto scan;
 	} else if (unlikely(!si->cluster_nr--)) {
 		if (si->pages - si->inuse_pages < SWAPFILE_CLUSTER) {
@@ -866,8 +864,6 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
 	if (si->cluster_info) {
 		if (scan_swap_map_try_ssd_cluster(si, &offset, &scan_base))
 			goto checks;
-		else
-			goto done;
 	} else if (si->cluster_nr && !si->swap_map[++offset]) {
 		/* non-ssd case, still more slots in cluster? */
 		--si->cluster_nr;
-- 
2.23.0