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 D1349C25B7C for ; Tue, 28 May 2024 15:11:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5AA846B00A5; Tue, 28 May 2024 11:11:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 558BB6B00A6; Tue, 28 May 2024 11:11:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 420686B00A7; Tue, 28 May 2024 11:11:40 -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 233D66B00A5 for ; Tue, 28 May 2024 11:11:40 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id CE7D21A1474 for ; Tue, 28 May 2024 15:11:39 +0000 (UTC) X-FDA: 82168143918.20.FB45A11 Received: from mail-oo1-f42.google.com (mail-oo1-f42.google.com [209.85.161.42]) by imf22.hostedemail.com (Postfix) with ESMTP id ED5D0C0027 for ; Tue, 28 May 2024 15:11:37 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=FpShbnrW; spf=pass (imf22.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.161.42 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716909098; a=rsa-sha256; cv=none; b=IngZKRAOWs3LRzIEEnGYi3L9Y4a5kfQkZ0SYX49D9mxOqN4h0fkqqcQID8jve2zhmlhAsJ qVvndCFA39jJGvS06r8RR+Nbt+mIupChmnGm8Oj4uyG9MyEzsjyEO/E+WX832JE+Hh07vS fyVLCIgqp1vsxjdFe/0M6v17EMQaKAw= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=FpShbnrW; spf=pass (imf22.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.161.42 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716909098; 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=jeEH13/IspqIN0QZgGiY2O/zNhcX61BWbHyoVtOdlDs=; b=nEeVqVjbUtQEpgu4twr7gYb26NCdLI6rGcK49iidDq+X7yYEZ6/sUpX41h8b5MO8r/RF9v gHYht9sxbVMc+p3oJYwdxgqmzQ0Jo7VoEtJrnNqeMv3fW5J0djQaZFKitPDqFFTDxAQnea KBC8YGhr83bM8BAbcK6s8dp4n7HF0ec= Received: by mail-oo1-f42.google.com with SMTP id 006d021491bc7-5b970a97e8eso581199eaf.1 for ; Tue, 28 May 2024 08:11:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716909097; x=1717513897; 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=jeEH13/IspqIN0QZgGiY2O/zNhcX61BWbHyoVtOdlDs=; b=FpShbnrWpMn3LIB12Wilos6gTEs1Aq0lJk6rzLiu7ee+F4lWoNxZ1R39mB06pyPbM5 za3xXnr+03sBiwl/gVRrKqY8qqSrxeCFZHYhpRmlMs2sy8K9/n/AbUOaMafnRzh7RyGP q+VwtJqWI/All7iJc2E/fV5Ld8PPPqXG+c0kgcFC2P5xMDHXAEvXRFssCR+JzXgw8Wse YJQ29FrWgllkrHoUwZJ5O46Fw28XVoLeIzHw7/2vclCbtayQZdFdl6k9n02/pD9tsjI/ YVzE++vMyrfn8Et2SyjVFff9Ji4GUU/cC+ZUjs6ZqYD9yiQFbevptLS3JAPLWz+xJRQ8 ULPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716909097; x=1717513897; 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=jeEH13/IspqIN0QZgGiY2O/zNhcX61BWbHyoVtOdlDs=; b=Ns20LkpXVxzlMJGNF/88I5AfZRAVjPNSWlnErLklRu40hoPdcUBOOMLVtI29qvrtEm 9eDcc5ZwwvvGINy7MkUAINWLKLEASjAatDizldJ2lgu6438Wue4qYo5mN6eFgyr6XHkN yjyh1+F1cF4JCKkMiFyRZugR9yPFFx/EU7T4lG7lFI9JxXmL/e7xEJ4uWWEjbM/v5ndB DwzFNAlryu4Jb3QDEmwsWMB81zuNHwyp+1w2PFMw28PSN1Tv560bMlJB/SwXna55GZQc dTjp9jChoc47wfWJs0tfKxc89Ck2tTlvQa33OD0tX0tYMj/Rp/KdxexmKVqY9aZikHga si2w== X-Forwarded-Encrypted: i=1; AJvYcCUlZ0qjZh2fzG5aHW2mUQhYvw1KF7AG/XeYRYJPjT7xj512WGPBR+lArqh44UuOLaV9MnSxQpByWHsfQJlXzSTWRFs= X-Gm-Message-State: AOJu0Yy3txdFR2mYcZHC00WphzkjQV3O+Cuc51FDMYB2wKvxDCMmxfHq 1P05PaHYALkw5SGE/QTzlGD6E9PK+Msp2tlhi8keUwOIabbNHbA+6PhIcJkOSYkj80gEwIHkeqH 3GagnyFEAR++bIKldcxjy13PduG4= X-Google-Smtp-Source: AGHT+IGCx7NWJ9+Jl2b5wiPsUNKoKYfa0J+bDBc8UFoMl8a0+PE+dUE+66EoIjeAUoZevVR8Ks3mh2G/I3New8Rk9l4= X-Received: by 2002:a05:6218:2615:b0:183:86c4:75d0 with SMTP id e5c5f4694b2df-197e50d264dmr965356655d.8.1716909096666; Tue, 28 May 2024 08:11:36 -0700 (PDT) MIME-Version: 1.0 References: <20240528043404.39327-2-flintglass@gmail.com> <20240528043404.39327-4-flintglass@gmail.com> In-Reply-To: <20240528043404.39327-4-flintglass@gmail.com> From: Nhat Pham Date: Tue, 28 May 2024 08:11:25 -0700 Message-ID: Subject: Re: [PATCH 2/3] mm: zswap: fix global shrinker error handling logic To: Takero Funaki 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-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: ED5D0C0027 X-Stat-Signature: gzrxqokteazpfaojgp6bsy6wjud3pihs X-HE-Tag: 1716909097-912196 X-HE-Meta: U2FsdGVkX1/9nFYJOkUbZyo59zB1D3lQmXKHJmOtkmX7mqj6PPWXgeNuzJJIeXaaOHjEVkYKwCgsyllkNCip23MQF4i6+OCO5yOksYNvG4ebhNlEROBBYATcwtXeqvtke00TyFFOWc/XE7yAxn9lbfwizSFPkJYTOzt4ORPjEbpCHbNWRbajMD/RegqwoAX+2dFsUNbPF96qJBdFMWglib9ZnkZQNsjo+D6+BYaDKKGREvtubL0vphKx3HcVNvOOWm9wevuv888iakVtd53pjtBSHTzeLe1kEwj4PvgMbhKJHYpXkvc1GShIXq9qGHHncqxmxu/MQkriOJC6WsF1hMFlP7/HMELji1EAF5kNVP1/+XpoKaq3GwsX+UOmY9XOZC3iPOSFRbp13S/uw2nDlC2ia9pmBOw6NnYdNHgAWstNB8MNRUt25sBdt25fFT5EQxybCYafyTKwWJNINIT8KyReD/YPIOnjP0ZIjRl72PHes5TE/HqUN6leQdk57X63yOcYj2cClkMz+LzqXWuHnnQ9UdiULpTrT3mJ9lr5QqI/7+ZRUaOxdtkGkOuw+80DUAh5cEjIlHh8aAgJYTLYWomMXUEV0y86zV1Hjn9Ug+l4ggE+5Iryvn0jOBBQEasZ2XJQdoFSdpBQAQy7S1SoJolOYPIhcPR7pd4VHblHRVHG0AJQReB+uLEee8oarow6fS9Wni/PqdLi7FQdHxuasHpqIcuZS45h676Jm4VX5beiS7BeLGpcI9IXKVovhfsL9NHuWJQMuJzDUbFCBsIEKnr4XNvUK4HZPSu9pg0Tg4U3qQqIjyLuXffojcT2SHAZechUGK1Rlqp/g6srNIAVfFoS+QlPRpVxvG80VuYKuBnj51KHa3L8FGCnJ1kbzLOMwwH/uWz9x/oM3e+P8+phGwEhXuGogjdyWYqA7Xjw8c71hgwORb0iqY+Vq8agmwpP5O6CKe2RoYBjH34kbFS qkrcWwsk SFZq60MeBqlVDYo+Uev1VX2D1DUn5r2ktg8Cnm0bM/cIL7iSy2xHeOP3R6RhYhqtybX4e3fsS/WWCTkFUNlXLcUo7AcvZtMoBx4JYTBsA9nQpbkDV1UkCGf4z7zrv+tsioseLAXF6RZn0Xx7EQNQNCctCv4etpk6KAfFSh8U8J73JcddCxW6waXCzg5hk8P5NzI5lolNJY715MaORU2nsTB3rkGU6i/DneYL6CMfGayn4P0P4gKiSBe34f42jrlmsHqAYucSUMEAz9scvH7uWk/avh1Bv9YdFZfoonIjh9wYnqTavD+Rcq/fvMb57umooNHSgfka2aviUsSsiFs9z6ErVUdDW30nNAQS0q2LUjm14aAKh+6y2YwOvhw== 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 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, memcg= , > &shrink_memcg_cb, NULL, &nr_t= o_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_RE= TRIES) > break; > > + progress =3D 0; > goto resched; > } > > @@ -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 >