Re: [PATCH 04/12] mm/swap: skip scanning cluster range if it's empty cluster

[Kairui Song]

On Wed, Feb 5, 2025 at 5:27 PM Baoquan He <bhe@xxxxxxxxxx> wrote:
>
> Since ci->lock has been taken when isolating cluster from
> si->free_clusters or taking si->percpu_cluster->next[order],
> it's unnecessary to scan and check the cluster range availability
> if i'ts empty cluster, and this can accelerate the huge page
> swapping.
>
> Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>
> ---
>  mm/swapfile.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 9c9a4ec6d4c6..61efde853eea 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -729,6 +729,9 @@ static bool cluster_scan_range(struct swap_info_struct *si,
>         unsigned long offset, end = start + nr_pages;
>         unsigned char *map = si->swap_map;
>
> +       if (cluster_is_empty(ci))
> +               return true;
> +

Hi Baoquan,

Thanks for the series.

Most commits are looking great, but this one is a bit questionable.
cluster_scan_range is only called by alloc_swap_scan_cluster, and it
already checks if the cluster has enough empty slots to use, so this
might be redundant.

It is possible that cluster_scan_range sees an empty cluster if the
cluster lock was dropped for reclaiming HAS_CACHE, but the chance
should be extremely low, that this might be a negative optimization.