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 3F657C27C43 for ; Wed, 29 May 2024 13:00:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 621696B009A; Wed, 29 May 2024 09:00:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 603636B009C; Wed, 29 May 2024 09:00:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 498AA6B009D; Wed, 29 May 2024 09:00: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 2E4816B009A for ; Wed, 29 May 2024 09:00:18 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 8D0C0160CA3 for ; Wed, 29 May 2024 13:00:16 +0000 (UTC) X-FDA: 82171441632.22.84FB43E Received: from mail-yw1-f180.google.com (mail-yw1-f180.google.com [209.85.128.180]) by imf15.hostedemail.com (Postfix) with ESMTP id ACB02A000B for ; Wed, 29 May 2024 13:00:14 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=UzOHY4ir; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf15.hostedemail.com: domain of flintglass@gmail.com designates 209.85.128.180 as permitted sender) smtp.mailfrom=flintglass@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716987614; 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=/8xSjHIJtxOZ2a/v+doe+8LgtILr0Bo4UnJUupQYDU8=; b=V67WM8lJohni8Qfz5AGyCVpgp6OnT+qk5Bgo5IHdFfnSyh81qt5B90KZvAwa8n8mo16CeO 2sM+cAymiv2DUZwPWTvWiA4M5lJ3OQ2kPDfNI/3JHxmVYJ3uZuAWwyCb+blNDE0lNqWvCA ql2UwUhqqfh7QuYp+zCx487qLn4lccw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716987614; a=rsa-sha256; cv=none; b=TFUdUZFF7qDQN33Ycce2twEiC40txHm+3GI5Yy6Q3Wis0VZqPdP+OSnmaehzXD2G431ZYI VtzMBv5KsjGpJiZFfyKbGSXIzk54Wkt1iHL9CNBTitlTRdl5Wrd2jUWlheB6+xSU8qcd5u MTj4ar1lkkA1Ey0tua/aL0HPR6jRPE0= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=UzOHY4ir; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf15.hostedemail.com: domain of flintglass@gmail.com designates 209.85.128.180 as permitted sender) smtp.mailfrom=flintglass@gmail.com Received: by mail-yw1-f180.google.com with SMTP id 00721157ae682-62a0849f8e5so19626147b3.2 for ; Wed, 29 May 2024 06:00:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716987614; x=1717592414; 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=/8xSjHIJtxOZ2a/v+doe+8LgtILr0Bo4UnJUupQYDU8=; b=UzOHY4ir934sZYk8L8uT8Sy0Ogsu2ZtNFYLb65itdXF3JtyN3eI2sTQIua9r8rO1ll RE4i+4ckKEdG26roY1lJXLpo1V6glLeXLmSjnyKFaWwAAxaFkN+FeSUQHyQI2K+a0nXQ jDbkScfYc+NaW17/1072z9ZAzAIYEzWjKDxLgwlFoxJKDtrXa9sdmkO3PatZngRDwxhb dAvLZ4SJNhnETWJb0oQ6Sxnt+pCo/dhCWvVvwbsBAYto76c5uzevcFhdWWGXsi2gIB2I jSuavVOZ52dJ+AdmeDumkLAsH3SSeUoloNIxktzqUDebVXYv4gcZHuCrCVI5Zs5sl1Rg wI4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716987614; x=1717592414; 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=/8xSjHIJtxOZ2a/v+doe+8LgtILr0Bo4UnJUupQYDU8=; b=JPF45IPsvvrNvVekPyplulEYI6fd7TPJIjX3BRkrj428rG4NHs9TKuhuZe3FoLAz14 IQodW4Z9byjaeMS2CYOdrCEf+gMNzfrjLWlQBHEUkmiw2g/XXWSN7/XJ5cIVP65pzKZo gq/rBhMQRjmatP/mPPf5dT4+/hdpKIFFQ9JLke8fFvHMHqLuHW7FBiYtZUSv8QdVyTFi XRrl2p99hoYDKSQ332tPFRCm1zk4OyJAqSd0T0HszpM2ixKXmpAaVkmWxYMAZUsbTsoZ VwxDB+QF5GN1GLC+qJJGkRtb/P+cMAmZrlZAcbjovAS9o30rF3/wmbqXFvZhaiA/UyxU f1Eg== X-Forwarded-Encrypted: i=1; AJvYcCVAa6lE/u3LJ2bw16WIj/T7r+QyBa2JWTL6RWIuyEYtI0XSuEBI8q4QvEYXsJHMKCjbKoJ2XSXKBjEgSUPhcXW9yf4= X-Gm-Message-State: AOJu0YwgaboxBs/JhWrlAmETep3sU5YrQExiUbUQoL8PxtIzC8v6y+1T WUG7reIzl7MUARBamUvNop17Ql4o4wOb6YNOsLZTDprMbeZu/UUtp7rhzK/dOU+2zuCd3NUryx0 saMg8cumO59NJGPmPbQ1gUbIyysY= X-Google-Smtp-Source: AGHT+IHec4C5468aehPFSf+H+IRsi2KOo7M1H9KdehWrWGG2japGsiZWWYbaXE8DwyOOFQCDUWY+syODWJolEyUTa3k= X-Received: by 2002:a81:6cd6:0:b0:61b:6757:a3df with SMTP id 00721157ae682-62a08eb4a6emr145503807b3.33.1716987612935; Wed, 29 May 2024 06:00:12 -0700 (PDT) MIME-Version: 1.0 References: <20240528043404.39327-2-flintglass@gmail.com> <20240528043404.39327-4-flintglass@gmail.com> In-Reply-To: From: Takero Funaki Date: Wed, 29 May 2024 22:00:02 +0900 Message-ID: Subject: Re: [PATCH 2/3] mm: zswap: fix global shrinker error handling logic To: Nhat Pham Cc: Johannes Weiner , Yosry Ahmed , 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-Stat-Signature: 5eyqf8fexjuzo3d11n4yos8kpx6wp1fd X-Rspamd-Queue-Id: ACB02A000B X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1716987614-815825 X-HE-Meta: U2FsdGVkX18Y1LU53kfv/A0QzX7nYqP82yqKLxLRbQKZCHcBvoeKwSOl/oRbBrmxImetxtAZEkDVi1ETX4hb/RXP5O0hEicz+8+a6rwjNVRrY+IHrhsJdCKHLz0YICSzH0UChW7rZ+QyB5g5UmFjXtP5PKnb4ddc/L77HxVbEbVuu/oaIRbKyuQm/FkZNOubAm3rkoIUmnDzPhoMyScbzxpkEd8b8pBGvcWO+x3zO9i9FnstIjwEebFGvbVvYIQb6so7ybFdosTdTz+U1oIsN3sz3Qgp6j3fkIVluAM0Jk9JqQpTAAY75iyvXgtXDLJC21t1N+CLfrA0MlaDIjfM31y8YoA0H0WWDqLFoKHxwBveahSoUTQhqcWmOibH3kTU5Lg60IwU1NMvNl8E3sct76YJ/bfDF9fPoOCcbXIeWbnDRwxWysj3i/pBbhk2jL1mCdRCN0qjn0G7QFQdpq2X4NZHGLVFjvBX90IF6szYFFLlWBWwkxXp0VILUHK97j4jOdSw8GyXQxKq8NY8XYiI10Sh6kAkAGBZchgtRO7IgU/uv1/SfkT6/QxFh8rIZTRIqnCscw7KWApz2MSZJF04J5ZP9lKMFEfLnbl2i/uyDNGLNQC74MHTAN4bdvp9kOslX1jTc4yCmq8+QhHPh57bFpU/DDxsgSlmwYkzLT5k+D8vR/wtFEh/MPpI8GufIdNJrvHgPB6pJh9TXwwmWsEC3YIkK+qNGFtDPy0EHCM13kjjDhZpzIK4T1A9DBjOQXnu9ve/GIbJptYsVLXOgvTYNrP2Tcl57nfBmfR6yrVgoIKZSRYYPks5djZH5iBKgI+OQaH7vooWAn2ySsv5HfFapJBJNPIvI+NzKqYdPmuZPjsY4deQsJULKM0fdSFt1DQ0pZhuN0UfkXXoXM7AHBwFr1utKOAQKb/EAjHE/rSO/vTGD991zKzUJn0eb+MLmHKIG/oMx8uxn8Pioj1zaNz 71skHxKi mv63T6R6KWpUlq+I7+Y/7k9SmSJczkKFpUA3ktdZgW5Ugd6ot3kf5+9j1q8vFljQEyhnRXgwdZDmA495tEdYbNaTfarF4mOtj7Wi1vKcgu66+i2yEkh7qtq+8/iA+/Rj/pskVhtCElj/MD79VhXIqqXgc/X/7d8RoWg91WueQH9KAjURCDFPOYi3QCdMrO9jGhU7UC8lx9/SZZVyGG7G64GEtD2mtNCFpQkj+y5W5NyX7d5nR8NL6Eh2e177zjSA3B4n0rqvdKYfuCKBCBXy2vjBEr5MLXOTk0jdN0+kJae6hV9t8LrV4cP+rWYlzqH4NQoPTm1gdXikeNGnbhdNBpeyFegQDy9noq8RGhoM0x9WpgvfmBctMq73Ktn/GlFHRolrpmoVIMK2S18VAreRZ5xfiTSATGqFkodkd 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: 2024=E5=B9=B45=E6=9C=8829=E6=97=A5(=E6=B0=B4) 0:11 Nhat Pham : > > On Mon, May 27, 2024 at 9:34=E2=80=AFPM 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 0b1052cee36c..08a6f5a6bf62 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -1304,7 +1304,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; > > @@ -1319,9 +1319,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, mem= cg, > > &shrink_memcg_cb, NULL, &nr= _to_walk); > > } > > + > > + if (!stored) > > + return -ENOENT; > > + > > return shrunk ? 0 : -EAGAIN; > > } > > > > @@ -1329,12 +1336,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, > > @@ -1366,9 +1379,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_= RETRIES) > > break; > > > > + progress =3D 0; > > goto resched; > > } Here, the `progress` counter tracks how many memcgs successfully evict a page in a tree walking. (not per the while loop) then reset to 0. progress > 0 ensures there is progress. If we visit the tree root (NULL) without any progress, it will be a failure= . Before the loop starts, progress counter is initialized to 1 because the first tree walk might not iterate all the memcgs, e.g. the previous worker was terminated at the very last memcg. > > > > @@ -1391,10 +1407,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; > > + > > Can we get into an infinite loop or a really long running loops here > if all memcgs have their writeback disabled? > > > 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 > > --=20