Re: [PATCH 1/7] mm: shmem: correctly pass alloced parameter to shmem_recalc_inode() to avoid WARN_ON()

[Kemeng Shi]

on 6/11/2025 3:29 PM, Baolin Wang wrote:
> 
> 
> On 2025/6/10 09:02, Kemeng Shi wrote:
>>
>>
>> on 6/9/2025 8:46 AM, Kemeng Shi wrote:
>>>
>>>
>>> on 6/7/2025 2:11 PM, Baolin Wang wrote:
>>>>
>>>>
>>>> On 2025/6/6 06:10, Kemeng Shi wrote:
>>>>> As noted in the comments, we need to release block usage for swap entry
>>>>> which was replaced with poisoned swap entry. However, no block usage is
>>>>> actually freed by calling shmem_recalc_inode(inode, -nr_pages, -nr_pages).
>>>>> Instead, call shmem_recalc_inode(inode, 0, -nr_pages) can correctly release
>>>>> the block usage.
>>>>>
>>>>> Fixes: 6cec2b95dadf7 ("mm/shmem: fix infinite loop when swap in shmem error at swapoff time")
>>>>> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>>    mm/shmem.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>>>> index 4b42419ce6b2..e27d19867e03 100644
>>>>> --- a/mm/shmem.c
>>>>> +++ b/mm/shmem.c
>>>>> @@ -2145,7 +2145,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
>>>>>         * won't be 0 when inode is released and thus trigger WARN_ON(i_blocks)
>>>>>         * in shmem_evict_inode().
>>>>>         */
>>>>> -    shmem_recalc_inode(inode, -nr_pages, -nr_pages);
>>>>> +    shmem_recalc_inode(inode, 0, -nr_pages);
>>>>>        swap_free_nr(swap, nr_pages);
>>>>>    }
>>>>
>>>> Have you tested your patch? When I inject an error to test your patch, the following issue will be triggered:As all issues are hard to trigger, I only run some simple test to ensure normal
>>> process is fine. Could you share how to inject the error to trigger following
>>> issue. I will have a deep look. Thanks
>> Sorry that the message is truncated. I mean I only test normal process is fine.
> 
> Please also test the swapin error case you try to fix. Obviously your current patch is incorrect.
> 
>> Besides, I think there is another long-standing issue which could trigger the
>> following issue. Here is the issue which is possible to blame:
>> When swap entry is replaced with error entry in shmem_set_folio_swapin_error(),
>> we will reduce info->swapped. Afterwards, error entry could be deleted in
>> shmem_undo_range() and the info->swapped is reduced again. As a result, we
>> reduce info->swapped twice for a single swap entry.
> 
> OK. So you should do something like in shmem_find_swap_entries() to avoid decreasing info->swapped again.
> 
> entry = radix_to_swp_entry(folio);
> /*
> * swapin error entries can be found in the mapping. But they're
> * deliberately ignored here as we've done everything we can do.
> */
> if (swp_type(entry) != type)
>     continue;
> 
>> A simple way to confirm this is injecting error to original code. Could you
>> share how to trigger the issue or could you do the same test to original code?
> 
> Yes, original code is good.
I still suspect that it's another long-standing issue which is triggerd by
this by accident.
> 
> A simple test procedure is to allocate some shmem memory and swap them out, then swap in the shmem while injecting an error to trigger the swap-in error case, and finally unmap the program.
> 
Sure, will fix the mentiond long-standing issue first and try to run this
test.
I will appreciate if you can share your test code if it's convenient.

Thanks