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 0D533C4332F for ; Thu, 2 Nov 2023 20:54:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8F8A98D00A8; Thu, 2 Nov 2023 16:54:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 889E38D000F; Thu, 2 Nov 2023 16:54:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6F9E18D00A8; Thu, 2 Nov 2023 16:54:55 -0400 (EDT) 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 595218D000F for ; Thu, 2 Nov 2023 16:54:55 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2E62E40D79 for ; Thu, 2 Nov 2023 20:54:55 +0000 (UTC) X-FDA: 81414218550.30.A35762C Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf03.hostedemail.com (Postfix) with ESMTP id 51AA720008 for ; Thu, 2 Nov 2023 20:54:53 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=e0AkPODC; spf=pass (imf03.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 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=1698958493; 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=c63Npi10wJWosRhm9gjhF9hBJGybG/sFGWhVUImu6vc=; b=qkG8jbrU7p9Y25DkjVIljmkMoQFsqReqZPQ4ck8StlyYUtsM9SoQHDAUFK7oCS06K53SqO KVeF7Ig8GijUDt6IKG7QQ1SNVrWQaJKgnKyZ4cD2UmlS0KAXPNZyhqfJrsTGaW1m/KN0l1 jr4gbosYkiwXlUbIwqvYtkioDFFT2vc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698958493; a=rsa-sha256; cv=none; b=Ts3KpUkKC81xcsXOmNKT63kJVBXzP2zb8De7Q6RREY+wc770oAkbb0AlwWu5c46BlITF6I ajiKcgI128Olco9SesqDIOxS+FGBG1wOk0ZC/kUgP4la/DjhAtWnISplXdkmi0i2PQnjvF dEf8wwWzCGtu9cnEpapw4VEEwqlADvo= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=e0AkPODC; spf=pass (imf03.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-99de884ad25so223631366b.3 for ; Thu, 02 Nov 2023 13:54:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698958492; x=1699563292; 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=c63Npi10wJWosRhm9gjhF9hBJGybG/sFGWhVUImu6vc=; b=e0AkPODC/GnFEXJxlCvd2MLLYR0Drz+5XCMkxDy/aXNeH9gIW+0vyFIozxX8/DoBjs 0rF9QPGWPpA4BaY1aRULzF6phUUGwcENjCVIMkAvhodzc4vlFdThaGA7wPNxcWD/ojec pZReQ83/1i6Y6MY2sjr95uDykjWDeeuAq9w/HYIUPYZB/AkqcmgX11LaT/kWmoaaOYY/ QXFIL41Y6/XoY+YXlPTSRZFL0BESM9hbJz4MpfJNtLM5g5SP7YFVoNIc1tHFtpXTzcJL KztTaYYYSB7o++wMdsfL8+ynjqLwDMeYaHGurIrYFSF/twR6ZlRQWuko7rjY28XsOkR0 Q85g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698958492; x=1699563292; 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=c63Npi10wJWosRhm9gjhF9hBJGybG/sFGWhVUImu6vc=; b=Z+fy+cxt8T4pU+j/iQuUrqXUXb4JRdZPQihm21lEW/9YxvHYd4sgkk51CuVvgNre1k E9DEqb2tTPfWeV+iLSRY/L+8BKnb8TuJTFQL7o7LOuTmgaKCGUVTdWA+jDqJNQ3DaOwx /N1lZoLYFy50fKxiRq6iPixfWlydB3lgCbbXdHBBhxhkOwI/P1H1vWkXGbm+Roj8LgbP xAgiAKkIy4gk9C1KSJwL8U8iwT7p8gPDIMGa9wq7jsyjoXGFjwTrbNjdkMbZvfq4MKj6 8HBJJTOLD2FkrKAOMtzQILOU/OjNk5RbbD4tW97rOg2CRI5UPCvbHrgeB0yubyIw3Qnn UYQA== X-Gm-Message-State: AOJu0YzOaCZmTPwSqbSkfSw2Hw8pi1s8lBwiEjx2FvMwmdxSc1Nif7m9 uYJmgxQe5QSiTb4zbvmU17Uu6hNl8px37fs4nHL24Q== X-Google-Smtp-Source: AGHT+IHLJLU+8CiChQllqKrDRluL6dH+cwOBHxVpM9opxi6ovqxvubfE++DRFh5SrqxT4kgbnkaxPOOFS6WfTLf12lE= X-Received: by 2002:a17:906:7309:b0:9c8:f128:2fdb with SMTP id di9-20020a170906730900b009c8f1282fdbmr5328519ejc.13.1698958491500; Thu, 02 Nov 2023 13:54:51 -0700 (PDT) MIME-Version: 1.0 References: <20231102200202.920461-1-nphamcs@gmail.com> <20231102205022.GA3265934@cmpxchg.org> In-Reply-To: <20231102205022.GA3265934@cmpxchg.org> From: Yosry Ahmed Date: Thu, 2 Nov 2023 13:54:12 -0700 Message-ID: Subject: Re: [RFC PATCH v2] 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 51AA720008 X-Rspam-User: X-Stat-Signature: pdqsjhszopd9dc6t7bw91xcx9pbfzwje X-Rspamd-Server: rspam03 X-HE-Tag: 1698958493-120205 X-HE-Meta: U2FsdGVkX1/hacOd0WzxpIsjmZKXg0X5HnSTur8WDXoVyAxXh+IbrZyv1XUFX/ztVCbxqG3pNaeWSvWGm0c0WDKFzEqDZAr0UDnVByP9xSOiiE1bFEUK14wQgjl7FoSGKUkvib6SzTPdjAA3NZZqKtEyQPgNoUlpxnqcgWQ6KsHXJ5Smclz44nYN7lphcMbM7qHhEYWohpLE8joPasr03q9p20FyFbR7qGRXWp0bcVvXh1jD52bFgE49QvsiRJJ9oDCu2cov+uaJJknr2G5rkCQuTVveMeomSFyhEy00HYesQgGz5SkQO8lx9bgxRXIbDe5YQDr4WbUKwMn7jrC2hm9uWoOeNP6T4hugL7Cmu3eAQvV3KGDeveMyR7aIkslyL5DHWweJ1Jn28LEyrxr7dfbdZKCvG0KOUsdNr0uLObTjv7n6klQJ3firk/jd2w6A2ce3QkQCPLOFaUJkSDnBhc2t9ulzle2DeIWcWVedf5n687G2e5BLR13qOymHcQbzptHIw6d0nmkwttHHM6ixUl3e7D7Uqs7rC9fnWneiVR06WqCAhparSYjClfbdhIBgkzgOKMrwMDF5sU0OH5JBDhmSc/MSJ/AZscBzz5twtBZlLROKRS9+0BPv5SkKYYJLph9yJa6oToctTWuq381BSdzmWZ/NT45RF/XsfDH7QUt2l4Newl+lwe0SWCRIPQIKZw3bNAKz5LY5Y+5VyL9DABb9NSXTw/y+2sPuZDPRyQRZDydkskOA2FTppW0/sFrScIgj2GWIyjyiJ60HqVgkrrw8NCa3hPURY+DRj6kIUnSMywKJOKSp3HfIBF35/qRm2wwdK71AQaEuCKolRCeOoa3brTUtHvkBuKz3q5L2TEzi5rV70nyK/VmwOVDI8ThucSfZRvJpC6NNuRgclyS3tqiA6/CqCDPP/sXOs4uPxzEcsyRovPlpUpqMUaclsRgkua6UOGMPNsIUveM0huK R0N8Ppxi TVtIlkm25pfzOECLoe0RT2x93edyngM+evEYkT/c7OqxysvR2mITBrQYFGHY3uiA+FNsdtWnA3duvs10RnXDRUTNamIzfQ72Rm/+U5W3NcYTiorrYuJ9QLeocBu09XJ+h3n/IlNJYtKpVqkRJm+sXpo4tIaSDnXiLFksGFsd4arNkOI5CL1qBuEPLEBtHfLRZ6P1FA9WSObqC7XRywy5u1zsVVUFN71dv3qtZlWBlgYnzkAbpdQJsdBzVZVTx7jTqn3W3tgLebRRj2QjWHUS1jwBevWpM8VGB7zqBu9qAtck/CJ7aadxKZ/vv4A== 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 Thu, Nov 2, 2023 at 1:50=E2=80=AFPM Johannes Weiner = wrote: > > On Thu, Nov 02, 2023 at 01:27:24PM -0700, Yosry Ahmed wrote: > > On Thu, Nov 2, 2023 at 1:02=E2=80=AFPM Nhat Pham wr= ote: > > > @@ -201,6 +201,12 @@ int swap_writepage(struct page *page, struct wri= teback_control *wbc) > > > folio_end_writeback(folio); > > > return 0; > > > } > > > + > > > + if (!mem_cgroup_zswap_writeback_enabled(folio_memcg(folio))) = { > > > + folio_mark_dirty(folio); > > > + return AOP_WRITEPAGE_ACTIVATE; > > > + } > > > + > > > > I am not a fan of this, because it will disable using disk swap if > > "zswap_writeback" is disabled, even if zswap is disabled or the page > > was never in zswap. The term zswap_writeback makes no sense here tbh. > > > > I am still hoping someone else will suggest better semantics, because > > honestly I can't think of anything. Perhaps something like > > memory.swap.zswap_only or memory.swap.types which accepts a string > > (e.g. "zswap"/"all",..). > > I had suggested the writeback name. > > My thinking was this: from a user pov, the way zswap is presented and > described, is a fast writeback cache that sits on top of swap. It's > implemented as this lookaside thing right now, but that's never how it > was sold. And frankly, that's not how it's expected to work, either. > > From the docs: > > Zswap is a lightweight compressed cache for swap pages. > > Zswap evicts pages from compressed cache on an LRU basis to the > backing swap device when the compressed pool reaches its size limit. > > When zswap is enabled, IO to the swap device is expected to come from > zswap. Everything else would be a cache failure. There are a few cases > now where zswap rejects and bypasses to swap. It's not a stretch to > call them accelerated writeback or writethrough. But also, these > represent failures and LRU inversions, we're working on fixing them. > > So from a user POV it's reasonable to say if I have zswap enabled and > disable writeback, I don't expect swapfile IO. Makes sense (although now you had me thinking whether memory.zswap.writethrough is a better name). > > But yes, if zswap isn't enabled at all, this shouldn't prevent pages > from going to swap. Right, at least mem_cgroup_zswap_writeback_enabled() should always return true if zswap is disabled. One more unrelated thing, I think we should drop the cgroup_subsys_on_dfl() check there. I understand the interface is only exposed in v2, but I don't see any reason why it wouldn't work in v1. Let's not make it harder for anyone who tries to expose this in v1 (whether upstream or in an internal patch).