Re: [External] Re: [RFC PATCH] zswap: add writeback_time_threshold interface to shrink zswap pool

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

Hi Nhat, thanks for your detailed reply.

> We're currently trying to solve this exact problem. Our approach is to
> add a shrinker that automatically shrinks the size of the zswap pool:
>
> https://lore.kernel.org/lkml/20230919171447.2712746-1-nphamcs@gmail.com/
>
> It is triggered on memory-pressure, and can perform reclaim in a
> workload-specific manner.
>
> I'm currently working on v3 of this patch series, but in the meantime,
> could you take a look and see if it will address your issues as well?
>
> Comments and suggestions are always welcome, of course :)
>

Thanks, I've seen both patches. But we hope to be able to reclaim memory
in advance, regardless of memory pressure, like memory.reclaim in memcg,
so we can offload memory in different tiers.

>
> My concern with this approach is that this value seems rather arbitrary.
> I imagine that it is workload- and memory access pattern- dependent,
> and will have to be tuned. Other than a couple of big users, no one
> will have the resources to do this.
>
> And since this is a one-off knob, there's another parameter users
> will have to decide - frequency, i.e how often should the userspace
> agent trigger this reclaim action. This is again very hard to determine
> a priori, and most likely has to be tuned as well.
>

I totally agree with you, this is the key point of this approach.It depends
on how we define cold pages, which are usually measured in time,
such as not being accessed for 600 seconds, etc. So the frequency
should be greater than 600 seconds.

> I think there might be some issues with just storing the store time here
> as well. IIUC, there might be cases where the zswap entry
> is accessed and brought into memory, but that entry (with the associated
> compressed memory) still hangs around. For e.g and more context,
> see this patch that enables exclusive loads:
>
> https://lore.kernel.org/lkml/20230607195143.1473802-1-yosryahmed@google.com/
>
> If that happens, this sto_time field does not tell the full story, right?
> For instance, if an object is stored a long time ago, but has been
> accessed since, it shouldn't be considered a cold object that should be
> a candidate for reclaim. But the old sto_time would indicate otherwise.
>

Thanks for your review，we should update the store time when it was loaded.
But it confused me, there are two copies of the same page in memory
(compressed and uncompressed) after faulting in a page from zswap if
'zswap_exclusive_loads_enabled' was disabled. I didn't notice any difference
when turning that option on or off because the frontswap_ops has been removed
and there is no frontswap_map anymore. Sorry, am I missing something?