[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <634a73ce-a24e-01d4-1d00-86272bc78860@huaweicloud.com>
Date: Thu, 15 May 2025 09:05:20 +0800
From: Kemeng Shi <shikemeng@...weicloud.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>, hughd@...gle.com,
akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()
on 5/14/2025 5:24 PM, Baolin Wang wrote:
>
>
> On 2025/5/15 00:50, Kemeng Shi wrote:
>> If multi shmem_unuse() for different swap type is called concurrently,
>> a dead loop could occur as following:
>> shmem_unuse(typeA) shmem_unuse(typeB)
>> mutex_lock(&shmem_swaplist_mutex)
>> list_for_each_entry_safe(info, next, ...)
>> ...
>> mutex_unlock(&shmem_swaplist_mutex)
>> /* info->swapped may drop to 0 */
>> shmem_unuse_inode(&info->vfs_inode, type)
>>
>> mutex_lock(&shmem_swaplist_mutex)
>> list_for_each_entry(info, next, ...)
>> if (!info->swapped)
>> list_del_init(&info->swaplist)
>>
>> ...
>> mutex_unlock(&shmem_swaplist_mutex)
>>
>> mutex_lock(&shmem_swaplist_mutex)
>> /* iterate with offlist entry and encounter a dead loop */
>> next = list_next_entry(info, swaplist);
>> ...
>>
>> Restart the iteration if the inode is already off shmem_swaplist list
>> to fix the issue.
>>
>> Fixes: b56a2d8af9147 ("mm: rid swapoff of quadratic complexity")
>> Signed-off-by: Kemeng Shi <shikemeng@...weicloud.com>
>> ---
>> mm/shmem.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 495e661eb8bb..0fed94c2bc09 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -1505,6 +1505,7 @@ int shmem_unuse(unsigned int type)
>> return 0;
>> mutex_lock(&shmem_swaplist_mutex);
>> +start_over:
>> list_for_each_entry_safe(info, next, &shmem_swaplist, swaplist) {
>> if (!info->swapped) {
>> list_del_init(&info->swaplist);
>> @@ -1530,6 +1531,8 @@ int shmem_unuse(unsigned int type)
>
> next = list_next_entry(info, swaplist);
> if (!info->swapped)
> list_del_init(&info->swaplist);
> if (atomic_dec_and_test(&info->stop_eviction))
> wake_up_var(&info->stop_eviction);
>
> We may still hit the list warning when calling list_del_init() for the off-list info->swaplist? So I hope we can add a check for the possible off-list:
Hello,
When entry is taken off list, it will be initialized to a valid empty entry
with INIT_LIST_HEAD(). So it should be fine to call list_del_init() for
off-list entry.
Please correct me if I miss anything. Thanks!
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 99327c30507c..f5ae5e2d6fb4 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1523,9 +1523,11 @@ int shmem_unuse(unsigned int type)
> cond_resched();
>
> mutex_lock(&shmem_swaplist_mutex);
> - next = list_next_entry(info, swaplist);
> - if (!info->swapped)
> - list_del_init(&info->swaplist);
> + if (!list_empty(&info->swaplist)) {
> + next = list_next_entry(info, swaplist);
> + if (!info->swapped)
> + list_del_init(&info->swaplist);
> + }
> if (atomic_dec_and_test(&info->stop_eviction))
> wake_up_var(&info->stop_eviction);
> if (error)
>
>> wake_up_var(&info->stop_eviction);
>> if (error)
>> break;
>> + if (list_empty(&info->swaplist))
>> + goto start_over;
>> }
>> mutex_unlock(&shmem_swaplist_mutex);
>>
>
Powered by blists - more mailing lists