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 69369C27C55 for ; Mon, 10 Jun 2024 20:28:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D6C1E6B008C; Mon, 10 Jun 2024 16:28:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D1C4B6B0092; Mon, 10 Jun 2024 16:28:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE3BC6B0096; Mon, 10 Jun 2024 16:28:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 9F9566B008C for ; Mon, 10 Jun 2024 16:28:05 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 4AF071C02BA for ; Mon, 10 Jun 2024 20:28:05 +0000 (UTC) X-FDA: 82216115730.30.B8DB2C1 Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com [209.85.218.42]) by imf30.hostedemail.com (Postfix) with ESMTP id 772608000D for ; Mon, 10 Jun 2024 20:28:02 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=G+pH6SJH; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 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=1718051282; 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=CmaAu+iZsuEjEiEeynNp1R7+/jNy6Qzg+Lrp5NYV5lE=; b=0hDluo+0nRulDxz9eAhkPOe6ZioHUZXqjwIIlNLBXdFsfnFpelKUBNQCU4yEt9A0rWyc5f K64BTMpEGfNXRyBcbxLZUwTVZBeoHALbg6Lj+aIfeN5A8gGHEbdwc0iNiEa+7/K5ZOurOd byLuNtwR690bhL6/pH7IAZthWYY0rtI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718051282; a=rsa-sha256; cv=none; b=chNJREZKHenHIko61E2rx4NuFQX2pELAmW2W0A+1KQpN1j5uSfh7mNQ4/4+f4x7HiaFOsT 96J2qzF1QizIZG8qcLsf8gzz0PqcFwxoGwPg2f7dtCrlr1Rj+PnJrKras1sUakJZqscsFh +x22oMOfKBNb7Ys4F4xQ3N47Vswf2+A= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=G+pH6SJH; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.42 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-a6ef8bf500dso38380766b.0 for ; Mon, 10 Jun 2024 13:28:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718051281; x=1718656081; 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=CmaAu+iZsuEjEiEeynNp1R7+/jNy6Qzg+Lrp5NYV5lE=; b=G+pH6SJHMJCNoOprlo0bVIWDa0G9EmonWWGYh09Dzz4n18Z/aL+IQmEKYBFpmek4Ov Fg4sT/uyljM5k6t55YsMnXezdWBaLPV2EBBOwFBhYV0w3cN4/cw/0b9fNMst4GKaKYNX sNXjhnjBu4HQP2/ymocbqtVhseeiKF3qD+2rbr1dV76QHJyZW6nrZATO3KFr2mHXv0BZ hDs4ZOIWei3BREIqp/76WsF7ESI/PUbRVoZMH7LtjA6eqL9nfpLXN4qHXSxc5x99SEaW 18aD8iIR2FZtSA0bj+P89AIcrkbHX32jXfWDQFaCIcSiM+1pDY5kt1SdgKf1CJG3oo0Z 7Nyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718051281; x=1718656081; 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=CmaAu+iZsuEjEiEeynNp1R7+/jNy6Qzg+Lrp5NYV5lE=; b=GI54bLp26JzVR9BUqJoZXwL6toGg6HGgRGY6P0p9K8Rz+sWuaGCc8GZP4PxlEH400P vTmA2RN7TDKSvr5F4NX7ajtTQq4X59nuWSNoLNrBleQCTuXDdLE6K3EQj1RdSXbo/8CR 4xM7gygjoUQzfROAUEZuH8AzQyZyMHi0lHq0Vy6vRhohuNoT/QFWkbHmGd+6QFOZTDNH KqiCfF/ceoXYyGClnek4os9j5P5L99zebwkoZyc0FTRLIYMZeUI8TE1RC6azBdnrgbiI pW+dtTsJMcOwp4pwVOMHy3PAHKeDkGjckrIyLesUdlJTT9qGwcPnpMhXzWcrqWTD1+IP cO+Q== X-Forwarded-Encrypted: i=1; AJvYcCUPZjP5wLvmx1WJLvX6acCQCBmeM1OtYKyhc/0//5kStzk1XhVQR9dDEC9ibXLQXZ2zCGUtDXqJkliluxDSPJ67Wp8= X-Gm-Message-State: AOJu0YyNE+2apVhIuEHWe9rjl9tfW7/dR3VcPH6EIiFzlTLeflAW0chz 5/krfVAh/iD2zzVSU9BdeRW62iEyjkN9zF2PcvwI4JYa26mO5g4F2bIkVE5q8KJjUIZXmx4k9Jq qDpPELXMq7caIZK8ADjNPA6T8xtlWq0W5yCCI X-Google-Smtp-Source: AGHT+IGXSvDrAADj9Pf8Xu53OIXItT5TQK5ibDqDLkBUNlFNRj/gYFiO9Qxz2XUO3Ze11843IeDDNQqVlc+Qa3ui1pw= X-Received: by 2002:a17:906:89a:b0:a6f:1fc0:3fc0 with SMTP id a640c23a62f3a-a6f1fc04524mr225232566b.6.1718051280487; Mon, 10 Jun 2024 13:28:00 -0700 (PDT) MIME-Version: 1.0 References: <20240608155316.451600-1-flintglass@gmail.com> <20240608155316.451600-3-flintglass@gmail.com> In-Reply-To: <20240608155316.451600-3-flintglass@gmail.com> From: Yosry Ahmed Date: Mon, 10 Jun 2024 13:27:22 -0700 Message-ID: Subject: Re: [PATCH v1 2/3] mm: zswap: fix global shrinker error handling logic To: Takero Funaki Cc: Johannes Weiner , Nhat Pham , Chengming Zhou , Jonathan Corbet , Andrew Morton , Domenico Cerasuolo , linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 772608000D X-Rspam-User: X-Rspamd-Server: rspam08 X-Stat-Signature: 5h9q43xibk3h8n4hq5fj6ktfbsmzfz5m X-HE-Tag: 1718051282-797716 X-HE-Meta: U2FsdGVkX19+d5QKnsrBOeZ2EtPktYvt/hThZFcAk1s0vI/UyWEIzeSyEfyjSlvtfGuhAJzCCxgF2x9OiTnk1O7UsoF5UruzaNAnI1QhWVF3AghHw7EPpwo1j3RNmu3br/BaYB1PvZy+LUWscVi3JzuRprtURtzQenhxNmFc0Tgz8KygYMe6S2hUG/mJmTgXqGsB1VvTEjGiI532YsDdKlQj+t1v5R4+tcIPqQgyVqSjo/G+9NG9SQ0rwbyC21/Z7KtKixDBsSz33bCPdqn+/BrBC76xSPdAgR5il66l9L4vM0UpZLojHcSfVcGgt8TLZk9N6oquVtO3A5vKUzeOYlZTyWkyaxhCR3s8iZOEb9G86azpHo5E10sMs315oKt6+Eof9mevcpCgnTXL5ZwbPF+/PGSV7minvINLP9XwqXZure9I12braiec4ZnE5p4dgL/37epJkpaFtGzH5uYr0QM6V2nYO4iy3VAOg3PkMtSfiAL63Rha1dB06CUyxbGJj155IUHn0PDE/VTGHhxuNrJgQEwrUbWyYDTDMyPq2BLOZBqZwGsByBs9VoFpXVhGA8olhaV0QstZ1/SImiF3BmamzmlvwlFIoIp4hDaxjkr9zKaZjvFw6PpfMEmzrzbvdqlmcFrKBterEE/PgiJ/RBe1ok679dCv+yRCmaVaVLG6cDZoGGf/2XnxLsZsTMkppqqhmrlqXL8vvO6g2iP1YdBCYv7XRV/kRzVX2jrwQsNzJQKn6VU2itEoGE5nA1w+FN8NQudohIxSR8bWoYK2WCfPgr6SKhCQvNDxXURgCFWrHTmmFFpH7V1PVD19pu8jpJMBSgtxbJX8Ij736VaHEnZedOAOqbsWnhrCpPONaTsC6QmDvx+6fepnqrQZ4AoCk6iK2fQPiNI+Spb62qlbRbVBnexZZjxwApjzoOb1Er0JaS6ddxO96ZpzcSJUEZB+sOyVs+2JAEHcosnfO18 u/FUcahK N2e6/p5O5LE2ALspGH6aHik74+Z5yb6xP0cIivusNkytSZaU7Sh2BJBCLQBTT7jeenD+EAEEP/pSYxQAb6bvhFp3dKwYd9SEexOoy4Jdu/unQL7TaQTCJo4hoIpLe7YAQngv5rHlvzbTlFwzTApqfBf5++i5ixuNeBZYy 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 Sat, Jun 8, 2024 at 8:53=E2=80=AFAM Takero Funaki = wrote: > > This patch fixes zswap global shrinker that did not shrink zpool as > expected. > > The issue it addresses is that `shrink_worker()` did not distinguish > between unexpected errors and expected error codes that should be > skipped, such as when there is no stored page in a memcg. This led to > the shrinking process being aborted on the expected error codes. > > The shrinker should ignore these cases and skip to the next memcg. > However, skipping all memcgs presents another problem. To address this, > this patch tracks progress while walking the memcg tree and checks for > progress once the tree walk is completed. > > To handle the empty memcg case, the helper function `shrink_memcg()` is > modified to check if the memcg is empty and then return -ENOENT. > > Fixes: a65b0e7607cc ("zswap: make shrinking memcg-aware") > Signed-off-by: Takero Funaki > --- > mm/zswap.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index d720a42069b6..1a90f434f247 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1393,7 +1393,7 @@ static struct shrinker *zswap_alloc_shrinker(void) > > static int shrink_memcg(struct mem_cgroup *memcg) > { > - int nid, shrunk =3D 0; > + int nid, shrunk =3D 0, stored =3D 0; > > if (!mem_cgroup_zswap_writeback_enabled(memcg)) > return -EINVAL; > @@ -1408,9 +1408,16 @@ static int shrink_memcg(struct mem_cgroup *memcg) > for_each_node_state(nid, N_NORMAL_MEMORY) { > unsigned long nr_to_walk =3D 1; > > + if (!list_lru_count_one(&zswap_list_lru, nid, memcg)) > + continue; > + ++stored; > shrunk +=3D list_lru_walk_one(&zswap_list_lru, nid, memcg= , > &shrink_memcg_cb, NULL, &nr_t= o_walk); > } > + > + if (!stored) > + return -ENOENT; > + Can't we just check nr_to_walk here and return -ENOENT if it remains as 1? Something like: if (nr_to_walk) return -ENOENT; if (!shrunk) return -EAGAIN; return 0; > return shrunk ? 0 : -EAGAIN; > } > > @@ -1418,12 +1425,18 @@ static void shrink_worker(struct work_struct *w) > { > struct mem_cgroup *memcg =3D NULL; > struct mem_cgroup *next_memcg; > - int ret, failures =3D 0; > + int ret, failures =3D 0, progress; > unsigned long thr; > > /* Reclaim down to the accept threshold */ > thr =3D zswap_accept_thr_pages(); > > + /* > + * We might start from the last memcg. > + * That is not a failure. > + */ > + progress =3D 1; > + > /* global reclaim will select cgroup in a round-robin fashion. > * > * We save iteration cursor memcg into zswap_next_shrink, > @@ -1461,9 +1474,12 @@ static void shrink_worker(struct work_struct *w) > */ > if (!memcg) { > spin_unlock(&zswap_shrink_lock); > - if (++failures =3D=3D MAX_RECLAIM_RETRIES) > + > + /* tree walk completed but no progress */ > + if (!progress && ++failures =3D=3D MAX_RECLAIM_RE= TRIES) > break; It seems like we may keep iterating the entire hierarchy a lot of times as long as we are making any type of progress. This doesn't seem right. > > + progress =3D 0; > goto resched; > } > > @@ -1493,10 +1509,15 @@ static void shrink_worker(struct work_struct *w) > /* drop the extra reference */ > mem_cgroup_put(memcg); > > - if (ret =3D=3D -EINVAL) > - break; > + /* not a writeback candidate memcg */ > + if (ret =3D=3D -EINVAL || ret =3D=3D -ENOENT) > + continue; > + We should probably return -ENOENT for memcg with writeback disabled as well= . > if (ret && ++failures =3D=3D MAX_RECLAIM_RETRIES) > break; > + > + ++progress; > + /* reschedule as we performed some IO */ > resched: > cond_resched(); > } while (zswap_total_pages() > thr); > -- > 2.43.0 >