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

[Baolin Wang]

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.

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.