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 B9AD7C5320E for ; Tue, 20 Aug 2024 09:38:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 290CB6B0082; Tue, 20 Aug 2024 05:38:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 240A06B0085; Tue, 20 Aug 2024 05:38:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 109796B0088; Tue, 20 Aug 2024 05:38:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id E662E6B0082 for ; Tue, 20 Aug 2024 05:38:27 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0C36D418AC for ; Tue, 20 Aug 2024 09:38:27 +0000 (UTC) X-FDA: 82472123454.06.A39040D Received: from mail-4323.proton.ch (mail-4323.proton.ch [185.70.43.23]) by imf19.hostedemail.com (Postfix) with ESMTP id C81701A001A for ; Tue, 20 Aug 2024 09:38:24 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=yhndnzj.com header.s=protonmail header.b=qcW3ZZOe; dmarc=pass (policy=quarantine) header.from=yhndnzj.com; spf=pass (imf19.hostedemail.com: domain of me@yhndnzj.com designates 185.70.43.23 as permitted sender) smtp.mailfrom=me@yhndnzj.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724146641; a=rsa-sha256; cv=none; b=JuEabNg/wlUZAn3PnRJR63I++OLOv4/KSk4LDoLZfneAkvafRmaAxJdRoOjNU0bo+nkV+Q QGBP4VEccU7kGvLTI/g1yYkzbZ+dARCXtsSG6E1OAsaVzzZyDhkIU0kxkaxhY3HyMR3qIG L7CnEKmiVRt9jjfHqK+wn9D0D8qoYXQ= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=yhndnzj.com header.s=protonmail header.b=qcW3ZZOe; dmarc=pass (policy=quarantine) header.from=yhndnzj.com; spf=pass (imf19.hostedemail.com: domain of me@yhndnzj.com designates 185.70.43.23 as permitted sender) smtp.mailfrom=me@yhndnzj.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724146641; 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=aG8Is3bPBXGEjPmskOUz3pigaZCNCWXw8yXDoLVbfRI=; b=Y1Btlv173mWq0IYZGwHVO/PfjSQLWPbAIDUcFUbSCJeLtW3XKow8y07f+DX6XR20O+3bDB 1rhXnZoRKZyJ6QEMeftPa8UyABT+l9PGwjb3JfIzeNXg1k40R/EMkSm62+K0B5r03v5psG 6OFE+i90HEPUppftQTUD75Nov4uHbKM= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yhndnzj.com; s=protonmail; t=1724146702; x=1724405902; bh=aG8Is3bPBXGEjPmskOUz3pigaZCNCWXw8yXDoLVbfRI=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=qcW3ZZOeCYyr6Kighv5Ee2LVSQuu1oCHSNvPljlIRGCPACT70maY8uA+TgLntLey8 ay5Dv3rREFmTdK1NATFjSlQxxvEdN+2tGkDkCBHVmoZlWwgtlneaDB3FD5w7h9RX37 QivLLJ7HLEdCaq0FldgnAONUgFIN8LWCsOp/slvTDTNftSBejXhDEnEm+uhfMcBui/ auKj4S8i1NZRYDdMzNmY2yxP3xB3wK6SLOa4aVFJ4i6r6U9eBuvanZ18ese/OS/AXL bvuaL9MVXtqWrgT9UGsSxM9Cko5aXnqhSzs2bjjhWxQ9QdZyb7hk0v4H+TMoMGfoSM BDWD8EZvzByMw== Date: Tue, 20 Aug 2024 09:38:14 +0000 To: Yosry Ahmed , Nhat Pham From: Mike Yuan Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, Johannes Weiner , Andrew Morton , Muchun Song , Shakeel Butt , Roman Gushchin , Michal Hocko Subject: Re: [PATCH v2 1/2] mm/memcontrol: respect zswap.writeback setting from parent cg too Message-ID: <45e2c372f59748262b6e4390dc5548f8ebf6c41a.camel@yhndnzj.com> In-Reply-To: References: <20240816144344.18135-1-me@yhndnzj.com> Feedback-ID: 102487535:user:proton X-Pm-Message-ID: 40197622d762237332e9e1d1b626e89a528eff27 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C81701A001A X-Stat-Signature: p7pfmhw4hg9fo5z9q6e3h7xqxg1kan14 X-Rspam-User: X-HE-Tag: 1724146704-578947 X-HE-Meta: U2FsdGVkX1/exQQctP1eLMZsqq6i+u8s/TlOuRGuhVEjJxccfBoNgQTXReaPRZnPL3kzkB/Iv2Vj2MllLqGHyUYZWXO+TYhmfjDXt/46yvexCTSgO+1A9EUw5/sjNPz2awr+5HSV0OXqiGVTjGKHabJWZQk/cFs9UErmiw7B/rRhS385Muznao1eUNnW+1bkxfvkCiqiZC7LnWE808chCexVvOFA1Z1618iiLRTjEJywC7tDM2mdO2mnjVLycmWphYriY9oDfpn4z3Uam4ccvAx2uj+lEMsQmB0s6kudNNVQu1Dh06q2f4e/Qe2nWu1o+lDCgoPB+gOLeb9l8s8SlgfI4Gi4JGrffF33OOuhBM1Tijzrig//CtIFQmNJ4xJesrXR+DOOENIT059hG9TMqROsFGLX0SnIrAfoDBKdSfdf1KUbOacnCeBi1SRYb2hh+vdY2RnPXQTrH6KOW82rqVXu5aQc0Vm+jxeb38SkJqFYAk8jDBqWfiQg+roa+UMs6YP7JoE3/1O6cJwgcFqtJ/yh8PjXbaTBLU4Z02Uo1Z3Aafao1eDmyZh+zdPe9TB5zNHOi1n7FCEkAxyurOsUG2vzhiXh4ziOWFEXAv5Jrz+VTj787PRzwONXQp2WomwJnzY9AF74TNuvFLkVMz7zrjRrt/04IBJU07UWS7P2Ay9LedARMYJMWaDKv1PRRL6E+h1CSfHl20dGPQhSC0PxGFj7oOSYpAGuHMIXZJNz0ZzSWD34sw5tdUVMTfZHu9zjHv64lWf2VoOT+3OJRc4w3X4iraOhYk1PsvZkLEu4Kn+JWmQ+EyUbHb/2eQv9vRCEtbmgTKG/3ihZ/B/BjSyAu2pKirBxfzUxCw/ni6kQ6ZNA04uY2pAWFyc4ZoCDG/mZEQyZnkZnzi7sRSVc7YQpHTizaf2Hxh1I7oPXDvUMclIgelRRjhtOg+wqcp+e7p9bUmglKJWmll2GAjpr0/r 7oyUhPGL bp4wjvhWswLQTPYf/vNXFWPICDj6BGR+yK/fAKRJAcs/kGwW4sGkggI/qshypaiSbxtGgEJ17sLRYwioPlwy9uoOpf0Ii/F7vJ+sdXNtc89lIY4RIrsK5+QeOG2Q2GbKT6+wKBrMs+5vPnDQwoMxtECpoe8T4r2E18apOZ+n9xmCGEDJ6IoF+3tL13xesbkST55UBOoA2K7OUVlXLE7IQ7s9L6nnVichaGuBCJrtEbk0N/ULHQ6JlAF1vbC1yTlRtuDP5TCb5v/Xn+kgYx7/9VF7eThztVW4mzSdyApFZugC2pNFoo41imUKTNJDqZl5TN59RDwy21aOFot6Py+ZyBJ5Rxc20jQ8jZ2lYbx3nXwbZK1gvfbzJo9VJNEV/mEUR6ayer3TaWKp3pQScTNPwDSKbfwKr11UJfMrUxlk2i4UpW1CYONVPAYo4O2vdEtqQ9NvSJq9b0JeMxMFvPHbQz9G3Kz+0czAZ02ezavUCma1/QrYxcG4IdOOksqR+cQ4IJU5O89icEALHL6DYCCB9hvjAdA== 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 2024-08-19 at 12:09 -0700, Yosry Ahmed wrote: > On Fri, Aug 16, 2024 at 7:44=E2=80=AFAM Mike Yuan wrote: > >=20 > > Currently, the behavior of zswap.writeback wrt. > > the cgroup hierarchy seems a bit odd. Unlike zswap.max, > > it doesn't honor the value from parent cgroups. This > > surfaced when people tried to globally disable zswap writeback, > > i.e. reserve physical swap space only for hibernation [1] - > > disabling zswap.writeback only for the root cgroup results > > in subcgroups with zswap.writeback=3D1 still performing writeback. > >=20 > > The inconsistency became more noticeable after I introduced > > the MemoryZSwapWriteback=3D systemd unit setting [2] for > > controlling the knob. The patch assumed that the kernel would > > enforce the value of parent cgroups. It could probably be > > workarounded from systemd's side, by going up the slice unit > > tree and inheriting the value. Yet I think it's more sensible > > to make it behave consistently with zswap.max and friends. > >=20 > > [1] > > https://wiki.archlinux.org/title/Power_management/Suspend_and_hibernate= #Disable_zswap_writeback_to_use_the_swap_space_only_for_hibernation > > [2] https://github.com/systemd/systemd/pull/31734 > >=20 > > Changes in v2: > > - Actually base on latest tree (is_zswap_enabled() -> > > zswap_is_enabled()) > > - Updated Documentation/admin-guide/cgroup-v2.rst to reflect the > > change > >=20 > > Link to v1: > > https://lore.kernel.org/linux-kernel/20240814171800.23558-1-me@yhndnzj.= com/ > >=20 > > Cc: Nhat Pham > > Cc: Yosry Ahmed > > Cc: Johannes Weiner > > Cc: Andrew Morton > >=20 > > Signed-off-by: Mike Yuan > > Reviewed-by: Nhat Pham >=20 > LGTM, > Acked-by: Yosry Ahmed >=20 > > --- > > =C2=A0Documentation/admin-guide/cgroup-v2.rst | 5 ++++- > > =C2=A0mm/memcontrol.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 | 9 ++++++++- > > =C2=A02 files changed, 12 insertions(+), 2 deletions(-) > >=20 > > diff --git a/Documentation/admin-guide/cgroup-v2.rst > > b/Documentation/admin-guide/cgroup-v2.rst > > index 86311c2907cd..80906cea4264 100644 > > --- a/Documentation/admin-guide/cgroup-v2.rst > > +++ b/Documentation/admin-guide/cgroup-v2.rst > > @@ -1719,7 +1719,10 @@ The following nested keys are defined. > > =C2=A0=C2=A0 memory.zswap.writeback > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 A read-write single value fi= le. The default value is "1". > > The > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 initial value of the root cg= roup is 1, and when a new > > cgroup is > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 created, it inherits the current = value of its parent. > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 created, it inherits the current = value of its parent. Note > > that > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 this setting is hierarchical, i.e= . the writeback would be > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 implicitly disabled for child cgr= oups if the upper > > hierarchy > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 does so. > >=20 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 When this is set to 0, all s= wapping attempts to swapping > > devices > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 are disabled. This included = both zswap writebacks, and > > swapping due > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index f29157288b7d..327b2b030639 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -5320,7 +5320,14 @@ void obj_cgroup_uncharge_zswap(struct > > obj_cgroup *objcg, size_t size) > > =C2=A0bool mem_cgroup_zswap_writeback_enabled(struct mem_cgroup *memcg) > > =C2=A0{ > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* if zswap is disabled, do = not block pages going to the > > swapping device */ > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return !zswap_is_enabled() || !me= mcg || READ_ONCE(memcg- > > >zswap_writeback); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!zswap_is_enabled()) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 return true; >=20 > This is orthogonal to this patch, but I just realized that we > completely ignore memory.zswap_writeback if zswap is disabled. This > means that if a cgroup has disabled writeback, then zswap is globally > disabled for some reason, we stop respecting the cgroup knob. I guess > the rationale could be that we want to help get pages out of zswap as > much as possible to honor zswap's disablement? Nhat, did I get that > right? Hmm, I think the current behavior makes more sense. If zswap is completely disabled, it seems intuitive that zswap-related knobs lose their effect. > I feel like it's a little bit odd to be honest, but I don't have a > strong opinion on it. Maybe we should document this behavior better. But clarify this in the documentation certainly sounds good :) >=20 > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 for (; memcg; memcg =3D parent_me= m_cgroup(memcg)) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 if (!READ_ONCE(memcg->zswap_writeback)) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return f= alse; > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return true; > > =C2=A0} > >=20 > > =C2=A0static u64 zswap_current_read(struct cgroup_subsys_state *css, > >=20 > > base-commit: d07b43284ab356daf7ec5ae1858a16c1c7b6adab > > -- > > 2.46.0 > >=20 > >=20