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 E3BBAC4332F for ; Thu, 2 Nov 2023 22:40:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6643580032; Thu, 2 Nov 2023 18:40:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 613FF8D000F; Thu, 2 Nov 2023 18:40:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B4A480032; Thu, 2 Nov 2023 18:40:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 38A5E8D000F for ; Thu, 2 Nov 2023 18:40:31 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 04DB3C01F9 for ; Thu, 2 Nov 2023 22:40:30 +0000 (UTC) X-FDA: 81414484662.12.A562D09 Received: from mail-il1-f179.google.com (mail-il1-f179.google.com [209.85.166.179]) by imf26.hostedemail.com (Postfix) with ESMTP id 4D6FB140019 for ; Thu, 2 Nov 2023 22:40:29 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fJR7wQ00; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf26.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.179 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1698964829; 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=Qc8asfYOeO/AOwc/brrfBGOOvr6zT/uDTRraWYt21O4=; b=yQ0TGFwnb0vuvOUv8WECTlONqo+BsVexZzmJAGiqx8fALJvdAQBhV7gaATjAsP3UCBQWF7 A/8IGkeaVukfkINX7PAi32aLNKp8CfUSKbpNhSLZziTe2cVSgb9ms2+ymmkQ/b1G6POejp 0cuIshrh//1uVAgkOooDLyfButRO6fc= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fJR7wQ00; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf26.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.179 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1698964829; a=rsa-sha256; cv=none; b=yq/P5OEUPkl0ojMgvPRa/Rabt5x9PNYIjpr71jw+v7f790019h2JNUWFQ9Keh50MyJdQV6 VCILMYprKZvDaPvB0uyaIDAD/jR8je6JXfICNrzn6uvrULsq8JnKUKepq2tYCKcUmgI1sS gDLY4w5+un0Y9T00IuddW76ioV1BXhI= Received: by mail-il1-f179.google.com with SMTP id e9e14a558f8ab-35957d77afcso2136105ab.0 for ; Thu, 02 Nov 2023 15:40:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698964828; x=1699569628; 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=Qc8asfYOeO/AOwc/brrfBGOOvr6zT/uDTRraWYt21O4=; b=fJR7wQ00kseX7z+lyuRzhJ/RIT9aUCo1ZdRlu4HovuytJTcyLdIbR0gHFhWLFloBWz IZhcjqM2ll2OGIiZm/BibKr+7JyD1uwrMVvkfdh+4O2CjDKa6i/yVc3WpyO3Cwx08Pi5 OMXmFXTS++mN8li4ZHnQnpEQXZ1wFkva0ojdshZ837JQhKL3I/Dr+nZCkivg5FzMJRY5 IQDTVcgFYwUK71TnHZqAeTa1r7gZWq7NLPzjhufe9TKe/CShIYNAIzgYmLOl82XToOTW K+J0FVXWTehrSXGZ1CB7+s3k+oBum0Z5YS5UFq15Sv0koi+OHu7gv0jbAi7Jh+7nu3ro V1qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698964828; x=1699569628; 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=Qc8asfYOeO/AOwc/brrfBGOOvr6zT/uDTRraWYt21O4=; b=qeWQ7gfSEBWSuoBheRz7rhENFk1v0//h1uzhZLr6JoWOMG6bSV3eKC+Gm0j7+8RhrI cBRZi3KcQXm0VHo0ztv8hQnW/56Nsvuq7JttL4G6hFCHi0EUmVS3kmM172vvwPQGNlAU diiScB4k3xj8U6Z8vImjtTJq0ZkqymEal8bI9J94BciQVorS3IEHoZLlK/69maVMxwRH T7uJMU4ENNKuPoJXdRc/CyFlM2/3n9rGMuBjW9yuZ9ILJHxInvfgSzzxaHfIHkGC6h1K xUiaoyzvViNSFLLHTgRqoCjLUYNPFe+kYdn0xUrxbfpxZJC0GDJQG3V184Vy8A70eqK8 kqwg== X-Gm-Message-State: AOJu0Yy5nXmUNKVsZZ62czlJBAN626+e1Mx3cmJ9kPWSpmkHJ8Fmw7+V SOpinUTpgzSghNBVfBHLcohX6EKPIEFUUHjTJA4= X-Google-Smtp-Source: AGHT+IEAD4K9sHQRK7LJB/PKdoDtBLxq6pbnMYlsV5/1F/rg1Wi1wcKDZddZCjp5HWhGR6oag/OCQe+mARwPp1pjcfY= X-Received: by 2002:a92:d74b:0:b0:359:3ff:17c6 with SMTP id e11-20020a92d74b000000b0035903ff17c6mr943748ilq.4.1698964828290; Thu, 02 Nov 2023 15:40:28 -0700 (PDT) MIME-Version: 1.0 References: <20231102200202.920461-1-nphamcs@gmail.com> <20231102205022.GA3265934@cmpxchg.org> In-Reply-To: From: Nhat Pham Date: Thu, 2 Nov 2023 15:40:17 -0700 Message-ID: Subject: Re: [RFC PATCH v2] zswap: memcontrol: implement zswap writeback disabling To: Yosry Ahmed Cc: Johannes Weiner , 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-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 4D6FB140019 X-Stat-Signature: gtrchixqdtm64qjegnugxyihoqjamixr X-HE-Tag: 1698964829-946376 X-HE-Meta: U2FsdGVkX18tRugXI0+S4QRmmwkVfXyiod45L2CWCfGXNgBPd9w8qWuvilNWDb3cuFT9aA2cyV7QQjLSxrXgttl6gfNXBGMFJlPzKZL2h2Au39YHREGT80HbvO4Likk7+AUBwse7ydgNWqFDz3dY5fH9t4LAUqLL0LeClw5E/AFd3E5gpMKf7Gj1hlIljDt5W/EO1YmbgC93fWuA8DZ7bVDyPd7JwmfchcCCTaxQLiatg8yTrUmHQqQBAVsKfi4eoiqPPt+7taACgbwbOo0QfWcMUfibGWy7nEriKKDHTsDnRQmIMG/xw4l+kxnxwdhoeod79NwIa80L1ZEI5iJ1s6hLIiyO5qp1Ay62Xp5vBcHhnXjgjQcs5uwmcLNjAgnooZ/LmnhHuEUJYJV6W2u4267kvXS+A9r091oJuqbF/zJrn8y9GgtaoGd92AO8tFla360efBP0vo4X7GmZjo/A6UZjrDg7jJWsN4y0ECWTXUpF39A+5u0aA+l3V3k0K6AJ71qHCeJKGTFb/AfAPGsszQMKoOZonTIoNygOj3Q/krx72/BG3RArtYUeesTJzaAppXg3pRk+uCk/4JQun6XrgfQjHtGhN/uMchOpHVmzeYjmAoHz5vk4Iamy3ljzkaIAr0vNyi7VvFjVwkPJDZYH6Nfh0x7YZZDP2qDr8nq1t3sgFsyREW6Izh8c6WH3poFqADP7ukYDXMaa2hc9zFAiQQ6ze/Q1+7eXaVrDP1nmgO8lE9dfS6J+nEVzoRG5FjgRlKoywpwdRDd6+PFMhdw+LDESebmKihQdGTgrTisbnVCoOMIqp4RLUu/iiQoANLoBGR34OG2l1W6seCPdXmcYJDZghB+6dBWW1ftKcAxsjvXnTD+o24s3uz6OvqRVdyVDwYjHdj+rAKfpgaHafBx4RIlKcbLSI+nzF6ztn0TDtNVKulUcQXi9nQE2tyeWS/HVNzMopZ9QRAXhdMcfQxk O70+Qoqu KSqTbtDBKzP5R4cmDhJdCqo/t8v4BHomBP+rmiQoJLn/KpR6Owaua+yrr6geP1uSARA3wtawIEeXY3Alb4x0ww/Vib2oUTznRRUfarBQzGyVkmEEXTTKCwb6qsKaWnLHRoGYLG6Q+wfH+byGNGA7vdvx7P/H5GZEhFJE7Vmb1vbBHYOQQrEWBaF0xJICiivw7+GIWv9gQCp0WWuYNfmFOnkvTbRmtj6NkP6TkPe3BwhnwyppjDDtTI+enGP8kfbrxVkAaOSMa+xemyEaL47DR8dfp/tpYNc6XXRDK8boVp/Q/0GClqwX9wpQ7GTwN6TU+FsULOqBKUYZdi4X6ncVmV+4hiN8VdTiZbWQlcYdXZFTj1FsOrhvxF0/LKWQjDXcExea0gApimYpv8Q0LCSeGdRjvjgKbSxtirel8xSlKaBHFmgEobtigvieYOkJtShna+mc6om2XrboH7KQ= 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:54=E2=80=AFPM Yosry Ahmed = wrote: > > 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 = wrote: > > > > @@ -201,6 +201,12 @@ int swap_writepage(struct page *page, struct w= riteback_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). Hmmm I lean towards writeback because it's already used in zswap context. Users not familiar with the writethrough concept might be confused by the naming. > > > > > 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. Sure enough! In the next version, I'll always return true in this case. > > 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). I don't have anything against cgroup v1 per se :) I just happened to notice that zswap charging is not available in v1, so I just played it safe here. If nobody objects I can unguard this feature from v1. Seems reasonable to me tbh.