Re: [PATCH mm-next] mm/swap: free swap cache folios the zap path gave up on

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 10/20/26 2:08 AM, Nhat Pham wrote:
> On Mon, Oct 19, 2026 at 7:37 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
[...]
>>>> Right.. But I think it will be much better to avoid such folios from
>>>> hunging in the swap cache in the first place instead of having a
>>>> worker to reclaim them. I'm not strongly against adding a reclaimer if
>>>> other approaches are too difficult at this point though.
>>> Also, IIRC, dropbehind can only be handled in task context, right?
>>> We'd need to make readahead bios to complete in task context
>>> (BIO_COMPLETE_IN_TASK). I wonder if this has any perf implications.
>>>
>>> Another idea: how about, in swap_read_folio(), where we are guaranteed
>>> to have the folio's lock, we check if folio is mapped. If it's not,
>>> invoke folio_free_swap(), unlock, put, then drop that folio from the
>>> submit batch?
>> At that point the swap count is still 1; the zap comes while the read
>> is in flight. Checking after the read works for the synchronous zswap
>> path, which I tried first, but the bdev read is unlocked in
>> swap_bio_read_end_io(), in interrupt context, where the slot cannot be
>> freed.
> Zap drops swap count first, then try to free swap cache. You're saying
> the read check the swap count before zap dropping it, but still holds
> the lock by the time zap try to free swap cache? :(



Yes, exactly that. With the check in swap_read_folio():

thread A (readahead)                   thread B
swap_cache_alloc_folio(B's entry)
   folio locked, added to cache
   swap count 1 (B's PTE)
swap_read_folio()
   folio_free_swap()
     count is 1, nothing to do
   zswap_load() / submit_bio()
   .                                    MADV_DONTNEED
   .                                      swap count 1 -> 0
   .                                      __try_to_reclaim_swap()
   .                                        folio_trylock() fails
   .                                        gives up
   read completes
folio_unlock()
folio_put()
-> count 0, folio in swap cache, unmapped

The check runs before the zap, so it sees count 1. The zap comes
while A holds the lock for the read.

Checking after the read instead is what I tried first. It works for
the synchronous path and not for the bdev one:

zswap (sync)                           bdev (async)
swap_read_folio()                      swap_read_folio()
   zswap_load()                           submit_bio()
   .        <- zap, count 1 -> 0          return     <- zap, count 1 -> 0
   read done, lock still held           ...
   count is 0: free the slot, fine      swap_bio_read_end_io() (irq)
folio_unlock()                           folio_unlock()
                                          no ci->lock here, slot stays

The write side is the same picture with swap_writeout() and
end_swap_bio_write().


> I was kinda hoping this kind of interleaving would be rare :(


It is rare per event, but it adds up. On production it takes days to
leave a few GiB behind. The DONTNEED there comes from jemalloc, which
issues it on every free the application does, so it never stops. Our
reproducer is the extreme case, built only to verify the problem: it
leaves 480 GB in about an hour. I'll put the numbers in the commit
message.


> And yeah, I think if we want to free the slot after read, readahead
> swap IOs have to be submitted with BIO_COMPLETE_IN_TASK. zswap
> writeback's dropbehind relied on that...


Right, and that would still leave the writeback side. The worker covers 
both in one place.


>
>>
>>> That takes care of the readahead race. Seems safe to me
>>> (I assume if a folio is not mapped and swap count == 0, then it's a
>>> pure readahead artifact, but maybe I'm missing a case).
>>>
>>> The write direction already tries folio_free_swap() before submitting
>>> the writeback, FWIW.
>> Same shape: the zap arrives after that check, while the folio is under
>> writeback. On our box with a swap partition that is where most of the
>> leftover came from.
>>
>> Every check on the holder's side runs while the count is still 1. Only
>> the zap sees it go to 0, and it cannot wait. That is why the patch has
>> the zap hand it over instead (very nasty i know it :( ).
> Yeah I got no smarter ideas :( sweep_work it is...