Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
MIME-Version: 1.0
References: <20231011051117.2289518-1-hezhongkun.hzk@bytedance.com> <CAJD7tka+35CKYDLHus+LnN1yi_J=t_+kYXrZYh3asiWR41GKww@mail.gmail.com>
In-Reply-To: <CAJD7tka+35CKYDLHus+LnN1yi_J=t_+kYXrZYh3asiWR41GKww@mail.gmail.com>
From:   =?UTF-8?B?6LS65Lit5Z2k?= <hezhongkun.hzk@bytedance.com>
Date:   Fri, 13 Oct 2023 22:02:14 +0800
Message-ID: <CACSyD1NdYL4whDURJOC7_gqhpuqjbqsOkUacxS78iZm1pZCW9A@mail.gmail.com>
Subject: Re: [External] Re: [RFC PATCH] zswap: add writeback_time_threshold
 interface to shrink zswap pool
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     akpm@linux-foundation.org, hannes@cmpxchg.org, nphamcs@gmail.com,
        sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Thanks for your reply.
> I prefer if this can be done through memory.reclaim when the zswap
> shrinker is in place, as others have suggested. I understand that this
> provides more control by specifying the time at which to start writing
> pages out, which is similar to zram writeback AFAICT, but it is also
> difficult to determine the right value to write here.
>
> I am also not sure how you decide that it is better to writeback cold
> pages in zswap or compress cold pages in the LRUs. The pages in zswap
> are obviously colder, but accessing them after they are written back
> is much more expensive, to the point that it could be better to
> compress more cold memory from the LRUs. This is obviously not
> straightforward and requires a fair amount of tuning to do more good
> than harm.

I do agree.  For some applications, a common value will work,
such as 600s. Besides, this patch provides a more flexible way
to offload compress pages.

>
> That being said, if we decide to move forward with this I have a
> couple of comments:
>
> - I think you should check out how zram implements idle writeback and
> try to make things consistent. Zswap and zram don't really see eye to
> eye, but some consistency would be nice. If you looked at zram's
> implementation you would realize that you also need to update the
> access time when a page is read (unless the load is exclusive).

Thanks for your suggestion=EF=BC=8Ci will fix it and check it again.

>
> - This should be behind a config option. Every word that we add to
> struct zswap_entry reduces the zswap savings by roughly 0.2%. Maybe
> this doesn't sound like much but it adds up. Let's not opt everyone in
> unless they ask for it.
>

Good idea=EF=BC=8C Thanks.