1. Patchew
  2. linux
  3. [PATCH 0/5] Some random fixes and cleanup to shmem
  4. View patch

[PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()

Kemeng Shi posted 5 patches 9 months ago
There is a newer version of this series
[PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()
Posted by Kemeng Shi 9 months ago 
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@huaweicloud.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)
 			wake_up_var(&info->stop_eviction);
 		if (error)
 			break;
+		if (list_empty(&info->swaplist))
+			goto start_over;
 	}
 	mutex_unlock(&shmem_swaplist_mutex);
 
-- 
2.30.0
Re: [PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()
Posted by Baolin Wang 9 months ago 

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@huaweicloud.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:

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);
>
Re: [PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()
Posted by Kemeng Shi 9 months ago 

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@huaweicloud.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);
>>   
>
Re: [PATCH 3/5] mm/shmem: Fix potential dead loop in shmem_unuse()
Posted by Baolin Wang 9 months ago 

On 2025/5/15 09:05, Kemeng Shi wrote:
> 
> 
> 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@huaweicloud.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!

Ah, yes. I got confused with list_del(), but I still think we should not 
continue to operate on an off-list entry.

>> 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);
>>>    
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.