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 AA38FC7EE23 for ; Fri, 26 May 2023 18:16:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0D86F900003; Fri, 26 May 2023 14:16:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 088FB900002; Fri, 26 May 2023 14:16:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E6B3A900003; Fri, 26 May 2023 14:16:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id D3657900002 for ; Fri, 26 May 2023 14:16:11 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 865BB401C2 for ; Fri, 26 May 2023 18:16:11 +0000 (UTC) X-FDA: 80833210542.13.72C9846 Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by imf24.hostedemail.com (Postfix) with ESMTP id 8CF2118001F for ; Fri, 26 May 2023 18:16:09 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=DOeSLj3z; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685124969; 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=ck6N4S4jiJu910yJXpyaQokUMWH4KKhSAsPW4EeI/fE=; b=2AiN97c2eeM3RnBK9+hn/8ALulo1OOJ6bW3PO3wojhNQhSGKrti1bdoQYRmxqBJ80Bfr/T YgXiwUpQRAxKq11ctzHTKEq5JmF75ylFjQlUsdZbox+sWnEE70J52k+Kux/mJE6/PZgZQJ 23DESoDSHh3cxo6Q7kBXNxWE1E7DaWw= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=DOeSLj3z; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685124969; a=rsa-sha256; cv=none; b=nraniQyWrSIrv9MbirFXRu2o0LoaWsYPMnHbXK42U8WYJzKxvEVbKBTD1/ApTI/RfiA9D3 LcZfdy9YG8jEBFFshzXIc4SEfUleaNLpseplHpPznzI6xy+i6qoPIPQkIXxtPucd92Zhg5 n+HGRNJhBdhqGexOrVdrmYDhkpXm7rE= Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-96f8d485ef3so166750066b.0 for ; Fri, 26 May 2023 11:16:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1685124968; x=1687716968; 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=ck6N4S4jiJu910yJXpyaQokUMWH4KKhSAsPW4EeI/fE=; b=DOeSLj3zYkASws6boClAYXGxuNjRdIm5LlmNF2AU7fqwyEzricfQWmZ4ylo+VNtTpv S7MnIj0t2qEqtkI+/722GXPNTAztrMPTUmFVfVF2on2cEClEIbSe1IDeUZXgEzEbYGDN jqIRkPlZksHS3N8NJ0yKrR6PiNbaNUItKiug58U6BHtmuMCOUllH1EED7zFCvKS/bnay Ocpwte5g7kWUpuikxYmgw4gEf7h6GJSWAJzybDwF0ZtYx2gPmjDhMaPfg5jSNMpcLLt0 InVE5bcMjqY1LH0s5tmA9uNXTy2qcAv7Zv9C3D4o5lQfz8vyDmBr45kEJBQ3csap2QMc 8J8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685124968; x=1687716968; 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=ck6N4S4jiJu910yJXpyaQokUMWH4KKhSAsPW4EeI/fE=; b=dfRuHgyTejcSgqZMEbKp3rgUuqRNz9WnquN3voPdh2aGX3456+c8uR4JgYC8Bf+E8i cmsYGijcomh/bmE51C2V4OpJDcs/Vyc20xMYyzS3OD6JRtKCSVf2VJ5m4AQb5VrFDUJG zU40luKQL/SQi3aK+y6vAqu4MJ02ODoAyHzPj76eaEcR/ARUwRUFxuASQsoVL3vm5Fk6 EVK0oW5tjtY6HzzYqlG9bFpAJpx7OZ4QeqgogqKOEyAdrgjO3rjIoTlFQAtkewKuDYNd V+3GJr29ydQPfd6naFgKh0X4h2jJnGRBKPy7/lnBc8sCQU9tP0GLmCCJlnFgHHavHp1Z qzjA== X-Gm-Message-State: AC+VfDxrSW7+uwOH0NxRIMMh2sLOpIWFGuid8XtbLAEYwdmL8D93UuGm Zu/U2//Nhawtf8cFGQUb5QG24QXbT4+j0KRvpxm85g== X-Google-Smtp-Source: ACHHUZ7bmJdZ/B7moM+bwuYRIKTMF0iIacS+JLxF+JNTLcpYnFSA1Zr3ohMkyz3HEFHj+DT9bNeVQpL5IkW7ZHx2CL0= X-Received: by 2002:a17:906:db0c:b0:96a:37af:ff37 with SMTP id xj12-20020a170906db0c00b0096a37afff37mr2666854ejb.15.1685124967602; Fri, 26 May 2023 11:16:07 -0700 (PDT) MIME-Version: 1.0 References: <20230526173955.781115-1-cerasuolodomenico@gmail.com> <20230526181023.GA49039@cmpxchg.org> In-Reply-To: <20230526181023.GA49039@cmpxchg.org> From: Yosry Ahmed Date: Fri, 26 May 2023 11:15:31 -0700 Message-ID: Subject: Re: [PATCH v2] mm: zswap: shrink until can accept To: Johannes Weiner Cc: Domenico Cerasuolo , linux-mm@kvack.org, linux-kernel@vger.kernel.org, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, kernel-team@fb.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 8CF2118001F X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 96a8485yiabogh6douztoj36ua3m3o9r X-HE-Tag: 1685124969-575431 X-HE-Meta: U2FsdGVkX18uNWG0wCjoj/fkyQgY2f4cOafWCoxjVZ/TsOnQIlT8wJ2WvQkT5SI4hyBbGS3cpK0n+JhQp9aO6rDVOYminP/hH9BlcwejmHwon63mezCFVfDb4/Az1t9ubvM875K0elM3ayqsN4+A95slXqldhhoJtwzgriysdEuwiTjIN/OI1QE7CjiDW/WolHR+iPo0q2wEL+QU76kPTFQ9QWd/Vc/3GQRYapJubEbAt8z1t0svfT+UVxx9/0/khdvgT5ZFXn0eKVRa1dafGl/BQsPBpCI9AxDUtWVWt+pPyJ/bBIMAfyPOyOuqr6iNnWLFnGL34wddGUjRe6ZDOjHGc8KBXRMSlYGIFPN6D9ZBC5mgD8x4JkK62awVYFTeggUPP7zH8r2fne2O7/5feSHq3QqLq9L5X4romqV+PP1NJr1A3p5xyLFnxmv1G5ffq3oQk59ujCSQOX6GIv3D3t46M+PbCP+NXI4C2SjRjhHp/sUQJ3k/x0jnWVh2UZY0SoZNwZrn3IFBR5tT+cSKKr7IYUVmVFf0MXzJ6QUZk+fvYekWRbfRJ2FYFGmS9sHcceZY5GtQvWzei2w+kLHpvLM4CSS4G80XyQBdArn7KuJgFPpTxNCSpxi8Njj5crogTh6Lybx9FR0X5eb4ADecDC3jLoQz+RnDEbx1YxNNg2zQN7zGCryP1Amm4x4hdv6loiC1bTeuadtLAv2QFKvxLQYqqzz/lgFKRhs78MOwOL7vt1EEsSC4XVh5PMKvKU73sR04/oWTl+4e5se9FTocEXPGc0WGpl58TZgJvI1I6pMlq/N1Lejsk5WHgq4yDER8bPPV9t53iwSwxsTozya0+3AdpFywK6dllpWl7imHEppUEJvF0TS2Cqof/5B2lsFN6myym9Pbdxz2RHAH55KIbkpWyFRGCmUmD9GD3DCvOEvJ/Gu/5bhh86Omwnxgr1QqlSnLoZYuTDqZJJ+0vHN TP9ZqrM/ wIwKEPUWCr5WVeI22Ew8ZMc0Fz1SQKGE3ToVz5gVOMj+pfnbhHZVHJB6lStOba2ePtgCagUlqPVA6Sj4VxpHsbflWr558AoycS91p3XwT3dBG/pC+LK1n6BvCR+Ru5Ni3FOyDpE7/5iXey64WSdMd7xLwwTvXbFWadOBMpQ3ZPCs7duugw82k+JmRuBYlZ9Xc+0bvGXDDW88ZQ76J8syezbuQu13NCnECrRnMHOVn8umAJ+lrkpAKBTQBE7ZyGXlNFtsTNiF/wCzCK1Drz1eZyu9POgr26ma3KQGrU7BCkaFWkmcO5XNAh8uQjQ== 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: On Fri, May 26, 2023 at 11:10=E2=80=AFAM Johannes Weiner wrote: > > On Fri, May 26, 2023 at 07:39:55PM +0200, Domenico Cerasuolo wrote: > > This update addresses an issue with the zswap reclaim mechanism, which > > hinders the efficient offloading of cold pages to disk, thereby > > compromising the preservation of the LRU order and consequently > > diminishing, if not inverting, its performance benefits. > > > > The functioning of the zswap shrink worker was found to be inadequate, > > as shown by basic benchmark test. For the test, a kernel build was > > utilized as a reference, with its memory confined to 1G via a cgroup an= d > > a 5G swap file provided. The results are presented below, these are > > averages of three runs without the use of zswap: > > > > real 46m26s > > user 35m4s > > sys 7m37s > > > > With zswap (zbud) enabled and max_pool_percent set to 1 (in a 32G > > system), the results changed to: > > > > real 56m4s > > user 35m13s > > sys 8m43s > > > > written_back_pages: 18 > > reject_reclaim_fail: 0 > > pool_limit_hit:1478 > > > > Besides the evident regression, one thing to notice from this data is > > the extremely low number of written_back_pages and pool_limit_hit. > > > > The pool_limit_hit counter, which is increased in zswap_frontswap_store > > when zswap is completely full, doesn't account for a particular > > scenario: once zswap hits his limit, zswap_pool_reached_full is set to > > true; with this flag on, zswap_frontswap_store rejects pages if zswap i= s > > still above the acceptance threshold. Once we include the rejections du= e > > to zswap_pool_reached_full && !zswap_can_accept(), the number goes from > > 1478 to a significant 21578266. > > > > Zswap is stuck in an undesirable state where it rejects pages because > > it's above the acceptance threshold, yet fails to attempt memory > > reclaimation. This happens because the shrink work is only queued when > > zswap_frontswap_store detects that it's full and the work itself only > > reclaims one page per run. > > > > This state results in hot pages getting written directly to disk, > > while cold ones remain memory, waiting only to be invalidated. The LRU > > order is completely broken and zswap ends up being just an overhead > > without providing any benefits. > > > > This commit applies 2 changes: a) the shrink worker is set to reclaim > > pages until the acceptance threshold is met and b) the task is also > > enqueued when zswap is not full but still above the threshold. > > > > Testing this suggested update showed much better numbers: > > > > real 36m37s > > user 35m8s > > sys 9m32s > > > > written_back_pages: 10459423 > > reject_reclaim_fail: 12896 > > pool_limit_hit: 75653 > > > > V2: > > - loop against =3D=3D -EAGAIN rather than !=3D -EINVAL and also break t= he loop > > on MAX_RECLAIM_RETRIES (thanks Yosry) > > - cond_resched() to ensure that the loop doesn't burn the cpu (thanks > > Vitaly) > > > > Fixes: 45190f01dd40 ("mm/zswap.c: add allocation hysteresis if pool lim= it is hit") > > Signed-off-by: Domenico Cerasuolo > > --- > > mm/zswap.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 59da2a415fbb..f953dceaab34 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -37,6 +37,7 @@ > > #include > > > > #include "swap.h" > > +#include "internal.h" > > > > /********************************* > > * statistics > > @@ -587,9 +588,17 @@ static void shrink_worker(struct work_struct *w) > > { > > struct zswap_pool *pool =3D container_of(w, typeof(*pool), > > shrink_work); > > + int ret, failures =3D 0; > > > > - if (zpool_shrink(pool->zpool, 1, NULL)) > > - zswap_reject_reclaim_fail++; > > + do { > > + ret =3D zpool_shrink(pool->zpool, 1, NULL); > > + if (ret) { > > + zswap_reject_reclaim_fail++; > > + failures++; > > + } > > + cond_resched(); > > + } while (!zswap_can_accept() && ret =3D=3D -EAGAIN && > > + failures < MAX_RECLAIM_RETRIES); > > It should also loop on !ret, right? > > AFAIU Yosry's suggestion was that instead of breaking only on -EINVAL, > it should break on all failures but -EAGAIN. But it should still keep > going if the shrink was successful and the pool cannot accept yet. > > Basically, something like this? > > do { > ret =3D zpool_shrink(pool->zpool, 1, NULL); > if (ret) { > zswap_reject_reclaim_fail++; > if (ret !=3D -EAGAIN) > break; > if (++failures =3D=3D MAX_RECLAIM_RETRIES) > break; > } > cond_resched(); > } while (!zswap_can_accept()); Yes, that's what I meant. Otherwise if shrink is successful we end up doing 1 page only, which is exactly what we are trying to avoid here.