On Fri, 10 Jan 2025 11:17:47 +0800 76824143@xxxxxx wrote:
> From: Hao Zhang <zhanghao1@xxxxxxxxxx>
>
> 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;
> +}
> +
