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 DAB77C52D7F for ; Wed, 14 Aug 2024 20:22:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 03B0F6B007B; Wed, 14 Aug 2024 16:22:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F2CAD6B0082; Wed, 14 Aug 2024 16:22:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DCD6F6B0083; Wed, 14 Aug 2024 16:22:43 -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 BDA3D6B007B for ; Wed, 14 Aug 2024 16:22:43 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 7788B1A1186 for ; Wed, 14 Aug 2024 20:22:43 +0000 (UTC) X-FDA: 82451974206.16.529A68F Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by imf15.hostedemail.com (Postfix) with ESMTP id 98A72A0009 for ; Wed, 14 Aug 2024 20:22:41 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=CacSny2p; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 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=1723666889; 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=2f7WeL/F6hdyZXXE2Ld3C8xDMLscvOaz+ELyawQJbHM=; b=gbKMWSQGeNYkTfrDrhtpje/075uMD7gwGE4BFB3LlWQgOikA7vQ+Vozq9yZheaO9K2C5Q4 tOfUI3K/CUTBNlC/9N+ALDI0ECt7voBJCYwsaTxfALUJR33XiFUdnp8A19FwHwCW43x4+W /BXBp5iI5tD6MZdQ8YGR1cy/7OMRGLc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723666889; a=rsa-sha256; cv=none; b=A6g48esxeRBWw9TTZTm4C/aZTboRtKR9DX0pwm2KJ/HeNuH4Hs+tu3YLJr83WYDPHjUCN8 /VaxZXmtkoM2USkCbA9RaMG+Neh6dMlPeX6N+wjvlwOxjlGcw+A4asfcaDLxQqu39HG8Km fgiQNMFEZz4UF0v328ZPrFAsK0ROauA= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=CacSny2p; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-a7d26c2297eso40315966b.2 for ; Wed, 14 Aug 2024 13:22:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723666960; x=1724271760; 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=2f7WeL/F6hdyZXXE2Ld3C8xDMLscvOaz+ELyawQJbHM=; b=CacSny2pZPGnJQCnB9gam9aXAYggbCIU3iWy0iB7EbtSW//BjVf9VMZYGcak8OPBE4 X/t9NvGUj5ZGI+/Hbc3uwBnqOtiw0Ufd78xhlwxdGLtZ5bC1vx+MCrr2aI/ARPPRkeCz Ukt7MGQDKLHVEOw7l+0TWRdDBF70shRpFKW33AiDKMAwZ3M58DnrnaCXYklUPZFWkPPV 70v4THF/7e9IjhB76tZaQcJbGQXEw+2ZKK2VCCadubyYC+zeIXWHNiSDniFZVei2drqi Xc8UF1ijlhnhIOUw21tQDZeXC1SiqR+ne7aoEXrlNLULnkqGklqDlMyBNBfsVW1l6Kld wqgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723666960; x=1724271760; 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=2f7WeL/F6hdyZXXE2Ld3C8xDMLscvOaz+ELyawQJbHM=; b=Q+kwiWzKTzvkaLzIJZdgyZEAI+RLZzfm8DBJCdIr93dcpQXx80VD/Dn+jYrpjq+p2a zS3rSn0+KWGNtl7uunMmKi+mcyjTgDK4myUIXkXsWjLZ21hdHL1BaPqgSHBY84RFcas2 LwkUUF0aNAmoSX6ZmObQDWRpxZGU3gXqL2/sBXmXFUMb2mWF0oaF9V18YJqKtRH4heIs rDlLVWFie5p4wd8t9FvpSCThsDei4dTXnxXzAtCtP6KOGwqkiptsx6zNOQPugJybiuPM +tlLLrpkyG8zFp7/4gBTgw8LsyzL1jPUilTlnAu+qZAW8WT1gbI9epotdLFxic3a3va4 zWZg== X-Forwarded-Encrypted: i=1; AJvYcCW3PyEjZryKpRWWQ8iUvHxXKm6p8DvCrLfTaW6eprTMPyHezGzWpcGn5sbeyF4f3ctLk9ioABk7es3HPrmNPqETc5o= X-Gm-Message-State: AOJu0Yw+Vk4w3LENbzSCAx0dBDwZKW39JB9dVQCttD/WWts/aPBn7Sxi JHESg2+Xmfg8q4Mf0FiMd37hSTQbH5g50d3CdEJTvsKBRp5A5vbkp2ZuJwptRkErtV0A3FqxXEe 4o416Pmu6TuQENHu5m8LM755E8zV8SPCnCzSJ X-Google-Smtp-Source: AGHT+IHwQ61fqTYGKDPjIPhRrXO7HDN0W2Mhl5Sz3JZ411uPh3wwlcX72uzM3/bHwcG34/XBmX+WMrAIjLKBdz4lwIQ= X-Received: by 2002:a17:906:4fc7:b0:a7a:a5ae:11bd with SMTP id a640c23a62f3a-a83670723e6mr287623166b.67.1723666959052; Wed, 14 Aug 2024 13:22:39 -0700 (PDT) MIME-Version: 1.0 References: <20240814171800.23558-1-me@yhndnzj.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 14 Aug 2024 13:22:01 -0700 Message-ID: Subject: Re: [PATCH] mm/memcontrol: respect zswap.writeback setting from parent cg too To: Nhat Pham Cc: Mike Yuan , linux-kernel@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, Andrew Morton , Muchun Song , Shakeel Butt , Roman Gushchin , Michal Hocko , Johannes Weiner Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 98A72A0009 X-Stat-Signature: auksxukbn41j3hp75yedzdzsjqbz1198 X-HE-Tag: 1723666961-30147 X-HE-Meta: U2FsdGVkX1+Jx9wlNBomKWLJPIv8U1wftetb3u0UyAyBNWFPpQwjYSFmzwKSNxNIi6bcCkS+FZjsn8+4quVLYzcV1Szjb6cWl3XKtRi9dnuVs2Xao5ZQZ2tGu25MxjkeMGL7Nd7WXGhlHJwhmOu/SvT/5vs0JicZB7bVHpQt+lx6ZLR5MOrBcBLAjt6+SGDhA326MjqHDVwtg4VgzNWjMuWFuv1nWCVVcSSlRQsxlyCFbxC48BQWSOfacUYIPIKDcRT8CkMo3tLo3JdefPd9Bcyvu0ZlZF7FvoJcokMqbvP+qSF1Qg2o+BclGEIXyWAni6diwYt5GM+XeTqYJ9zsQ3e+muwOTVkVQrSPNW6LQUpxf6ULVwgoN3g3aM7KTrewRzc1f7+oYqXCUpXW5LeOVfiyb5GnSMxBFXSMtOop62TN7mq2nGmgJa/IttJy1t+Ed9z228lerMJUnbmTs6AWFsyy94JQ+9wKXWJXZzjqIMHDxZC1Ms+ck0tt4nEwj4iby9FxJb/O434jCkpDr33+mt9LP6qY/de7oTvEbCQD0b2h+q1vIOsfY12GX1B6bsuSl+34C63GkMB9cnUd7U5vGzQXgzgfyeYgPIlBhXwGBP1oURgikThtbVf8Tu8Dn1qdkqdV5tgq59oONthjpD9VnPqXFFQhKCpiLGmSf7jTqoKR84TU3zY612ezd7Bpzq5NzFLoVLsSjcpdNMkQKY06631gmPiDIGnxfcirVy0iaxXCRamoOD9lCX94CCs3yC8Tpi70tC2OQhHKOIN7c5Qo3+TSW1gYQMw2mlXxeHicdoYwrnuTyolwwfqXO7ciLXvv0mHtQC9vg0md684+D706QWr4tUFJA7QMaJEk+clQZbTb9lPu2CwwZ0tbMXD561FHaZGb8/7pR4k3LEplN7qQwIWvIo0cLwv8NRXAIEX9SBHuH+EPcD+JrrOFmQVW1aMgxaGGoslD3ttkQ/uU0BU sv7IQpqW iYHyuFkzzOFLoFM151RsZOl6BzFGMM03hdYLjGdNKwKneM3tXt2sJ8cWMKZf9QJRR0FWUV4vXiT2W64dmZn5F3zR7HjCHJ6yFG/JbghoZ2yukEB4FKpYDtSMjfUuYsDOJA9ytz409ZRwhN6eDZ0b3CAYD0cunkN01FuZ8vr7RR5vVDTewg2SQ/5Oku51RFGQ1HkwqBPzma9wu1zR27MddiBN0kgDRzmUXwp/B8xk62sT5uxHKJipDriLDD5a5d2LlDA+VJ8Gj8EdN8IhvsL+sRoNGpGlYdIwcIRPY+WcT+IwS26TitOFbQIaaS1VLkafeAzIgK2ab2F5isb13YZCLvpQNUw== 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 Wed, Aug 14, 2024 at 12:52=E2=80=AFPM Nhat Pham wrot= e: > > On Wed, Aug 14, 2024 at 10:20=E2=80=AFAM Mike Yuan wrote= : > > > > 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. > > > > The consistency 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 inherit the value. Yet I think it's more sensible > > to make it behave consistently with zswap.max and friends. > > May I ask you to add/clarify this new expected behavior in > Documentation/admin-guide/cgroup-v2.rst? > > > > > [1] https://wiki.archlinux.org/title/Power_management/Suspend_and_hiber= nate#Disable_zswap_writeback_to_use_the_swap_space_only_for_hibernation > > This is an interesting use case. Never envisioned this when I > developed this feature :) > > > [2] https://github.com/systemd/systemd/pull/31734 > > > > Signed-off-by: Mike Yuan > > --- > > Personally, I don't feel too strongly about this one way or another. I > guess you can make a case that people want to disable zswap writeback > by default, and only selectively enable it for certain descendant > workloads - for convenience, they would set memory.zswap.writeback =3D=3D > 0 at root, then enable it on selected descendants? > > It's not super expensive IMHO - we already perform upward traversal on > every zswap store. This wouldn't be the end of the world. > > Yosry, Johannes - how do you two feel about this? I wasn't CC'd on this, but found it by chance :) I think there is a way for the zswap maintainers entry to match any patch that mentions "zswap", not just based on files, right? Anyway, both use cases make sense to me, disabling writeback system-wide or in an entire subtree, and disabling writeback on the root and then selectively enabling it. I am slightly inclined to the first one (what this patch does). Considering the hierarchical cgroup knobs work, we usually use the most restrictive limit among the ancestors. I guess it ultimately depends on how we define "most restrictive". Disabling writeback is restrictive in the sense that you don't have access to free some zswap space to reclaim more memory. OTOH, disabling writeback also means that your zswapped memory won't go to disk under memory pressure, so in that sense it would be restrictive to force writeback :) Usually, the "default" is the non-restrictive thing, and then you can set restrictions that apply to all children (e.g. no limits are set by default). Since writeback is enabled by default, it seems like the restriction would be disabling writeback. Hence, it would make sense to inherit zswap disabling (i.e. only writeback if all ancestors allow it, like this patch does). What we do today dismisses inheritance completely, so it seems to me like it should be changed anyway. I am thinking out loud here, let me know if my reasoning makes sense to you= . > > Code looks solid to me - I think the upward tree traversal should be > safe, as long as memcg is valid (since memcg holds reference to its > parent IIRC). >