On Sun, Nov 19, 2023 at 7:20 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote: > > Chris Li <chriscli@xxxxxxxxxx> writes: > > > On Thu, Nov 16, 2023 at 12:19 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > >> > >> Not bypassing the swap slot cache, just make the callbacks to > >> invalidate the zswap entry, do memg uncharging, etc when the slot is > >> no longer used and is entering the swap slot cache (i.e. when > >> free_swap_slot() is called), instead of when draining the swap slot > >> cache (i.e. when swap_range_free() is called). For all parts of MM > >> outside of swap, the swap entry is freed when free_swap_slot() is > >> called. We don't free it immediately because of caching, but this > >> should be transparent to other parts of MM (e.g. zswap, memcg, etc). > > > > That will cancel the batching effect on the swap slot free, making the > > common case for swapping faults take longer to complete, righ? > > If I recall correctly, the uncharge is the expensive part of the swap > > slot free operation. > > I just want to figure out what we are trading off against. This is not > > one side wins all situations. > > Per my understanding, we don't batch memcg uncharging in > swap_entry_free() now. Although it's possible and may improve > performance. Yes. It actually causes a long tail in swapin fault latency as Chris discovered in our prod. I am wondering if doing the memcg uncharging outside the slots cache will actually amortize the cost instead. Regardless of memcg charging, which is more complicated, I think we should at least move the call to zswap_invalidate() before the slots cache. I would prefer that we move everything non-swapfile specific outside the slots cache layer (zswap_invalidate(), arch_swap_invalidate_page(), clear_shadow_from_swap_cache(), mem_cgroup_uncharge_swap(), ..). However, if some of those are controversial, we can move some of them for now. When draining free swap slots from the cache, swap_range_free() is called with nr_entries == 1 anyway, so I can't see how any batching is going on. If anything it should help amortize the cost. > > -- > Best Regards, > Huang, Ying