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 780D8C48260 for ; Fri, 16 Feb 2024 20:01:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D81788D0006; Fri, 16 Feb 2024 15:01:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D07B18D0002; Fri, 16 Feb 2024 15:01:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B82558D0006; Fri, 16 Feb 2024 15:01:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 9ECDD8D0002 for ; Fri, 16 Feb 2024 15:01:56 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 5816B8028F for ; Fri, 16 Feb 2024 20:01:56 +0000 (UTC) X-FDA: 81798737832.16.BDC8233 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by imf06.hostedemail.com (Postfix) with ESMTP id 98DB3180005 for ; Fri, 16 Feb 2024 20:01:54 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=B+AiyJ91; spf=pass (imf06.hostedemail.com: domain of 3Mb_PZQoKCCQYOSRYAHMEDGOOGLE.COMLINUX-MMKVACK.ORG@flex--yosryahmed.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3Mb_PZQoKCCQYOSRYAHMEDGOOGLE.COMLINUX-MMKVACK.ORG@flex--yosryahmed.bounces.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=1708113714; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=hnC1Wqu1hjzO44T2i0P4fr+YtBb6/PxLiNAZpb948xQ=; b=U+j0uUP7ECGqg4uHFfB4pHzoGiLWdr9KgUW8B1Rs9Rrc6tvZYQZKJ4ARQnd4wNvCfijOYP 45rxhRktFms0YIgg9rL/t8OGvxfqAdSqqKIRLrbtqjIgujlx001wUIFa6tekInLrPymFue yNY3eQr2nUs3u5GvbGQ+mvmNZuacago= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708113714; a=rsa-sha256; cv=none; b=Gms3L4c8l1J97ZTbodS2CIcIAAbRQ3vW6Tq7Qo0shAXsN64yU15ACNfNdn1AjV08n9rlxp nstijOs6oyU5ZZ4KqNJecTTYlTG47fCAaAHbGJ+oWbgKEGuW85ZQuLxXjlpQbO1B2RnFZ3 tfj82Ad9scQy+JQM6UGjXG1qLfCaFOI= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=B+AiyJ91; spf=pass (imf06.hostedemail.com: domain of 3Mb_PZQoKCCQYOSRYAHMEDGOOGLE.COMLINUX-MMKVACK.ORG@flex--yosryahmed.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3Mb_PZQoKCCQYOSRYAHMEDGOOGLE.COMLINUX-MMKVACK.ORG@flex--yosryahmed.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-dc6b26845cdso3986754276.3 for ; Fri, 16 Feb 2024 12:01:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1708113713; x=1708718513; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=hnC1Wqu1hjzO44T2i0P4fr+YtBb6/PxLiNAZpb948xQ=; b=B+AiyJ91ikaVqfKrShJdlPmMxDHEpno2ca9czcdpl0RI55zic6WigU2T+Men3dpmyl xX0RwX5T77TJkJ+36YUNKJeJ4Ufwo+hrv7/3EaS1szP2nkmVu9V1YB+wmuNT0GMXRf4Q KGKWprUO4FJbbex21f/vjdkvuynC9scuoWs9k6xK8gIah62Df21UIMykF8h1y+zNFopb tj57GxI5b7H+UbjcuTfA3uzSKjPfHYSx1xl3FKw5RmgcvFEUyz+QUC7gP3XfPVoPCatA EGqp3Lur2aUCAjZhC5nOPkbCssBcBZemecF3l4RkSoUDRtOHbZEVAHkNuIB9Y+CfAwxi f34w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708113713; x=1708718513; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=hnC1Wqu1hjzO44T2i0P4fr+YtBb6/PxLiNAZpb948xQ=; b=kUIemplk2tyO1Ga2m1FChXmBVt1Ofm4oVYjiEIb1Ketf1HmI133sGB4UbaP7m7Bw0P 1LSGrcf3Sps9KO8gZrqSE76+0AvKyU5zMl4tSLoX5DYi3Lzg1PigAbJYEmOeeuj8mtuV 5z97TuNMrly/HthSB+t1vX+gqbbdefckFwzbU1Mouw0rYluvPj7s6i+wJS0R7G2mE1ok KQc8CeLdnVs52JoTeHyNLSvwU7/1JC+MUFD7sqTx1VEmfFdAjSwqNB3KlbcJg+tXL4C0 I1CUPZckdttv9gtiwZRn49x6EwL8X2/RAXyiU+t7DBc8uR42jUmdfangWfH4M6TpfWwj aKtg== X-Forwarded-Encrypted: i=1; AJvYcCUUz7Vcc9R6xNzddHlR85HEDcdrxc2K/brZAiqxEdOxzI79LleHDQgDBJGSLr+weJG4bhReT5QsciLGpWZJJMyePX0= X-Gm-Message-State: AOJu0YzOtTkhX1C7Rb0L74SECz2fiBMZCtTasstZlwug/19P5YY8UybA jbddPb1sXBmImqwXkYWBKGevJge5nqcnzYaN8oYQq5WOPv88rkPYqKqxqEKlCV7BBauAEi5w+yp HJ9k+qN7dsdAoIGMrww== X-Google-Smtp-Source: AGHT+IFsCdaVZr8ZBA4nJqkvY7/NE5G/bJ2OgSA7Qdq0i5ahbMzq/UWaYnaW84Y6C8WrjD7VtzJgJ7eIeU2LSu+N X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a05:6902:1105:b0:dc6:d678:371d with SMTP id o5-20020a056902110500b00dc6d678371dmr1411009ybu.3.1708113713690; Fri, 16 Feb 2024 12:01:53 -0800 (PST) Date: Fri, 16 Feb 2024 20:01:51 +0000 In-Reply-To: Mime-Version: 1.0 References: <20240216030539.110404-1-21cnbao@gmail.com> Message-ID: Subject: Re: [PATCH] mm: zswap: increase reject_compress_poor but not reject_compress_fail if compression returns ENOSPC From: Yosry Ahmed To: Barry Song <21cnbao@gmail.com> Cc: Chengming Zhou , akpm@linux-foundation.org, hannes@cmpxchg.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Barry Song , Nhat Pham , Sergey Senozhatsky Content-Type: text/plain; charset="us-ascii" X-Stat-Signature: b9sge9xrjj4j6jb41jzkqem3bkg7njc7 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 98DB3180005 X-Rspam-User: X-HE-Tag: 1708113714-645722 X-HE-Meta: U2FsdGVkX19uJ4wGfvwjoacE+LPILkgyKLFKHeUwBDXsQuBkq5L/PB+fqCopl2cK6et68GL5+JMgf8uhwlEm1A1WMksNnwb81gtGTeu8/409ARPEVfOzoNRnY8N8G6oPo10qGkAIfNHg7vR3fxUbzE7Kv6Tw+RyDw3FRyeB9auKCfLcZfXvAb+BsN0p50KMsRlRurRbjK80GCzvCxWzpZMKo/zbvWHa1HZQE3PXB4f1EO+wwE25ZjXE8g4cnii0mawVIFkG/fw4vaAWmVTwhN8h/lHGia1Tw0M0oZf9MI0iDw+tGAnPYZTTXNAl+o2zsT08pezKQ0EQ3EgTKFgv8YbJRv/srDPGzRBFcDcHmlOJcnhdRaHBJr1JseWH0inlisu3KK1DDb/jFomrJl7bqYaRdxwCVNNgmvcmz/t7nizgy8OKqyet7ESIM04B1L0TtZ/frQ3xcXXvGwutaeUqsDzBNnoBKhHpgjy3rRqT9lkG/q9xpRzBaV1WEilFFTSxKEWrVbWWfyFPDPoYHj3LC1o6gHPfZhpUw+MQVmOV1NuczCK+vL7ymX+EpwdT+0/WomFujdTWP+R6IRvL5HO9m7SbiQQw+aCTnxIpAoGbjV5lo9kZ3MW9wE9R/MHGHuVBUltBAewPQifKI5hgt9wlWk5dnDX0tG+z+J+IiYieZyqbPl3boqiJaQSjjNrXfQa3Da2r00HLu735am8OwRx/a82VXsrfjiCtDGvpXIvkwS+CsRnv1osw3XqwQF8/NgXatEXl9680NF32jnt3u5pK9f4OB50mBG8+PRm1kh8Dp97koJGLDAmgZByK0L93asN1k+GA9OV6AgHjrRAPjwWBXPKs73wsO5HW2c1bVq3SvfkuWUKZFPpIKwE0W0Z7RhteKiYZ1Ie2tJK2mneajOo0Nia31xAaluJfjZvFGbg0EOt4O6PRe4wsrfwplfq70wvf2ayCI41En0aEJltENH4y gF/Xxe8p kXNpm9Q26S+apqH7wErsW4VXjTkXQQRnGROiWeTXa3GvpCqrQ+FhgcWD0HrEv5sHQJLAYBuHxZm882DZmaVVwkhJ9q6L6xYnjzg4uarRFIRUPuq8OV7B/7GgI2CMizfISu3MkHqq4yXsJyjsg4kAE0vcahHttHSm+ntSRYZm9etGjHQIf+y+bmacYhlFKWYKcACnHogydpW4vwywPg/6eHCQrhoUluH9xpbBvyv8mc3pW2rbilw7oR3LV9qzjIzxuB/zxqQfqLWQ5ks1kiawmDTSFYtbvs7Uk61MpymyIXD00oNz8Gq/WvGw0kG6ECBzJ1MTrjNzXNM1ofRhF2xSDItXwqtQIcsS+MCO7YOx2bbJaGX/wyrnsnhDKP8fsj3a2ct4SvzXYGaR70ZvJQ6qdtrDyM0kT8FjFHFbeV3iD1Gp182g= 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: > > >> diff --git a/mm/zswap.c b/mm/zswap.c > > >> index 6319d2281020..9a21dbe8c056 100644 > > >> --- a/mm/zswap.c > > >> +++ b/mm/zswap.c > > >> @@ -1627,7 +1627,10 @@ bool zswap_store(struct folio *folio) > > >> dlen = acomp_ctx->req->dlen; > > >> > > >> if (ret) { > > >> - zswap_reject_compress_fail++; > > >> + if (ret == -ENOSPC) > > >> + zswap_reject_compress_poor++; > > >> + else > > >> + zswap_reject_compress_fail++; > > > > > > With this diff, we have four locations in zswap_store() where we > > > increment zswap_reject_compress_{poor/fail}. > > > > > > How about the following instead?A > > > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > > index 62fe307521c93..3a7e8ba7f6116 100644 > > > --- a/mm/zswap.c > > > +++ b/mm/zswap.c > > > @@ -1059,24 +1059,16 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) > > > */ > > > ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait); > > > dlen = acomp_ctx->req->dlen; > > > - if (ret) { > > > - zswap_reject_compress_fail++; > > > + if (ret) > > > goto unlock; > > > - } > > > > > > zpool = zswap_find_zpool(entry); > > > gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; > > > if (zpool_malloc_support_movable(zpool)) > > > gfp |= __GFP_HIGHMEM | __GFP_MOVABLE; > > > ret = zpool_malloc(zpool, dlen, gfp, &handle); > > > - if (ret == -ENOSPC) { > > > - zswap_reject_compress_poor++; > > > - goto unlock; > > > - } > > > - if (ret) { > > > - zswap_reject_alloc_fail++; > > > + if (ret) > > > goto unlock; > > > - } > > > > > > buf = zpool_map_handle(zpool, handle, ZPOOL_MM_WO); > > > memcpy(buf, dst, dlen); > > > @@ -1086,6 +1078,10 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) > > > entry->length = dlen; > > > > > > unlock: > > > + if (ret == -ENOSPC) > > > + zswap_reject_compress_poor++; > > > + else if (ret) > > > + zswap_reject_alloc_fail++; > > > > Here have two cases: zswap_reject_compress_fail, zswap_reject_alloc_fail. Ah brain fart, sorry. > > Is it safe to differentiate these two cases by checking ret == -ENOMEM ? > otherwise, it seems the original patch still makes more sense? I don't think it is in all cases, some allocators return other error codes. It seems unlikely that we'll get any of them, but it can be missed in the future. How about we use different return codes to differentiate failures, and still centralize the counters handling. Something like the following (ideally that one is not a brain fart): diff --git a/mm/zswap.c b/mm/zswap.c index 62fe307521c93..20ba25b7601a7 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -1021,12 +1021,12 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) { struct crypto_acomp_ctx *acomp_ctx; struct scatterlist input, output; + int comp_ret = 0, alloc_ret = 0; unsigned int dlen = PAGE_SIZE; unsigned long handle; struct zpool *zpool; char *buf; gfp_t gfp; - int ret; u8 *dst; acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx); @@ -1057,26 +1057,18 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) * but in different threads running on different cpu, we have different * acomp instance, so multiple threads can do (de)compression in parallel. */ - ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait); + comp_ret = crypto_wait_req(crypto_acomp_compress(acomp_ctx->req), &acomp_ctx->wait); dlen = acomp_ctx->req->dlen; - if (ret) { - zswap_reject_compress_fail++; + if (comp_ret) goto unlock; - } zpool = zswap_find_zpool(entry); gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; if (zpool_malloc_support_movable(zpool)) gfp |= __GFP_HIGHMEM | __GFP_MOVABLE; - ret = zpool_malloc(zpool, dlen, gfp, &handle); - if (ret == -ENOSPC) { - zswap_reject_compress_poor++; - goto unlock; - } - if (ret) { - zswap_reject_alloc_fail++; + alloc_ret = zpool_malloc(zpool, dlen, gfp, &handle); + if (alloc_ret) goto unlock; - } buf = zpool_map_handle(zpool, handle, ZPOOL_MM_WO); memcpy(buf, dst, dlen); @@ -1086,6 +1078,13 @@ static bool zswap_compress(struct folio *folio, struct zswap_entry *entry) entry->length = dlen; unlock: + if (comp_ret == -ENOSPC || alloc_ret == -ENOSPC) + zswap_reject_compress_poor++; + else if (comp_ret) + zswap_reject_compress_fail++; + else if (alloc_ret) + zswap_reject_alloc_fail++; + mutex_unlock(&acomp_ctx->mutex); return ret == 0; } > > > > > > mutex_unlock(&acomp_ctx->mutex); > > > return ret == 0; > > > } > > > > > >> goto put_dstmem; > > >> } > > >> > > >> -- > > >> 2.34.1 > > >> > > Thanks > Barry