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

[Kemeng Shi]

on 2/23/2025 1:19 AM, Kairui Song wrote:
> 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:
Thanks for feedback, I will improve commit summary and message in next
verstion.
> 
> Reviewed-by: Kairui Song <kasong@xxxxxxxxxxx>
>