linux-kernel - Re: [PATCH 1/6] mm: swap: avoid losting cluster in swap_reclaim_full

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c745e8a1-5c94-4510-56fe-f1528295575c@huaweicloud.com>
Date: Mon, 24 Feb 2025 09:17:43 +0800
From: Kemeng Shi <shikemeng@...weicloud.com>
To: Kairui Song <ryncsn@...il.com>, akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] mm: swap: avoid losting cluster in
 swap_reclaim_full_clusters()



on 2/23/2025 1:19 AM, Kairui Song wrote:
> On Sat, Feb 22, 2025 at 3:12 PM Kemeng Shi <shikemeng@...weicloud.com> 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@...weicloud.com>
>> ---
>>  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@...cent.com>
>