Re: [PATCH 06/16] mm/shmem: hold off eviction in shmem_writeout()

[David Hildenbrand (Arm)]

On 10/26/26 17:13, Kiryl Shutsemau wrote:
> On Mon, Oct 26, 2026 at 10:40:39AM +0100, David Hildenbrand (Arm) wrote:
>> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
>>
>> Once shmem_writeout() has moved the folio from the page cache to the
>> swap cache, the folio lock no longer keeps the inode alive: truncation
>> and eviction find a swap entry, free it and do not wait for anybody.
>> That did not matter as long as swap_writeout() was the last thing the
>> function did.  But since it puts the folio back when swap_writeout()
>> returns AOP_WRITEPAGE_ACTIVATE, it uses the mapping and the inode
>> afterwards:
>>
>>   reclaim                               last close of an unlinked file
>>   shmem_writeout()
>>     shmem_delete_from_page_cache()
>>     swap_writeout()
>>       zswap_store() fails               shmem_evict_inode()
>>       writeback is disabled               shmem_truncate_range()
>>       return AOP_WRITEPAGE_ACTIVATE         shmem_free_swap()
>>                                         destroy_inode()
>>                                         ... RCU grace period, freed
>>     shmem_add_to_page_cache(mapping)
>>       xas_lock_irq()            <- use after free
>>
>> swap_writeout() behaves that way if zswap is enabled, cannot take the
>> folio and the memcg or one of its ancestors has memory.zswap.writeback
>> set to 0.
>>
>> shmem_unuse() already added infrastructure to deal with that.
>> Reuse info->stop_eviction to protect the inode during writeout. Increment
>> it before removing the folio from the page cache, and decrement it after
>> swap_writeout() and any attempted reinsertion and inode accounting.
>> In shmem_evict_inode(), wait for the counter to reach zero even if
>> swapoff has already removed the inode from the swap list. Add some helpers
>> to make the code easier to digest.
>>
>> Fix generated by an LLM and sent by Josef Bacik in private.
>>
>> Fixes: 6344a6d9ce13 ("mm/shmem: writeout free swap if swap_writeout() reactivates")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
>> [ david: code cleanups; improve description ]
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> 
> ->stop_eviction is ugly, but, oh, well...

yeah, I was not particularly happy to learn that this exists ...

Thanks everybody for the review!

-- 
Cheers,

David