From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id C94BECDB474 for ; Tue, 17 Oct 2023 05:34:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F41088D00F2; Tue, 17 Oct 2023 01:34:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EEFEF8D00DE; Tue, 17 Oct 2023 01:34:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DB7C48D00F2; Tue, 17 Oct 2023 01:34:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id C76F48D00DE for ; Tue, 17 Oct 2023 01:34:06 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 96530A0E22 for ; Tue, 17 Oct 2023 05:34:06 +0000 (UTC) X-FDA: 81353837292.07.87248D0 Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) by imf02.hostedemail.com (Postfix) with ESMTP id 6C6748000D for ; Tue, 17 Oct 2023 05:34:04 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=t+Aryqze; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697520844; a=rsa-sha256; cv=none; b=3vLvd+CDJF2V4eZGpAhsNzYZLwqjM2GZITr+iQ7mSw07fbhFh6P467NJAzvXd/9LekxwcC BfzXpHqO8934Nhi7n2Uq8T+byfaBAXeR5uLtxwB6T/cu/0UywSm5kLDYm2NtYUrQ5PwRRl dkuutM/xqphaumQH4BktATVcFAju7CY= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=t+Aryqze; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697520844; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Lz/4Nv/Pi/BmjZt/Tm3iO+wXoh8fdTpmJ29M1nJbe88=; b=bL035QD4J5nbzeFsFbiaVkbc42dZZYoBM4Z8CTEF5nsRl/AWXYWUphZO+b2c/YQryc54fy HQLaksK5MfPqQZ74sfgtQtTxj/zO/LngK1N++8qSqW50tH3odhF6ptNNVwn0opg5DyGOkb GTFnY8EBa/syKqKBrFSderm2sv3G4ow= Received: by mail-ej1-f48.google.com with SMTP id a640c23a62f3a-9ba081173a3so867082966b.1 for ; Mon, 16 Oct 2023 22:34:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697520843; x=1698125643; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Lz/4Nv/Pi/BmjZt/Tm3iO+wXoh8fdTpmJ29M1nJbe88=; b=t+Aryqzeep8A1hRxaZcAEyWqWvOtsF7YWOsSwFSRthDER25KA1lxlHaK0cBKZn6j/r //92nd2WZ38TGp8s7ls0YEjjVUhAHtC/dbal7aFycl2sH3dmwR/rqBL5/yPdOxlnIKuB nJpfrPmj6rMYwKKQ0AGGCtqB9f6j9EbcNeYQof6rCgIcsX6HVi5lvsT/SVQhaFu7kwQh D4Nwnqu7+Vaf5A18Hcf/5m58N4toH2XrjSBlGQRa9e16E+P2RAaAZfRKpD+6qhKQCUjF TxwsDuaNDHyNUkCa4ucV73lPs/kD2Dx4qdOBm4sf7P/uaStv0jK9wVYzeQyYKcW1Zvr3 NlCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697520843; x=1698125643; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Lz/4Nv/Pi/BmjZt/Tm3iO+wXoh8fdTpmJ29M1nJbe88=; b=ZfuidAUPOtOk/8a/+4OQ+mljPRR+NTq1Hg2Io01T/lunEWGY156FmBnBCONZtISiyX pMmpSO5zMVMLdCt1AWUuucaLbIrsDRlT0/7pgpWbFTIE9ObJAxCl0GP2bGEKWtNRrGNS DXiDWL6ceIYqGGPninMTvtjGNYMFSje7XriDATyvHDivifsnfFvtIXOL7i+LU50qTuiI XRv4OJtMov7ydU/ojUYf/N1JNsd7ljTaeO5/LJqdJgqvDGWXXDz12NaD3ZgZvsnLc5tz dToUP27LlX9o3oCFXdOnqEbKZd+wMJcOahJLrArQLguta6Yr+rHQoqcM2v1secY23gp+ T9dA== X-Gm-Message-State: AOJu0YyPJL/oVZVCN1SX/SNBcASa6A8s95ccTpbx0mK5ytpWcAydVy53 k/MUpyRAYp2Og5TW4hp8bMmJLXs75+kTr8wKUOLyTg== X-Google-Smtp-Source: AGHT+IE8YGCi6YMLuJ7SPZiyiFZVWgOC7cRRv8mMpc2DaDSjzrVF6ikAXRTRx9PGjn0DcV9w2JnDgYee0wZSLg8fscQ= X-Received: by 2002:a17:907:749:b0:9a6:1446:852c with SMTP id xc9-20020a170907074900b009a61446852cmr813495ejb.27.1697520842460; Mon, 16 Oct 2023 22:34:02 -0700 (PDT) MIME-Version: 1.0 References: <20231017003519.1426574-1-nphamcs@gmail.com> <20231017044745.GC1042487@cmpxchg.org> In-Reply-To: <20231017044745.GC1042487@cmpxchg.org> From: Yosry Ahmed Date: Mon, 16 Oct 2023 22:33:23 -0700 Message-ID: Subject: Re: [PATCH 0/2] minimize swapping on zswap store failure To: Johannes Weiner Cc: Nhat Pham , akpm@linux-foundation.org, cerasuolodomenico@gmail.com, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, hughd@google.com, corbet@lwn.net, konrad.wilk@oracle.com, senozhatsky@chromium.org, rppt@kernel.org, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, david@ixit.cz, Wei Xu , Chris Li , Greg Thelen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 6C6748000D X-Stat-Signature: s5yotfh5qm7qbohrkuyxaph3ysun7x17 X-Rspam-User: X-HE-Tag: 1697520844-182840 X-HE-Meta: U2FsdGVkX1+nBli5zXzFF0JGsoyU5g2EbGfqdxcDkuOyXluKtK1rhySi2HmSzb5aniiB1Uh8+45kKZvTzbB2RyVIuqva6ELAVw5AclGbw/XHPdDyZk3YVqjEc473S3VPgV4Q9GoYNKSk41zKGg0/ysxBmJdY4mm9dKdwo1bSKmt8PybcEo5/JwI2nyXrExaaN1Km+r2Qauvhm/fpTXwkj3EktU7TK4zvWg+VhsXZn5RFRIT7vhqQ6g/XWjDevlXU/3ofhY6n02zfdxwPoEC7DDrLPDHf/o31ff4WjFSpcQaSZNYAo9yzu2Fo5iy09EqLdK3ElDhi/0H/TFN1zjVtO5lPDp6Erwpxqu3zNO9M4vx8jPnfEBg5xVkwCffZsmox2Wb6FuwsT3UUELNLJFD/Mi3nQ4nFxsBOlysFPufzrsrWBk2pHJbbtpEZj8JSl0tRq42Bph0H3C6GDIiFAkpCzN4NBpEDBEwLMaUKSc+5nBEbrjhPltcWpkPVylOsqWfnWLGjC5w6rpcPK2hf4DvdLB9ouBmnVsj+f8m5MkdHVRrcf4sAEyE4Q+ReMiQiej4ZPv6CKxffdn+uInQg7iHIdGOBC9Jj5AinK1s7VhxBwrW58UAYx0qOMwyC+tvlKBvnpG1gAAurAlOvW1NTinhgfasMrtPHa7AxFmxHab+BFKbRPBDGkpRxIgNh7W+IxFAHCdFYzLBBomIJxvqNaLI5puMupsGTG8M3rXrshNs1AZdLlcKiK8IDvlRe+QJ6vIJ0Fu7x2v5j0pdW9SnWZKUR3HyPEgE+GXsxGBGKYer3Jp5AM3zNC2o+gGTQP1LqoBHYGGqzfkDarObrkxL3Ka4vqsGRTrtkyC53F05ZBc2ENNv/RMrLiiSrHl1+U9Jh3KASgv9SxWV2SsJB20Wscl9atIcYQFXxxDuQiqMLG4SZgw4X0MBvShEK/RGabbx04sRjDzDhu9WpQNzEwfHutn7 riOCP3zo L14ZrF9noEz002EArtkjT/YWKoAOY4vOUFj9hMA6FPDbVEEb1JTwUzCkg6xW1L0RoBrvaxqSJUR2qy8cU1iqerLCXCJEVnSMJLPBk4lTQmX9+BcQa29BL9sK1ZaPx6X+sNlC16G4BV9kR/T2xXnDnKZAYBtuc56MCCLDWgEh/oqioAqoECfLzmH44LwT+TOQLcJakTvEkLS16lnfxPTvl4k5G1GhTs/B+hhwDIiVeWmqXhkT+Njp0kJGo4uvtVCGum3WetMmO4YyXbvSi19duTY6sXgozsKNkXPFKgFGDV4umv+7RS+fHShIVTA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Oct 16, 2023 at 9:47=E2=80=AFPM Johannes Weiner wrote: > > On Mon, Oct 16, 2023 at 05:57:31PM -0700, Yosry Ahmed wrote: > > On Mon, Oct 16, 2023 at 5:35=E2=80=AFPM Nhat Pham w= rote: > > > > > > Currently, when a zswap store attempt fails, the page is immediately > > > swapped out. This could happen for a variety of reasons. For instance= , > > > the compression algorithm could fail (such as when the data is not > > > compressible), or the backend allocator might not be able to find a > > > suitable slot for the compressed page. If these pages are needed > > > later on, users will incur IOs from swapins. > > > > > > This issue prevents the adoption of zswap for potential users who > > > cannot tolerate the latency associated with swapping. In many cases, > > > these IOs are avoidable if we just keep in memory the pages that zswa= p > > > fail to store. > > > > > > This patch series add two new features for zswap that will alleviate > > > the risk of swapping: > > > > > > a) When a store attempt fail, keep the page untouched in memory > > > instead of swapping it out. > > > > What about writeback when the zswap limit is hit? I understand the > > problem, but I am wondering if this is the correct way of fixing it. > > We really need to make zswap work without a backing swapfile, which I > > think is the correct way to fix all these problems. I was working on > > that, but unfortunately I had to pivot to something else before I had > > something that was working. > > > > At Google, we have "ghost" swapfiles that we use just to use zswap > > without a swapfile. They are sparse files, and we have internal kernel > > patches to flag them and never try to actually write to them. > > > > I am not sure how many bandaids we can afford before doing the right > > thing. I understand it's a much larger surgery, perhaps there is a way > > to get a short-term fix that is also a step towards the final state we > > want to reach instead? > > I agree it should also stop writeback due to the limit. > > Note that a knob like this is still useful even when zswap space is > decoupled from disk swap slots. We still are using disk swap broadly > in the fleet as well, so a static ghost file setup wouldn't be a good > solution for us. Swapoff with common swapfile sizes is often not an > option during runtime, due to how slow it is, and the destabilizing > effect it can have on the system unless that's basically completely > idle. As such, we expect to continue deploying swap files for physical > use, and switch the zswap-is-terminal knob depending on the workload. > > The other aspect to this is that workloads that do not want the > swapout/swapin overhead for themselves are usually co-located with > system management software, and/or can share the host with less > latency sensitive workloads, that should continue to use disk swap. > > Due to the latter case, I wonder if a global knob is actually > enough. More likely we'd need per-cgroup control over this. In conjunction with ghost swapfiles, we have a knob to determine the type of swapfile to use for each cgroup (normal, ghost, either, or none). This achieves what you are describing, allowing different workloads on the same machine to use zswap only or disk swap, although in practice we only use zswap now. I am not saying that's necessarily the correct way of doing it. Having a zswap-is-terminal knob per-cgroup is another way to achieve this. I will loop in folks maintaining this code internally to see what they think. > > [ It's at this point where the historic coupling of zswap to disk > space is especially unfortunate. Because of it, zswap usage counts > toward the memory.swap allowance. If these were separate, we could > have easily set memory.zswap.max=3Dmax, memory.swap.max=3D0 to achieve > the desired effect. > > Alas, that ship has sailed. Even after a decoupling down the line it > would be difficult to change established memory.swap semantics. ] Fully agree here. This is unfortunate. > > So I obviously agree that we still need to invest in decoupling zswap > space from physical disk slots. It's insanely wasteful, especially > with larger memory capacities. But while it would be a fantastic > optimization, I don't see how it would be an automatic solution to the > problem that inspired this proposal. Well, in my head, I imagine such a world where we have multiple separate swapping backends with cgroup knob(s) that control what backends are allowed for each cgroup. A zswap-is-terminal knob is hacky-ish way of doing that where the backends are only zswap and disk swap. > > We still need some way to reasonably express desired workload policy > in a setup that supports multiple, simultaneous modes of operation. > > > > b) If the store attempt fails at the compression step, allow the page > > > to be stored in its uncompressed form in the zswap pool. This maintai= ns > > > the LRU ordering of pages, which will be helpful for accurate > > > memory reclaim (zswap writeback in particular). > > > > This is dangerous. Johannes and I discussed this before. This means > > that reclaim can end up allocating more memory instead of freeing. > > Allocations made in the reclaim path are made under the assumption > > that we will eventually free memory. In this case, we won't. In the > > worst case scenario, reclaim can leave the system/memcg in a worse > > state than before it started. > > Yeah, this is a concern. It's not such a big deal if it's only a few > pages, and we're still shrinking the footprint on aggregate. But it's > conceivable this can happen systematically with some datasets, in > which case reclaim will drive up the memory consumption and cause > OOMs, or potentially deplete the reserves with PF_MEMALLOC and cause > memory deadlocks. > > This isn't something we can reasonably accept as worst-case behavior. Right. > > > Perhaps there is a way we can do this without allocating a zswap entry? > > > > I thought before about having a special list_head that allows us to > > use the lower bits of the pointers as markers, similar to the xarray. > > The markers can be used to place different objects on the same list. > > We can have a list that is a mixture of struct page and struct > > zswap_entry. I never pursued this idea, and I am sure someone will > > scream at me for suggesting it. Maybe there is a less convoluted way > > to keep the LRU ordering intact without allocating memory on the > > reclaim path. > > That should work. Once zswap has exclusive control over the page, it > is free to muck with its lru linkage. A lower bit tag on the next or > prev pointer should suffice to distinguish between struct page and > struct zswap_entry when pulling stuff from the list. Right. We handle incompressible memory internally in a different way, we put them back on the unevictable list with an incompressible page flag. This achieves a similar effect. A missing point here is that those pages, if dirtied, may be compressible again. When we have them on the LRUs, we rely on periodic scanning (similar to the MGLRU-based periodic scanning we proposed before) to check the dirty bit and make those pages evictable again. If we leave them on the zswap LRU, we will incur a fault instead to pull them back to the LRUs. For anon pages, that's probably fine, as in both cases by the time we reach zswap the page has been unmapped, and accessing it again incurs a fault anyway (whether it's in zswap LRUs or in the reclaim LRUs). For shmem though, we put the incompressible pages back in the page cache, preventing a page fault on the next access. This is a drawback of the zswap LRU approach AFAICT. Not sure how much it matters in practice. > > We'd also have to teach vmscan.c to hand off the page. It currently > expects that it either frees the page back to the allocator, or puts > it back on the LRU. We'd need a compromise where it continues to tear > down the page and remove the mapping, but then leaves it to zswap. Right. > > Neither of those sound impossible. But since it's a bigger > complication than this proposal, it probably needs a new cost/benefit > analysis, with potentially more data on the problem of LRU inversions. Makes sense. > > Thanks for your insightful feedback, Yosry.