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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

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>