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

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

Posted by David Carlier 4 days, 4 hours ago

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 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;
 }
-- 
2.55.0

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

Posted by Baolin Wang 3 days, 17 hours ago


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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
> Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>   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.

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

Posted by David CARLIER 1 day, 23 hours ago

On Mon, 21 Sept 2026 at 03:45, Baolin Wang
<baolin.wang@linux.alibaba.com> 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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
> > Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >   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.

Cheers

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

Posted by Baolin Wang 1 day, 19 hours ago


On 9/23/26 5:03 AM, David CARLIER wrote:
> On Mon, 21 Sept 2026 at 03:45, Baolin Wang
> <baolin.wang@linux.alibaba.com> 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@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
>>> Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
>>> Signed-off-by: David Carlier <devnexen@gmail.com>
>>> ---
>>>    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@linux.alibaba.com>

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

Posted by Andrew Morton 4 days ago

On Sun, 20 Sep 2026 16:50:02 +0100 David Carlier <devnexen@gmail.com> wrote:

> A failed shmem swapin

from -EIO, for example.

> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=23b25ba3c6bf971f9c57
> Fixes: ac2d3268284b ("mm/swapfile.c: remove the unneeded checking")
> Signed-off-by: David Carlier <devnexen@gmail.com>

I guess we should backport - errors can happen and WARN can crash
machines.  What do others think?