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 43E96C3DA49 for ; Wed, 17 Jul 2024 02:40:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CF90A6B009B; Tue, 16 Jul 2024 22:40:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C82396B009C; Tue, 16 Jul 2024 22:40:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AFBDC6B009D; Tue, 16 Jul 2024 22:40:21 -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 8F1D86B009B for ; Tue, 16 Jul 2024 22:40:21 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 3477BC08AB for ; Wed, 17 Jul 2024 02:40:21 +0000 (UTC) X-FDA: 82347690642.19.84901E3 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by imf10.hostedemail.com (Postfix) with ESMTP id 4C9C6C0014 for ; Wed, 17 Jul 2024 02:40:19 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=d2w3U8pc; spf=pass (imf10.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.44 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=1721184000; 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=Zm3VZqqxmL9kr77/aMbZkRuLYRNgUU0cP/KPPl+urNY=; b=BHjayVb4Z/9uhhZH2Im3BDqdH9aF8rAXqnZv4ndaK4mtx3pEH9uVrY3xyl2UFItXsz9WTE Y38+mcAfgIlI8rzghOWe2hDoYzxWtSLDWobGdWJNXRe+/oivCvpWMQwSWPkFGG5f9bTXvC VKFdB8/ipw7j2TipXaJYLv/tpUvtdpA= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=d2w3U8pc; spf=pass (imf10.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721184000; a=rsa-sha256; cv=none; b=xwxb+XPNOXZdONZC49EUPMd16gJSMWM/aYi/w/U1lKAHaJLGLkIVo9OvRYobIgjYlJh/g6 b6ZYBbFzTBPfazIM+Qvm1nYWvm8ZVl3C9/RiWnMMgr4YtPdC/NDoYp1wVT8IpXIeFYFkXP /GU8ZSs6zKk20nWIEAVEJZ/nb1JmEEA= Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-36799a67d9cso4900147f8f.0 for ; Tue, 16 Jul 2024 19:40:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1721184018; x=1721788818; 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=Zm3VZqqxmL9kr77/aMbZkRuLYRNgUU0cP/KPPl+urNY=; b=d2w3U8pczEtswplsg3a/3C+r3XnqdAq80LPjbipHuqsGvUY9UhmTZcEfwK6Q16rTOp Hu00+aRy1SX8Dn/c6KbpHY4dj3b37nBoVAeJCwAsEYcwqIUT9ubhKLcVN6KjmC/WLMNo ZeOnGYZ6D6OSTGOEEZXWr01TmH+fwJWPA+QcAl0pq63SLXt7vyORmPjGQo+2aO9DRL+Z YpkBhkdgW5XJPFwn9jEmFua1KTPNyyolri5mGPdJIeG8A5o10IALhKQmPbzXL/Jih2D5 tA8qbUHbbdNf9QeDo/szH4/cbHhArxRpq91pOLe3ig4ytyth+l6w5z/g5MBF1uJ4tjNn xOmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721184018; x=1721788818; 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=Zm3VZqqxmL9kr77/aMbZkRuLYRNgUU0cP/KPPl+urNY=; b=d4dKC5QZJEPzKQn0nP6tNdNFBEY9FMl2lQQG8lVK2jw8hfQs13XV8f0dd5DcY90UHD eSofSbz6id6EeoOBb2lPnmo2abs+5Tz4h2ZCZ+4djdfmPVrITZAeuoYLR9CX2KMsB6Te zc2TcsOqMFguhFbVYoEceKQhcm95w/TfQVAu3RUd7KhWtTkBwPA29qgfU8VkR7yiZ0zO 8V3b2BC9efnz1rN2w0q0EqHXAxASnxDmWpOgvpkyTz+NRLRkHkCMOhxolk0nzQMTf+gy r/L6gxOPuwouYm9bqHGl30bZ3U4Oc744xpcCUb6a3QwpK3l02ykeSrQartJxJFlIRNDo BvGA== X-Forwarded-Encrypted: i=1; AJvYcCWAXcgOQk5wxqQiJqZpu1JtOfqYZ11L/c9tCvskPsowydD4C8Rq1xLqHoXMqg3pF8VnBRB9cIsTIzNGZskj55kEvp0= X-Gm-Message-State: AOJu0YzDD05fkgws0Oyi/fchJQ0hroKkLmYpYz747i6UGSjtThKpljFy 85msDLLKBUf6j2sIoNfrQAk+AL7IjW2CK33N8nVpjATjBd3gwtYAkwDs89S0EJk438USwd+H6H0 qythDaOqO7DomBee8firVfLYEMctr4Ecw+M5E/Qt48JCnwga6KlLl X-Google-Smtp-Source: AGHT+IGQJ3deHF+c7iwxQd/1wvM8XLYmCaUkv2t0Cl8krKimBCrVFIGo89cvJXhgULf848FrvUlMSsoQF2E/ZdHjGYQ= X-Received: by 2002:a05:6000:e87:b0:367:8a2f:a6db with SMTP id ffacd0b85a97d-3683176a8f0mr198641f8f.58.1721184017376; Tue, 16 Jul 2024 19:40:17 -0700 (PDT) MIME-Version: 1.0 References: <20240706022523.1104080-1-flintglass@gmail.com> <20240706022523.1104080-3-flintglass@gmail.com> In-Reply-To: <20240706022523.1104080-3-flintglass@gmail.com> From: Yosry Ahmed Date: Tue, 16 Jul 2024 19:39:39 -0700 Message-ID: Subject: Re: [PATCH v2 2/6] 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-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 4C9C6C0014 X-Stat-Signature: zy93dk5myc61imm64en9cbexoyet3xb1 X-HE-Tag: 1721184019-2891 X-HE-Meta: U2FsdGVkX19rsLPk2IwnpzPg7idL3oLe6LoSIm6o3r4Ez5ZAPWr7IfX9g0rHXcXqFzvobJxqzlkegnl4u/YxAaUl6qh0NWHQcB4bTP0F6d4NjPRytuVxT6m1JNlhRshl16SxypWDEOs1jqURXugK6FcJbztkam859ivR9F72WzM0nDqXQcVqAxoJk8xVpf29rYwsQ7b5C8kvKhDNjj7S0v8T6kxlAfYjpzGiPxSPAOiyW9BEmwmeif/WTCLm9qrnwnUmV8G/L3P0fdJNeqY193bBg5afE/5E3isZuQOpJGN8gi96mCFj6+udNHtpCjSI0Fz6LIBiqoHPXTBjK50M4Wvnfli2+wLs5QlSSf2WVR9ANFI0S0ahxy2/UXMSP6Wu0/dsYLcQM+zeasK/ndiC90bpSyDU6CEd7Da1XmbpfdpIXbqxd7+Ugg1sO010puHNBIKLWJMFtmakkmOtZnfNktXxFesDALkXKJQAjV0snBpkAEAHIm7z0rP19UEOUKpOdPrDED9Ngid/jduEdjYBCDZMNAUaZSX3Qkn2JPjqP97TYhONPPRTKB8MK2M3OoNZu06L0fnHeOudUXFUUbeYVyDZO5Dh0N8WQBFNntNDbqAntkBt7CnwGcv+xcr/bQmEz63ZrT7hnxCT5pQpWYCdi9cu8QkPfm6VLENAJXfUSBFnl2S1raq3sfvTJIwie8vv9Eg5frX7bAHE1na/gLEOL2393N+GP0bDZrRKwj2KLxJXvzwa8UQjViElOPP1TSAXLPAqRBFMHc9tF1JMPOvqvdgK1Xu0DVqHNf05HtM3syKa2hRkVUZim3cp0APf9mcZXAT5uGetLBspf8GWWJTf8bkwLk4UBStsWuO9aaQ8z50Y2wDed3Ebr0MvwuQK5B6hYuMlpxYJnKYz7ay0MCm4UOTCLTIbRqqMe62asIfXbmJuN+RWv5VKyEdc/tuoWIAprET7eHVUHaFjwcfumZE FXHCkqVh dT+t7Dn4P7s3g6skve+AXHADZ4NuqK3UN6SWyjg0k4hyS3872ltBQtZnEgGe3ypXp5zieNEcz9fp83h0M3StMigIS3lTus0L1+J4appK4LI84u0IdZW1KJLLiRHkNhF2YlGaq08PaoHX6cgdWQxpS1jpy0VhGpQJPHewhOKZzFxIcetH/s60IsBGIBXo9Yhr+YRk+ 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 Fri, Jul 5, 2024 at 7:25=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, 25 insertions(+), 6 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 29944d8145af..f092932e652b 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1317,10 +1317,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 > @@ -1334,19 +1334,30 @@ 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; > 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; > + I think this is unneeded complexity imo. Doing one less retry if we happen to start at the last memcg should be fine. > /* global reclaim will select cgroup in a round-robin fashion. > * > * We save iteration cursor memcg into zswap_next_shrink, > @@ -1390,9 +1401,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; > } > > @@ -1407,10 +1421,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 */ Unneeded comment. > + if (ret =3D=3D -ENOENT) > + continue; > + > if (ret && ++failures =3D=3D MAX_RECLAIM_RETRIES) > break; > + > + ++progress; > + /* reschedule as we performed some IO */ Unneeded comment. > resched: > cond_resched(); > } while (zswap_total_pages() > thr); > -- > 2.43.0 >