[PATCH] mm/vmscan: extract calculated pressure balance as a function

[PATCH] mm/vmscan: extract calculated pressure balance as a function

[76824143]

From: Hao Zhang <zhanghao1@kylinos.cn>

The pressure balance calculation is only used in the
fraction scenario. Extract into functions to avoid
unnecessary calculations. This doesn't change current
behaviour.

Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 mm/vmscan.c | 73 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 30 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 9a859b7d18d7..97754406dd2e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2367,6 +2367,43 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 	}
 }
 
+static void calculate_pressure_balance(struct scan_control *sc, int swappiness,
+			u64 *fraction, u64 *denominator)
+{
+	unsigned long anon_cost, file_cost, total_cost;
+	unsigned long ap, fp;
+
+	/*
+	 * Calculate the pressure balance between anon and file pages.
+	 *
+	 * The amount of pressure we put on each LRU is inversely
+	 * proportional to the cost of reclaiming each list, as
+	 * determined by the share of pages that are refaulting, times
+	 * the relative IO cost of bringing back a swapped out
+	 * anonymous page vs reloading a filesystem page (swappiness).
+	 *
+	 * Although we limit that influence to ensure no list gets
+	 * left behind completely: at least a third of the pressure is
+	 * applied, before swappiness.
+	 *
+	 * With swappiness at 100, anon and file have equal IO cost.
+	 */
+	total_cost = sc->anon_cost + sc->file_cost;
+	anon_cost = total_cost + sc->anon_cost;
+	file_cost = total_cost + sc->file_cost;
+	total_cost = anon_cost + file_cost;
+
+	ap = swappiness * (total_cost + 1);
+	ap /= anon_cost + 1;
+
+	fp = (MAX_SWAPPINESS - swappiness) * (total_cost + 1);
+	fp /= file_cost + 1;
+
+	fraction[WORKINGSET_ANON] = ap;
+	fraction[WORKINGSET_FILE] = fp;
+	*denominator = ap + fp;
+}
+
 /*
  * Determine how aggressively the anon and file LRU lists should be
  * scanned.
@@ -2379,12 +2416,11 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 {
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
-	unsigned long anon_cost, file_cost, total_cost;
 	int swappiness = sc_swappiness(sc, memcg);
 	u64 fraction[ANON_AND_FILE];
+	bool calculated = false;
 	u64 denominator = 0;	/* gcc */
 	enum scan_balance scan_balance;
-	unsigned long ap, fp;
 	enum lru_list lru;
 
 	/* If we have no swap space, do not bother scanning anon folios. */
@@ -2433,35 +2469,7 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 	}
 
 	scan_balance = SCAN_FRACT;
-	/*
-	 * Calculate the pressure balance between anon and file pages.
-	 *
-	 * The amount of pressure we put on each LRU is inversely
-	 * proportional to the cost of reclaiming each list, as
-	 * determined by the share of pages that are refaulting, times
-	 * the relative IO cost of bringing back a swapped out
-	 * anonymous page vs reloading a filesystem page (swappiness).
-	 *
-	 * Although we limit that influence to ensure no list gets
-	 * left behind completely: at least a third of the pressure is
-	 * applied, before swappiness.
-	 *
-	 * With swappiness at 100, anon and file have equal IO cost.
-	 */
-	total_cost = sc->anon_cost + sc->file_cost;
-	anon_cost = total_cost + sc->anon_cost;
-	file_cost = total_cost + sc->file_cost;
-	total_cost = anon_cost + file_cost;
 
-	ap = swappiness * (total_cost + 1);
-	ap /= anon_cost + 1;
-
-	fp = (MAX_SWAPPINESS - swappiness) * (total_cost + 1);
-	fp /= file_cost + 1;
-
-	fraction[0] = ap;
-	fraction[1] = fp;
-	denominator = ap + fp;
 out:
 	for_each_evictable_lru(lru) {
 		bool file = is_file_lru(lru);
@@ -2544,6 +2552,11 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
 			/* Scan lists relative to size */
 			break;
 		case SCAN_FRACT:
+			if (!calculated) {
+				calculate_pressure_balance(sc, swappiness, fraction, &denominator);
+				calculated = true;
+			}
+
 			/*
 			 * Scan types proportional to swappiness and
 			 * their relative recent reclaim efficiency.

base-commit: eea6e4b4dfb8859446177c32961c96726d0117be
-- 
2.25.1

Re: [PATCH] mm/vmscan: extract calculated pressure balance as a function

[Andrew Morton]

On Fri, 10 Jan 2025 11:17:47 +0800 76824143@qq.com wrote:

> From: Hao Zhang <zhanghao1@kylinos.cn>
> 
> The pressure balance calculation is only used in the
> fraction scenario. Extract into functions to avoid
> unnecessary calculations. This doesn't change current
> behaviour.

The code which has been moved was only executed after setting
`scan_balance = SCAN_FRACT', so there are presently no "unnecessary
calculations", unless I'm missing something.

And the new `bool calculated' had to be added to avoid creating
"unnecessary calculations".

So I'm not seeing any benefit here.

I guess as a little cleanup we could move all that code into the new
calculate_pressure_balance() and call calculate_pressure_balance() in
the same place (just after `scan_balance = SCAN_FRACT;').  The compiler
should inline calculate_pressure_balance() so no runtime effect.

> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2367,6 +2367,43 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
>  	}
>  }
>  
> +static void calculate_pressure_balance(struct scan_control *sc, int swappiness,
> +			u64 *fraction, u64 *denominator)
> +{
> +	unsigned long anon_cost, file_cost, total_cost;
> +	unsigned long ap, fp;
> +
> +	/*
> +	 * Calculate the pressure balance between anon and file pages.
> +	 *
> +	 * The amount of pressure we put on each LRU is inversely
> +	 * proportional to the cost of reclaiming each list, as
> +	 * determined by the share of pages that are refaulting, times
> +	 * the relative IO cost of bringing back a swapped out
> +	 * anonymous page vs reloading a filesystem page (swappiness).
> +	 *
> +	 * Although we limit that influence to ensure no list gets
> +	 * left behind completely: at least a third of the pressure is
> +	 * applied, before swappiness.
> +	 *
> +	 * With swappiness at 100, anon and file have equal IO cost.
> +	 */

And this comment could perhaps be moved into the introductory comment
above the calculate_pressure_balance() site.

I don't know if the end result would represent much improvement though.

> +	total_cost = sc->anon_cost + sc->file_cost;
> +	anon_cost = total_cost + sc->anon_cost;
> +	file_cost = total_cost + sc->file_cost;
> +	total_cost = anon_cost + file_cost;
> +
> +	ap = swappiness * (total_cost + 1);
> +	ap /= anon_cost + 1;
> +
> +	fp = (MAX_SWAPPINESS - swappiness) * (total_cost + 1);
> +	fp /= file_cost + 1;
> +
> +	fraction[WORKINGSET_ANON] = ap;
> +	fraction[WORKINGSET_FILE] = fp;
> +	*denominator = ap + fp;
> +}
> +