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 00C80C3DA6E for ; Wed, 20 Dec 2023 08:59:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7D4DE6B0080; Wed, 20 Dec 2023 03:59:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 786236B0081; Wed, 20 Dec 2023 03:59:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 64D856B0082; Wed, 20 Dec 2023 03:59:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 544676B0080 for ; Wed, 20 Dec 2023 03:59:56 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 23C59160924 for ; Wed, 20 Dec 2023 08:59:56 +0000 (UTC) X-FDA: 81586599192.08.833B143 Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) by imf09.hostedemail.com (Postfix) with ESMTP id 42BAA14001B for ; Wed, 20 Dec 2023 08:59:54 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dUzKGRen; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1703062794; 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=MLR+Picg0YGbnnq1391FrVjY92aX0Fgy3Wr8+PVd9bw=; b=kEpimHZdbf4KzXLsVYWG3tCo2HsOXB+M4GAL5Tjk7fVMguwZ00lv8J23V1k5uMB6qbqQEF wibqtVkn3/MNi0L0xlRIRIDUvOJ8daskKa6T+fpiOXQxgYJHB0ty3hPWM6OEp5U5VVGw/W LW0Q3fIcUc63bLacj6jYzt3o6HnrWCo= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dUzKGRen; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703062794; a=rsa-sha256; cv=none; b=BpJzEsNxQbWHEUZNWAd63niGC4XqJvxfsp60YtTRTQzdNV4ZpqNbx0w09vw1cEJeM74RU/ 3sw/ZK6FgA+P/bTuMtMyYt97uP5+etuajs7OxUOKyEvx+c3Isv5i3KL84WFMdCArFRTW90 p+hr72Nb7T9irSwYjDn7LncvcJnN4hw= Received: by mail-ej1-f48.google.com with SMTP id a640c23a62f3a-a2358a75b69so95970066b.1 for ; Wed, 20 Dec 2023 00:59:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1703062793; x=1703667593; 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=MLR+Picg0YGbnnq1391FrVjY92aX0Fgy3Wr8+PVd9bw=; b=dUzKGReni3QI+IjSOUioNhBc66J2wBisUws2wBQtBAr/uH3ehE/1bcUC+57y1F5Cf6 blphhmb2Ui0kfI1Ikov5MX4mG+Dnfri2aXGfxwbce7P+zLgzWpUC4Pfj24f22qGiYcOM 1GdadUIsSYp+VAB0GdFOLpxV5CLIREnEWoONZFzv40WChskE9Uu97kLbBthtgfV7Y6tB jm6qCzxHqgs6v9a8Kl3n5i7EX5Xf/m1M20e1ffN/tPzBOf/k6XwLROUn02iIqlwQ7RZM 7+IxtXZXTttWzxmyaya17ID0J1OpoX8c0WfebWuuPXxA4xz3etFIDozqLVx+RHqoMNID bpIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703062793; x=1703667593; 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=MLR+Picg0YGbnnq1391FrVjY92aX0Fgy3Wr8+PVd9bw=; b=wHt8/MB1FfEJJQzAhzagYCJeqMeVOMmZcdBsLhbB0K0DDYXkslukiH0dGs8PkAVQbf 2tLL1WfqlZLec4AvDmaM7F+SRRfDHtOY0oAGPIsyGO2wQq3UWJlag48k+K1cSxXyvCOi FQDqtU3mHc/q93F25O5DbgkKFH/5oNJwvrMnF1UMOkr9ZIHKjUGJ64oxZQpkfPDb+ewi j9FPw+59CN8gE4Qb7393dmgQWRBKEdT+AFnbWhTCuEHFZub20WIWruHtvgFBIwayJN7k VNa6HeN3+OBofVu6mOIuIuEWUd1WhK6X6wafDjbmOuYM+0bOEBbO/d/QPWZ2rIfZ7s6Q OW8A== X-Gm-Message-State: AOJu0YwONN+VfabTVdNoQ8z+zB4Y8uoyrkmUFUR5BASubXN8pUU8SsqI eEEAhGd6Iip6733GTJx5oVlYcyu8NeHJVZeh+bWahtN12m0P X-Google-Smtp-Source: AGHT+IHYRWeO+3YIShZ1cYf6+2ND6yE4M9EA0qe8haJZaUeBo81F4ZbCNe42GvjFLhqnrfwVF2zP0/Tv+/P2iuKmpls= X-Received: by 2002:a17:906:3405:b0:a23:74a8:ade with SMTP id c5-20020a170906340500b00a2374a80ademr2246512ejb.25.1703062792537; Wed, 20 Dec 2023 00:59:52 -0800 (PST) MIME-Version: 1.0 References: <20231207192406.3809579-1-nphamcs@gmail.com> <20231218144431.GB19167@cmpxchg.org> <20231220051523.GB23822@cmpxchg.org> In-Reply-To: <20231220051523.GB23822@cmpxchg.org> From: Yosry Ahmed Date: Wed, 20 Dec 2023 00:59:15 -0800 Message-ID: Subject: Re: [PATCH v6] zswap: memcontrol: implement zswap writeback disabling To: Johannes Weiner Cc: Nhat Pham , akpm@linux-foundation.org, tj@kernel.org, lizefan.x@bytedance.com, cerasuolodomenico@gmail.com, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, mhocko@kernel.org, roman.gushchin@linux.dev, shakeelb@google.com, muchun.song@linux.dev, 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, linux-doc@vger.kernel.org, david@ixit.cz, chrisl@kernel.org, Wei Xu , Yu Zhao Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 42BAA14001B X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: nzhwhmwipgpue91f68k65sjux8buyyza X-HE-Tag: 1703062794-211121 X-HE-Meta: U2FsdGVkX18gOeM2Mt5iEhdk3QuUHF9z1XYG+YcQzk+YRPb6JbkTkK4ag9jbgH9jLscr2sRQYYGP9PCAQtQpFlei7086fi18tgdEGG9bdFVv2CLX9u+OWb4DMCUXkphrjABJYJQOz3c95jJL7E7SIuUrmXTEjM14dtdfg5YrNCB2W548pRgYmHG7AlwZKNphhS6WUTbzwuERGxEWUrUC380g7J7TuqYFwp/C8LZACXCVPhLAmC49QalbQwvyDlj1OfOJ2Q49tI7GmZMA5sPRM/bHb8N8IaNJ1ew11TcL+LxF4dLH5366GfniMZfeXw6kW3v8cfYusWJEnLZnsQmuYphSdGISRcNdRXuX8kH4lQ3FFEJma6j0t8EzRFjD9SATux56gdlaFDJDGKlnBqXoLvobyc86E3Bkr4XFBf/jrPQGvGSbdxbdkY6Aao4sKcvJAA+V8NHrfUwO8R+EzfG4M7FbqL39FIzf91ZMO8xbK61CvKZ+0WScXpVjgE2BLKIqV0kJdFNNZj/CmgsdAh+R44YXfJPTTonvEm+MHeUfTFkETajPrdcnPatVoteTHDD35pVP1kgORyFAQmm+BZIWZ7USfKnFG5iv+DpHE2Fw0raBNQADTwW3QlrfjOD7jX6oCtYDPwUF5GyaFRMqB9clEisHZzRYk0qefeAvM1oNCBd8EldpLps0fJ/IxxuPN5/XtEeExzTagxQ6bF2i8WKvTOKp2AdLuBXOHor4mjInb0WgC7FaeVz9mxwmt2tNC34hfr/XeweOn/w4UMM0Ql4Cy4l/2d8lX81ggHsCxjayFFcJdGGefGaswaDCAtvA4eK84JFOB8Wqmq1+YO3mUFXloMzI+m3dz6Kwpp7QgKyrIECnxP50FVDp+QIyzLIhxsgQWWrfCJHO9gxCcLU82hgaN0OIIDRuwKxqUpdfEqmDDfAhGY1oBynXShwbpbDDbuxPfe8MjwnC4J9JktodY4Y 0dqXdq8R Qm12j49Ggy1xhR655Fgi5TDT/6iCNvYemWbjauqUKi+5tsce7DeGHArCJD89TpwvLZbrgzTcEetnZo0B6wpT1wIKgbX/DT+CSJ/9FCxttC5T5OntnT81d/2GlKi9kantoi1umyaXzCbb3I/jNEzKbv3ZydcYIawFOBjN4xmFTLs9KEDN5hfYRgo145W3g2zlwIPdFzEAH3DBoNaKF1gVN7D6+TCuZYd/mcs01mhR1rrYcQCEUHMxgcwuiPiDGimaYUjQWHppU2ZdxSzc7uRr0+oPFlK9ZO07M75eFkwhuoxbSDP1GbWr/MHuwgsmI+vJK3mB2eAXL63FRR/tTzvLXcIQWQgYbyoIreHZTmJZsF7pHVlg8r1kiMa+Ukj0CK6LtM9ay/hh4snBVxJiBtK3zljj0mw== 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: List-Subscribe: List-Unsubscribe: On Tue, Dec 19, 2023 at 9:15=E2=80=AFPM Johannes Weiner wrote: > > On Mon, Dec 18, 2023 at 01:52:23PM -0800, Yosry Ahmed wrote: > > > > Taking a step back from all the memory.swap.tiers vs. > > > > memory.zswap.writeback discussions, I think there may be a more > > > > fundamental problem here. If the zswap store failure is recurrent, > > > > pages can keep going back to the LRUs and then sent back to zswap > > > > eventually, only to be rejected again. For example, this can if zsw= ap > > > > is above the acceptance threshold, but could be even worse if it's = the > > > > allocator rejecting the page due to not compressing well enough. In > > > > the latter case, the page can keep going back and forth between zsw= ap > > > > and LRUs indefinitely. > > > > > > > > You probably did not run into this as you're using zsmalloc, but it > > > > can happen with zbud AFAICT. Even with zsmalloc, a less problematic > > > > version can happen if zswap is above its acceptance threshold. > > > > > > > > This can cause thrashing and ineffective reclaim. We have an intern= al > > > > implementation where we mark incompressible pages and put them on t= he > > > > unevictable LRU when we don't have a backing swapfile (i.e. ghost > > > > swapfiles), and something similar may work if writeback is disabled= . > > > > We need to scan such incompressible pages periodically though to > > > > remove them from the unevictable LRU if they have been dirited. > > > > > > I'm not sure this is an actual problem. > > > > > > When pages get rejected, they rotate to the furthest point from the > > > reclaimer - the head of the active list. We only get to them again > > > after we scanned everything else. > > > > > > If all that's left on the LRU is unzswappable, then you'd assume that > > > remainder isn't very large, and thus not a significant part of overal= l > > > scan work. Because if it is, then there is a serious problem with the > > > zswap configuration. > > > > > > There might be possible optimizations to determine how permanent a > > > rejection is, but I'm not sure the effort is called for just > > > yet. Rejections are already failure cases that screw up the LRU > > > ordering, and healthy setups shouldn't have a lot of those. I don't > > > think this patch adds any sort of new complications to this picture. > > > > We have workloads where a significant amount (maybe 20%? 30% not sure > > tbh) of the memory is incompressible. Zswap is still a very viable > > option for those workloads once those pages are taken out of the > > picture. If those pages remain on the LRUs, they will introduce a > > regression in reclaim efficiency. > > > > With the upstream code today, those pages go directly to the backing > > store, which isn't ideal in terms of LRU ordering, but this patch > > makes them stay on the LRUs, which can be harmful. I don't think we > > can just assume it is okay. Whether we make those pages unevictable or > > store them uncompressed in zswap, I think taking them out of the LRUs > > (until they are redirtied), is the right thing to do. > > This is how it works with zram as well, though, and it has plenty of > happy users. I am not sure I understand. Zram does not reject pages that do not compress well, right? IIUC it acts as a block device so it cannot reject pages. I feel like I am missing something. > The fact that there are antagonistic workloads doesn't > mean the feature isn't useful. This flag is optional and not enabled > by default, so nobody is forced to use it where it hurts. > > I'm not saying it's not worth optimizing those cases, but it doesn't > look like a requirement in order to be useful to a variety of loads. But we don't even understand the impact on those workloads today to properly document it. What happens with a workload using zbud for example and has quite a bit of memory that gets rejected? Is the feature usable for such a setup or not? There has also been discussions upstream about introducing a compression threshold for zswap in general (see below), this seems to be going in an opposite direction. If we already want to support taking pages away from the LRUs when rejected by zswap (e.g. Nhat's proposal earlier), doesn't it make sense to do that first so that this patch can be useful for all workloads? > > > Adding Wei and Yu for more data about incompressible memory in our > > fleet. Keep in mind that we have internal patches to cap the > > compression ratio (i.e. reject pages where the compressed size + > > metadata is not worth it, or where zsmalloc will store it in a full > > page anyway). But the same thing can happen upstream with zbud. > > I hate to bring this up, but there has been a bit of a disturbing > trend in the zswap discussions recently. > > Please do not argue with private patches. Their behavior, the usecases > they enable, and their dependencies are entirely irrelevant to patches > submitted in this forum. They do not need to cater to them or consider > the consequences for them. The only thing that matters is the upstream > codebase and the usecases enabled by it. Sorry if my intention wasn't clear. I am not arguing that this patch affects our internal patches in any way. All I am saying is that we do something similar internally, and we would like to move to an upstream solution if possible -- so naturally we want the upstream solution to work for us as well. Besides, this can happen in the upstream codebase with zbud as I mentioned earlier, and there has been discussions upstream about introducing such a compression threshold as well (e.g. [1]). So it is not something unique to Google. If this is where we think we are headed upstream (and is already the case with zbud), I think it's not unreasonable to bring it up. [1]https://lore.kernel.org/lkml/7a0e3229-be63-4a24-a3fe-7e3ff517de10@byteda= nce.com/