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 E4197C3DA5D for ; Mon, 22 Jul 2024 21:51:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BE796B0083; Mon, 22 Jul 2024 17:51:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 36D7D6B0088; Mon, 22 Jul 2024 17:51:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20DFC6B0089; Mon, 22 Jul 2024 17:51:31 -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 0338F6B0083 for ; Mon, 22 Jul 2024 17:51:30 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9B5BC41A7F for ; Mon, 22 Jul 2024 21:51:30 +0000 (UTC) X-FDA: 82368735540.21.7CF19C9 Received: from mail-yw1-f173.google.com (mail-yw1-f173.google.com [209.85.128.173]) by imf14.hostedemail.com (Postfix) with ESMTP id D1484100002 for ; Mon, 22 Jul 2024 21:51:28 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AD7gT1Hy; spf=pass (imf14.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.128.173 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=1721685044; 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=z1AyKNnzGAkcTki8rPtv93FNMTvw546VYOSBbwMVqlA=; b=NtZ9MmB+1TS6aaG+oVeVSD93JENlYG1flbBBf3fhL5j9uLUK3PiVWDBqAL8f6IdjYuEkGs FEXflq6dGvWi83FucH79h2W3L6O55c4HqfHrov/Qnjh5k1Xr6G7mCWzhXCo1K3rJCSsw3Z CsQVQq6ROAOXQUTU7OGpHutfeUlBGyw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721685044; a=rsa-sha256; cv=none; b=BmqKoWjXNo2TSUKy/lRMIEOGt8mqbZXnM1z3Vvo+jT3EGApxXcXCZbGTk8HtVpGKOb3N4b Tnu91iVCtdHu45t0kzA19jqQ1k5caj1nruOaq5+/HOwQ5OYi+BEDcTaB9bKF7t0xQQ8ST0 3Anlm00QBaYG/OTKjuY3PZcTqpLoErg= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AD7gT1Hy; spf=pass (imf14.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.128.173 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-yw1-f173.google.com with SMTP id 00721157ae682-66599ca3470so49202717b3.2 for ; Mon, 22 Jul 2024 14:51:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721685088; x=1722289888; 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=z1AyKNnzGAkcTki8rPtv93FNMTvw546VYOSBbwMVqlA=; b=AD7gT1Hy2xo9WQzwpD0LZU/KPOfsv7YipDcpwNYSEKIVJokAjbCTDLxpSf1Teiik+0 lGuEB7zBXpzanJD04yax4fewphQq8/ErlAj9pAouQr+hOfjwQudTsi8iP4BJgfA/+NfS if9ohrfhL1fEMUrSykdVv+2/dakubzRzyr7+vAHMsCRe8K8JWpsa2iP3+jd47BgO31ga 3HiIfPMHfyAALnKdERmuMMJJnbcf0Ub46I7NMLDfzHUDT46u3JXT16gDrPWQqR89D9t5 CMktpsoZ/8a8EkyYVmGvayOzC7vqf/bcO584mSCb66lEnGqrhaeKIuiGUoKiU0oXvEOP YwvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721685088; x=1722289888; 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=z1AyKNnzGAkcTki8rPtv93FNMTvw546VYOSBbwMVqlA=; b=EHnSv20jc01XAyNzrlFt2DUwT5qlc+9D8bj6hqGx2y8k+ng8LzSgoNV3gPuW7CYcOQ 92fgdX4K7qg4nu/W8es8jizs6Ygk8WbsCZJlR24IoPX1H1IK0iRz48tv0vP/TyXVZzfP QPpLPvmEAcMbnKDOF9aV0JIwr/AzqkrwqBWf/jWWj4h3JhRv2jVxbLhhimdfmn363sLg 1W/pDQru/rf97zlRGpbV/CUOQY7Gfz3XuHwzcTbW8BpMrpurmhvROXurLAFAQw4z+0WZ ta1ox4XuvPr8lhc5LW0m51fZtjrvijxvO0HY40V+e5K7UYEALskH0rP+bLjx+z2DdxN9 wE5Q== X-Forwarded-Encrypted: i=1; AJvYcCUVImcSST7JIsRtK2Rd7iwh5rVsX5jxTzOQ01D1tezFnRxBdz3eSB/NL+eOQPrjmqz0UsEm7UM0y52WH7CTQIlhNRk= X-Gm-Message-State: AOJu0YzUGa4vrkYwVtnTMlOopB8mSTCB4HMLq9742aGyyi7r/NthVbXm UbisLBresnYzmsQmRETi48JgLzYnLDkVxPTD0V080x/mHgrqK5Anx5uk4R9u34ufLyihUX+kzMW Ogv8eeqr8jQpr+0nALNEFgueMbqA= X-Google-Smtp-Source: AGHT+IHB2RGwQBbyl5TKS5qq8ko4uSod5+wC0Mfz4fQZedFcwBKtXANszbYFgszowCQwwQe5uFGR3n45q6BwPNpa+k4= X-Received: by 2002:a05:690c:3249:b0:63b:d242:4fa0 with SMTP id 00721157ae682-66a67104a21mr101512367b3.21.1721685087744; Mon, 22 Jul 2024 14:51:27 -0700 (PDT) MIME-Version: 1.0 References: <20240720044127.508042-1-flintglass@gmail.com> <20240720044127.508042-3-flintglass@gmail.com> In-Reply-To: <20240720044127.508042-3-flintglass@gmail.com> From: Nhat Pham Date: Mon, 22 Jul 2024 14:51:14 -0700 Message-ID: Subject: Re: [PATCH v3 2/2] mm: zswap: fix global shrinker error handling logic To: Takero Funaki Cc: Johannes Weiner , Yosry Ahmed , Chengming Zhou , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D1484100002 X-Stat-Signature: qgn3c58mbfubrow5p89x8fc1ushm4org X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1721685088-101030 X-HE-Meta: U2FsdGVkX188r4FFOqughPDRfOqlfes7sCCpqR7uD5Qt5T0ZGvYaNk27QgC6mnQTDSbvHh6KmNR5GJSxsakpsM/qjPr0AawyysW9JgG8hIeqjeFyh0dc0H3vLyl0GVNmTX//vvmeuT/L14BS9ogYVt6ZE1qIRZb0DYYSe6ftfEojSmWY6ljndQbUP29xZMIZl2KjZ+9qLDNUMPHQa3ai+/yAaVGZqN11IONs+JzHbzxWql6VSC3fAuuwCSlBfIMzRItcBDsINzVLdjFA/jD9OSdWHUCrJvGN/ITB+PdSy93slBEfiiSWO0q+80VkpFHrBhYJW+BXOo65JbcjKnW9UbKeG9n9EdhD+BczzxGVz8vdwE97wt0otlfWSSOLpTRs2ZpE/TWtSKvDfiIq9JhU9DyY83UD/w3+bjIa0dC8BsZHnnU+DuufX2AH08a8V9mKKsyUKGTG79sKQ8jaPpca7tof6hjteMUWmr/UgW2PQjj182+whg0T0yF1hNJfCHCHTxe6PppYF5IIrRy90Y3rBmKDgkzewKhvYcqrzZyPzdV/VAdwZ1z0RUcHAlio4Vh7Va8Fg7vo6fVrOLF1D3byj/h4SgI96upnFGZzAD4dqd65pFy+ta8uGJjYXjq+I42G1IYaPuHnRmuysN9YRwjrtvPAZgxzAqzTDhW3kWwi9zXGWTcskt+B6SABzuR7iJzK4QScX/zFscb9aPvGAKnfvRy2e7sWAAhuIriVsF4doJgq4U7WRuBUXNl2iYTCK0xtgf9j6Qd4rs5TQeEplWrwVMRCSnPINWmLIZkow/jsYoHB8qw47sWrUkRBxtyLpcQSAi5dmOCRwAwVevM0Q7Mp0+p5dbFHGf1j0HvWxkCTisgzDQeIY54KkUknvYpAd4tZ6+ru1wOS6bTTwQaz52uGV5m5/Gd51pdkPanBWdFkSkLXJn9OUiBwCYOPzyKfrIC7zvbdXhklDdckz70goM7 ETmcDiTL g0H8gtO39KGJRdjKGzSLMTMN+8U3ZdaILEa/4qqx/f7+vNYjX8x+rR1qy52uHp44dO2C7DmdV/7S7FcIiR6u8/UgtkoD+SVg8rwenpeza+TAT0/M+MVXLGqUN6y1a4G8CEdSX6Yfva6IUXoKaVKfT8dol1e4gETwitcws05M1PJ4AmU8LE9V6N3xY+CaeKdLE0sIlguUpZ/eIqiHJpQQfquJkoIOPxq+VJX6BJ12W0HCnqBKwYey0zb/YEar2mxUzSrY9/FeIMdKmhUtR7euJzN/2tTx5egf9knPSzO6YxwttwKglpUZeDeibNWt9gFhIsfNB X-Bogosity: Ham, tests=bogofilter, spamicity=0.000027, 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 Fri, Jul 19, 2024 at 9:41=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 code itself seems reasonable to me, but may I ask you to document (as a comment) all the expected v.s unexpected cases? i.e when do we increment (or not increment) the failure counter? My understanding is, we only increment the failure counter if we fail to reclaim from a selected memcg that is non-empty and writeback-enabled, or if we go a full tree walk without making any progress. Is this correct? > > 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 | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 6528668c9af3..053d5be81d9a 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1310,10 +1310,10 @@ 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, scanned =3D 0; > > if (!mem_cgroup_zswap_writeback_enabled(memcg)) > - return -EINVAL; > + return -ENOENT; > > /* > * Skip zombies because their LRUs are reparented and we would be > @@ -1327,14 +1327,19 @@ static int shrink_memcg(struct mem_cgroup *memcg) > > shrunk +=3D list_lru_walk_one(&zswap_list_lru, nid, memcg= , > &shrink_memcg_cb, NULL, &nr_t= o_walk); > + scanned +=3D 1 - nr_to_walk; > } > + > + if (!scanned) > + return -ENOENT; > + > return shrunk ? 0 : -EAGAIN; > } > > static void shrink_worker(struct work_struct *w) > { > struct mem_cgroup *memcg; > - int ret, failures =3D 0; > + int ret, failures =3D 0, progress =3D 0; > unsigned long thr; > > /* Reclaim down to the accept threshold */ > @@ -1379,9 +1384,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; > } > > @@ -1396,10 +1404,13 @@ static void shrink_worker(struct work_struct *w) > /* drop the extra reference */ > mem_cgroup_put(memcg); > > - if (ret =3D=3D -EINVAL) > - break; > + if (ret =3D=3D -ENOENT) > + continue; > + > if (ret && ++failures =3D=3D MAX_RECLAIM_RETRIES) > break; > + > + ++progress; > resched: > cond_resched(); > } while (zswap_total_pages() > thr); > -- > 2.43.0 >