On Wed, Nov 15, 2023 at 7:33 PM 贺中坤 <hezhongkun.hzk@xxxxxxxxxxxxx> wrote: > > On Thu, Nov 16, 2023 at 4:13 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > > > I think we may need to try to lock the folio. Otherwise we may race > > with reclaim reading the dirty bit before we set it. > > > > Taking a step back, this seems like we are going behind exclusive > > loads. We "should" keep the page in zswap as exclusive loads are > > disabled and the page is not yet invalidated from zswap (the swap > > entry is still in use). What you are trying to do here is sneakily > > drop the page from zswap as if we wrote it back, but we didn't. > > If we want to reclaim the cached zswap_entry, Writing back might > be the easy way. > > > We just know that it was already loaded from zswap. We are essentially > > making the previous load exclusive retroactively. > > > > Is there a reason why exclusive loads cannot be enabled to achieve the > > same result in the (arguably) correct way? > > > > zswap_exclusive_loads is not enabled by default, so the shrink_worker > may fail if there are many cached zswap_entries on the zswap_pool->lru. It can be enabled at runtime, or enabled by default by using CONFIG_ZSWAP_EXCLUSIVE_LOADS_DEFAULT_ON. > > > Is it possible to make zswap_exclusive_loads the only way in zswap_load? > It only makes sense when the page is read and no longer dirty. > If the page is read frequently, it should stay in cache rather than zswap. > The benefit of doing this is very small, two copies of the same page > in memory. The reason I added it behind runtime and config knobs is to preserve the existing behavior in case someone depends on it. At Google, we have been using exclusive loads for a long time. If other users of zswap agree to make this the default behavior or make it the only way to do zswap loads I don't have a problem with it. > > > Thanks.
