Re: [PATCH] mm/shmem: don't release a swapin-error marker as a swap entry

[Baolin Wang]

On 9/23/26 5:03 AM, David CARLIER wrote:
> On Mon, 21 Sept 2026 at 03:45, Baolin Wang
> <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> > On 9/20/26 11:50 PM, David Carlier wrote:
> > > A failed shmem swapin frees the swap slot and leaves a PTE_MARKER_POISONED
> > > entry in the page cache. On truncate or eviction shmem_free_swap() passes
> > > that marker to swap_put_entries_direct(), which warns because it is not a
> > > swap entry. There is nothing left to release either way.
> > >
> > > Skip the release for non-swap entries, as every other caller already does.
> > >
> > > Reported-by: syzbot+23b25ba3c6bf971f9c57@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
> > > Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
> > > Signed-off-by: David Carlier <devnexen@xxxxxxxxx>
> > > ---
> > >    mm/shmem.c | 6 ++++--
> > >    1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/shmem.c b/mm/shmem.c
> > > index b572c60f2af8..94f2c59c8cfc 100644
> > > --- a/mm/shmem.c
> > > +++ b/mm/shmem.c
> > > @@ -1184,6 +1184,7 @@ static long shmem_free_swap(struct address_space *mapping,
> > >                            pgoff_t index, pgoff_t end, void *radswap)
> > >    {
> > >        XA_STATE(xas, &mapping->i_pages, index);
> > > +     const softleaf_t swp = radix_to_swp_entry(radswap);
> > >        unsigned int nr_pages = 0;
> > >        pgoff_t base;
> > >        void *entry;
> > > @@ -1200,8 +1201,9 @@ static long shmem_free_swap(struct address_space *mapping,
> > >        }
> > >        xas_unlock_irq(&xas);
> > >
> > > -     if (nr_pages)
> > > -             swap_put_entries_direct(radix_to_swp_entry(radswap), nr_pages);
> > > +     /* A swapin-error marker holds no swap slot, so just drop it. */
> > > +     if (nr_pages && softleaf_is_swap(swp))
> > > +             swap_put_entries_direct(swp, nr_pages);
> > >
> > >        return nr_pages;
> > >    }
> >
> > Makes sense to me.
> >
> > But another issue caught my eye. Since we already call
> > shmem_recalc_inode() to decrement 'info->swapped' when handling poisoned
> > entries in shmem_set_folio_swapin_error(), shmem_free_swap() still
> > returning 'nr_pages' for poisoned entries would cause a second call to
> > shmem_recalc_inode(inode, 0, -nr_swaps_freed), thus triggerring
> > WARN_ON(i_blocks) in shmem_evict_inode().
> >
> > I'm not sure if you've observed this warning. IIUC, shmem_free_swap()
> > should return 0 for poisoned entries.
>
> Hi Baolin,
>
> Thanks for the review.
>
> I think returning nr_pages is needed. shmem_set_folio_swapin_error()
> decrements both alloced and swapped, so shmem_recalc_inode() sees
> nothing freed and the block stays accounted. The second decrement in
> shmem_undo_range() is what finally releases it. Returning 0 there
> would leave i_blocks non-zero at evict time.

Yes, you are right. My memory is coming back :). So,

Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>