> >>>> The second difference is the handling of lru entry, which is easy that we > >>>> just zswap_lru_del() in tree lock. > >>> > >>> Why do we need zswap_lru_del() at all? We should have already isolated > >>> the entry at that point IIUC. > >> > >> I was thinking how to handle the "zswap_lru_putback()" if not writeback, > >> in which case we can't use the entry actually since we haven't got reference > >> of it. So we can don't isolate at the entry, and only zswap_lru_del() when > >> we are going to writeback actually. > > > > Why not just call zswap_lru_putback() before we unlock the folio? > > When early return because __read_swap_cache_async() return NULL or !folio_was_allocated, > we don't have a locked folio yet. The entry maybe invalidated and freed concurrently. Oh, that path, right. If we don't isolate the entry straightaway, concurrent reclaimers will see the same entry, call __read_swap_cache_async(), find the folio already in the swapcache and stop shrinking. This is because usually this means we are racing with swapin and hitting the warmer part of the zswap LRU. I am not sure if this would matter in practice, maybe Nhat knows better. Perhaps we can rotate the entry in the LRU before calling __read_swap_cache_async() to minimize the chances of such a race? Or we can serialize the calls to __read_swap_cache_async() but this may be an overkill.
