Re: [PATCH 1/6] mm: swap: avoid losting cluster in swap_reclaim_full_clusters()

[Kairui Song]

On Sat, Feb 22, 2025 at 3:12 PM Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> wrote:
>
> If no swap cache is reclaimed, cluster taken off from full_clusters list
> will not be put in any list and may not be reused. Do relocate_cluster
> for such cluster to fix the issue.
>
> Fixes: 3b644773eefda ("mm, swap: reduce contention on device lock")
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> ---
>  mm/swapfile.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 34baefb000b5..e5f58ab86329 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -861,6 +861,10 @@ static void swap_reclaim_full_clusters(struct swap_info_struct *si, bool force)
>                         offset++;
>                 }
>
> +               /* in case no swap cache is reclaimed */
> +               if (ci->flags == CLUSTER_FLAG_NONE)
> +                       relocate_cluster(si, ci);
> +
>                 unlock_cluster(ci);
>                 if (to_scan <= 0)
>                         break;
> --
> 2.30.0

Thanks. A little nick pick, "losting" is not a word, I think you mean "leaking".

And BTW maybe it's better to describe the result of this leak in a bit
more details, "cluster leaking from lists" and "will not be reused"
looked a bit scary at a glance to me. But realizing it's full
clusters, they will be moved back to a list if any slots on them are
freed, so the worst result is inefficiently reclaiming of HAS_CACHE
slots, we didn't really lose these clusters.

We do need to fix it though. So other than the commit summary and
message nitpick:

Reviewed-by: Kairui Song <kasong@xxxxxxxxxxx>