Hi Nhat, On Thu, Dec 7, 2023 at 5:03 PM Nhat Pham <nphamcs@xxxxxxxxx> wrote: > > On Thu, Dec 7, 2023 at 4:19 PM Chris Li <chrisl@xxxxxxxxxx> wrote: > > > > Hi Nhat, > > > > > > On Thu, Dec 7, 2023 at 11:24 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote: > > > > > > During our experiment with zswap, we sometimes observe swap IOs due to > > > occasional zswap store failures and writebacks-to-swap. These swapping > > > IOs prevent many users who cannot tolerate swapping from adopting zswap > > > to save memory and improve performance where possible. > > > > > > This patch adds the option to disable this behavior entirely: do not > > > writeback to backing swapping device when a zswap store attempt fail, > > > and do not write pages in the zswap pool back to the backing swap > > > device (both when the pool is full, and when the new zswap shrinker is > > > called). > > > > > > This new behavior can be opted-in/out on a per-cgroup basis via a new > > > cgroup file. By default, writebacks to swap device is enabled, which is > > > the previous behavior. Initially, writeback is enabled for the root > > > cgroup, and a newly created cgroup will inherit the current setting of > > > its parent. > > > > > > Note that this is subtly different from setting memory.swap.max to 0, as > > > it still allows for pages to be stored in the zswap pool (which itself > > > consumes swap space in its current form). > > > > > > This patch should be applied on top of the zswap shrinker series: > > > > > > https://lore.kernel.org/linux-mm/20231130194023.4102148-1-nphamcs@xxxxxxxxx/ > > > > > > as it also disables the zswap shrinker, a major source of zswap > > > writebacks. > > > > I am wondering about the status of "memory.swap.tiers" proof of concept patch? > > Are we still on board to have this two patch merge together somehow so > > we can have > > "memory.swap.tiers" == "all" and "memory.swap.tiers" == "zswap" cover the > > memory.zswap.writeback == 1 and memory.zswap.writeback == 0 case? > > > > Thanks > > > > Chris > > > > Hi Chris, > > I briefly summarized my recent discussion with Johannes here: > > https://lore.kernel.org/all/CAKEwX=NwGGRAtXoNPfq63YnNLBCF0ZDOdLVRsvzUmYhK4jxzHA@xxxxxxxxxxxxxx/ Sorry I am traveling in a different time zone so not able to get to that email sooner. That email is only sent out less than one day before the V6 patch right? > > TL;DR is we acknowledge the potential usefulness of swap.tiers > interface, but the use case is not quite there yet, so it does not I disagree about no use case. No use case for Meta != no usage case for the rest of the linux kernel community. That mindset really needs to shift to do Linux kernel development. Respect other's usage cases. It is not just Meta's Linux kernel. It is everybody's Linux kernel. I can give you three usage cases right now: 1) Google producting kernel uses SSD only swap, it is currently on pilot. This is not expressible by the memory.zswap.writeback. You can set the memory.zswap.max = 0 and memory.zswap.writeback = 1, then SSD backed swapfile. But the whole thing feels very clunky, especially what you really want is SSD only swap, you need to do all this zswap config dance. Google has an internal memory.swapfile feature implemented per cgroup swap file type by "zswap only", "real swap file only", "both", "none" (the exact keyword might be different). running in the production for almost 10 years. The need for more than zswap type of per cgroup control is really there. 2) As indicated by this discussion, Tencent has a usage case for SSD and hard disk swap as overflow. https://lore.kernel.org/linux-mm/20231119194740.94101-9-ryncsn@xxxxxxxxx/ +Kairui 3) Android has some fancy swap ideas led by those patches. https://lore.kernel.org/linux-mm/20230710221659.2473460-1-minchan@xxxxxxxxxx/ It got shot down due to removal of frontswap. But the usage case and product requirement is there. +Minchan > make too much sense to build up that heavy machinery now. Does my minimal memory.swap.tiers patch to support "zswap" and "all" sound heavy machinery to you? It is the same implementation under the hood. I am only trying to avoid introducing something that will be foreseeable obsolete. > zswap.writeback is a more urgent need, and does not prevent swap.tiers > if we do decide to implement it. I respect that urgent need, that is why I Ack on the V5 path, under the understanding that this zswap.writeback is not carved into stones. When a better interface comes alone, that interface can be obsolete. Frankly speaking I would much prefer not introducing the cgroup API which will be obsolete soon. If you think zswap.writeback is not removable when another better alternative is available, please voice it now. If you squash my minimal memory.swap.tiers patch, it will also address your urgent need for merging the "zswap.writeback", no? Chris