Re: [External] Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios

[贺中坤 <hezhongkun.hzk@bytedance.com>]

>
> Ah my bad, I should have been clearer.
>
> I was looking at the zswap shrinker patch series (specifically the
> cgroup-aware LRU patch), which moves the counter update out of
> zswap_writeback_entry. If we apply that patch on top of that series, we will
> screw up the counter. Should be easily fixable anyway though.

Got it.

>
> Ah I think I understand the point of the patch a bit better now.
>
> Essentially, we're invalidating these entries, which does reclaim the
> memory used for these compressed objects, but there is no IO involved.
> Writeback-less shrinking, if you will.
>
> This will still screw up one of the heuristics I'm using for the zswap
> shrinker a bit, but that should be easily fixable with some plumbing.
> Same goes for the writeback counter - but depending on the order in
> which Andrew apply the patches, you might have to resolve the conflicts
> there :)

OK,  I will fix it.

>
> Other than this objection, I think this optimization makes sense to me:
>
> In the first case, we already freed the swap entry. Might as well also
> dropped the zswap entry.
>
> In the second case, we already have another copy in memory, so
> dropping the compressed copy to make space for warmer objects
> coming into zswap makes sense to me. Might be worth doing a
> micro-benchmark to verify this intuition, but I agree that it's more
> important to maintain the LRU ordering than any CPU saving from
> skipping re-compression.
>
> I would suggest that you should expand on this on the commit log
> to make clearer the motivation behind this optimization, if you were
> to re-submit this patch for some reason (for e.g to resolve the
> aforementioned conflicts with the zswap shrinker series).
>
> But otherwise, LGTM!
>
> Feel free to add the following tag:
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>

Thanks, there are still some commits from Yosry,
after that, I will send it again.