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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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