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 05861C77B73 for ; Tue, 30 May 2023 16:53:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 64637900002; Tue, 30 May 2023 12:53:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5F5E36B0074; Tue, 30 May 2023 12:53:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 49722900002; Tue, 30 May 2023 12:53:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 376DD6B0072 for ; Tue, 30 May 2023 12:53:18 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id BC1211A021B for ; Tue, 30 May 2023 16:53:17 +0000 (UTC) X-FDA: 80847516834.25.FCA7D02 Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf18.hostedemail.com (Postfix) with ESMTP id B2A021C000D for ; Tue, 30 May 2023 16:53:15 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=pDuJR480; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.43 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=1685465595; 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=3aF7XT/V8A+EmUF4H6TrosCGCD/VolbgayQqm4IfFj8=; b=2/yzARYHqqKoPYkw7RHblbIvnmk9vyz4KPpH625TsWQVYMGxhMX9UClTA7lsyiaGZAz2fl k8I9bCxcNCDKDWJbnopGI4bG4P5nJ6jbbRCHuPybgmL1lZforwNvueDTzxU6GJC8RQFjla pS4GOnppjg4KoU7z+FdqgWmUcFdv6MM= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=pDuJR480; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685465595; a=rsa-sha256; cv=none; b=r4SJzZqyCW7UVl11xen63HXnoua1MzcBR+iKUhoxRl35UfIDMtZNMt+WNN6Flhzv8G2+0o ebF1LlK1fY1amHXavgOUPApjDqn09JTBqBPD1ijtde0CIts7KWRC7hZB+HRtIHzA0Jrli8 XpmRUTezXROIm9u0eG/dvgZSj4Q84HY= Received: by mail-ed1-f43.google.com with SMTP id 4fb4d7f45d1cf-5149390b20aso5890455a12.3 for ; Tue, 30 May 2023 09:53:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1685465594; x=1688057594; 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=3aF7XT/V8A+EmUF4H6TrosCGCD/VolbgayQqm4IfFj8=; b=pDuJR480Ut22E+122SeneUS/qZYcQos4cTdGtYoEzTIcNpP5N6GvjOH959lQL+utw4 a0KaW5Hj0d8m9uR0hLeoPP3DDBC+CahD50PIMQ41DQh5DFvIOtFHUrjBTt05Y4neWqlo maSL+ZK97MdtfCV3hBrWlvmANB5SA0uZoLGM+kLpz4sNyhpyMM54I4Tc20q9ANByD2ZN XMqqlNY6NrLBObrFxq3lS1vV7/DeVbBCff1EahUpUuve9hdjEnVpYwRx1oUC676g6Tgk JDq5xDEGI6G9b06SEtmpaDQeiV8IrYV9adVtBjXmla6zJP8s/ozMhjKiL7siBQxh/Wp4 nVZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685465594; x=1688057594; 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=3aF7XT/V8A+EmUF4H6TrosCGCD/VolbgayQqm4IfFj8=; b=DrMrFrxbFVvHXdXKtfM1cvcaPrcrPFL/s6svRMlDnotL4qeG+KkYoNqu+4iErlm0dM k61Z1B3ko/TLQmv8xXR3DeeODlW1bNVTU6zpvgLuVTH4/cXkoSBIox8su/LWu407u3dY fQ+xoDVOzDrCWEA+nEtnahnemyxLEFIpQTT/lXgrFXbLiNBJUZZBA8fj9XJ/A2kbqMwT AsAtyvXztC2D/Y6J35UuPWYXnQgflaV9ADmJrpZKDs6NklyisRxWd6Ra2e+JCmojtiIu 2mLxkzRFE2dKHqNMfhpjnpX+pFGw2MAT8FedJ/IHGecn9Cu/cb/lTHbJDkp6rPM80Urh SzTw== X-Gm-Message-State: AC+VfDxYx4vad/RU3m6Fd3ehAGyUsPAj+SISyQKK6KK/Pk6MQagg9xNe komyRa7YBThTt3aXUV031U+1WWlOmeT3mSHzJNx8kQ== X-Google-Smtp-Source: ACHHUZ5/nDAhDcXoOlJhT1KwpeTwEyHUXOiNccneHAYAYOvfYSB2cThFyg1g+cUdqhp0f5NDo9llx3ymUi/VcCegkok= X-Received: by 2002:a17:907:868d:b0:957:17c5:8705 with SMTP id qa13-20020a170907868d00b0095717c58705mr3119009ejc.51.1685465593698; Tue, 30 May 2023 09:53:13 -0700 (PDT) MIME-Version: 1.0 References: <20230530162153.836565-1-nphamcs@gmail.com> In-Reply-To: <20230530162153.836565-1-nphamcs@gmail.com> From: Yosry Ahmed Date: Tue, 30 May 2023 09:52:36 -0700 Message-ID: Subject: Re: [PATCH] zswap: do not shrink when memory.zswap.max is 0 To: Nhat Pham Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, cerasuolodomenico@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: B2A021C000D X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: skb3dtczjjy497yu3styyny9yz8buhpp X-HE-Tag: 1685465595-762325 X-HE-Meta: U2FsdGVkX191iBv/1gKTFV/Z+XJVWBl1QFsNDUjU5qRPsC71KGBGnWqyc9c+XU6QMmHV5h4VJ73AGUGosZYeFDGRiffOzOB1ixlOGXzcv12ik0VtuqtciwNYq6zEGY51lx0fJB8peviMCY5728zMPOv1Pr5cl09KPbn+UjJCEhzjI817AhzogI2BrzQvKT/IZX1m+lcqTJiz2C17528vP9Hge8BnT6iiCxBi6MXuhbzhgB9jOh+ddd4dfij692i27kx2G8sTtBLSbb4Kv0X3bxuGhL/3l9lqkwdPK92deJnksPbdYiI3hhiDKjXxICKVHVoqmG1Sx+eUxNpihk5xPjr+qx7y3QlU4JTDNy8DUTYLs5Y0nsnz54HBuYHR6gYkyZm+M4PiYlt51uPRaTJnxEt0Z0en5pWvB4bPLxmGSJyiqC0Pkh03HVKsE5FRwko8yP+PhJZ5h1YWRxxtWlL9bJp3m4JJHpQTzTwFEN/v9aYyFxhjEKssi0KUKk6WBCV4nZhJ3mhYkOq/PK4YdPepnhdQ1WMLXpBPLVETaSKQimKBSYk47TxM86nVRpI6DlnjqPe/B07FbH18NzBgy9yVHlrylHKpQWqw8I5q2M3JE7NDnjIqZs/U7sJ8yWgqooTPCqGTbRwF7eefxxyyLdyy89AD+9FzH9HudEU2X5yy8Qvj25iCahTV8V8LlCfC8gXf1ht1/fXY1Tty08RC0PkbH/AzIjcBPJt8MjZ0npZrKhV01VKg52AOaqo2VIvsq86cGu3cn5DTprX+uzG0gXc1VAM9KYigE65WHhRZaYv23zNJ+umouxTiHwCqfqb1xNz4u+FNP0QkcLAGCmT2Ou/JzvAYqXOhtxxZ29+We6i75Prcjdt1/2a48dcOg5OvdZh5D4e6SI1KkdX0N3wYDOrGIbX6ff0tLy7YzMIOo9D6CzOw5X0IhaJ5RLGMBXV9uCmaTT4SrU7D0M2L7ZDNhAI MiutXlhI 5OMmm3tIeuFeYnqIGP2VQnI3Wqdn8fTfuFAbbP2tkL4jr2FETsK5wt5+lqfOU3RDhUrkO4STF0eSNWbwKHIXDkNSFRL2TLYR2th7FzDDpExUlJxKEQfXB5+9A9K05/XMzqhmXdfwyVlizeo7pO6YTkeDRQlN87maM4kqsgPrwXctu5R+ArL81U1U09SBqwkDwAGX0z1J5kowaj6An7LeJlsj3+KEXco51Mu+KBvNqjyrWNQuxwochYAmamA== 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: On Tue, May 30, 2023 at 9:22=E2=80=AFAM Nhat Pham wrote= : > > Before storing a page, zswap first checks if the number of stored pages > exceeds the limit specified by memory.zswap.max, for each cgroup in the > hierarchy. If this limit is reached or exceeded, then zswap shrinking is > triggered and short-circuits the store attempt. > > However, if memory.zswap.max =3D 0 for a cgroup, no amount of writeback > will allow future store attempts from processes in this cgroup to > succeed. Furthermore, this create a pathological behavior in a system > where some cgroups have memory.zswap.max =3D 0 and some do not: the > processes in the former cgroups, under memory pressure, will evict pages > stored by the latter continually, until the need for swap ceases or the > pool becomes empty. > > As a result of this, we observe a disproportionate amount of zswap > writeback and a perpetually small zswap pool in our experiments, even > though the pool limit is never hit. > > This patch fixes the issue by rejecting zswap store attempt without > shrinking the pool when memory.zswap.max is 0. > > Fixes: f4840ccfca25 ("zswap: memcg accounting") > Signed-off-by: Nhat Pham > --- > include/linux/memcontrol.h | 6 +++--- > mm/memcontrol.c | 8 ++++---- > mm/zswap.c | 9 +++++++-- > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 222d7370134c..507bed3a28b0 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1899,13 +1899,13 @@ static inline void count_objcg_event(struct obj_c= group *objcg, > #endif /* CONFIG_MEMCG_KMEM */ > > #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP) > -bool obj_cgroup_may_zswap(struct obj_cgroup *objcg); > +int obj_cgroup_may_zswap(struct obj_cgroup *objcg); > void obj_cgroup_charge_zswap(struct obj_cgroup *objcg, size_t size); > void obj_cgroup_uncharge_zswap(struct obj_cgroup *objcg, size_t size); > #else > -static inline bool obj_cgroup_may_zswap(struct obj_cgroup *objcg) > +static inline int obj_cgroup_may_zswap(struct obj_cgroup *objcg) > { > - return true; > + return 0; > } > static inline void obj_cgroup_charge_zswap(struct obj_cgroup *objcg, > size_t size) > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 4b27e245a055..09aad0e6f2ea 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -7783,10 +7783,10 @@ static struct cftype memsw_files[] =3D { > * spending cycles on compression when there is already no room left > * or zswap is disabled altogether somewhere in the hierarchy. > */ > -bool obj_cgroup_may_zswap(struct obj_cgroup *objcg) > +int obj_cgroup_may_zswap(struct obj_cgroup *objcg) > { > struct mem_cgroup *memcg, *original_memcg; > - bool ret =3D true; > + int ret =3D 0; > > if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) > return true; > @@ -7800,7 +7800,7 @@ bool obj_cgroup_may_zswap(struct obj_cgroup *objcg) > if (max =3D=3D PAGE_COUNTER_MAX) > continue; > if (max =3D=3D 0) { > - ret =3D false; > + ret =3D -ENODEV; > break; > } > > @@ -7808,7 +7808,7 @@ bool obj_cgroup_may_zswap(struct obj_cgroup *objcg) > pages =3D memcg_page_state(memcg, MEMCG_ZSWAP_B) / PAGE_S= IZE; > if (pages < max) > continue; > - ret =3D false; > + ret =3D -ENOMEM; > break; > } > mem_cgroup_put(original_memcg); > diff --git a/mm/zswap.c b/mm/zswap.c > index 59da2a415fbb..7b13dc865438 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1175,8 +1175,13 @@ static int zswap_frontswap_store(unsigned type, pg= off_t offset, > } > > objcg =3D get_obj_cgroup_from_page(page); > - if (objcg && !obj_cgroup_may_zswap(objcg)) > - goto shrink; > + if (objcg) { > + ret =3D obj_cgroup_may_zswap(objcg); > + if (ret =3D=3D -ENODEV) > + goto reject; > + if (ret =3D=3D -ENOMEM) > + goto shrink; > + } I wonder if we should just make this: if (objcg && !obj_cgroup_may_zswap(objcg)) goto reject; Even if memory.zswap.max is > 0, if the limit is hit, shrinking the zswap pool will only help if we happen to writeback a page from the same memcg that hit its limit. Keep in mind that we will only writeback one page every time we observe that the limit is hit (even with Domenico's patch, because zswap_can_accept() should be true). On a system with a handful of memcgs, it seems likely that we wrongfully writeback pages from other memcgs because of this. Achieving nothing for this memcg, while hurting others. OTOH, without invoking writeback when the limit is hit, the memcg will just not be able to use zswap until some pages are faulted back in or invalidated. I am not sure which is better, just thinking out loud. Seems like this can be solved by having per-memcg LRUs, or at least providing an argument to the shrinker of which memcg to reclaim from. This would only be possible when the LRU is moved to zswap. > > /* reclaim space if needed */ > if (zswap_is_full()) { > -- > 2.34.1 > >