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 BC9CFC3064D for ; Tue, 25 Jun 2024 04:31:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F16376B0328; Tue, 25 Jun 2024 00:31:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EC7A06B032A; Tue, 25 Jun 2024 00:31:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DB4C46B032B; Tue, 25 Jun 2024 00:31:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id B6C176B0328 for ; Tue, 25 Jun 2024 00:31:24 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 2AB101C25D7 for ; Tue, 25 Jun 2024 04:31:24 +0000 (UTC) X-FDA: 82268136888.17.00570F5 Received: from out-179.mta0.migadu.com (out-179.mta0.migadu.com [91.218.175.179]) by imf01.hostedemail.com (Postfix) with ESMTP id 834B64000A for ; Tue, 25 Jun 2024 04:31:21 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=bRWEk8ny; spf=pass (imf01.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.179 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719289867; 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=2+STWZO5CMAxCSj0NI0Jy02Uub6PHOuwZTkIl4GXzvo=; b=H53Y+ohuFSAXds55aeye93Bq5/XsIZWs6Zi5npb9MMktF5SJ4o/w/sIxR+pLgP13ZVEXmd MhwjQZb3X9jlvFuPzD1uzfCvAEc7jZUj2cZYhkUMyZFP55ylcER2MskXG57NMQBZFy9XD1 XeXOHn8jCb1PqbVBAdX7kh+EdUtJ+EA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719289867; a=rsa-sha256; cv=none; b=mG961dd1bZL2+9UzT/HljsZzdDhY2+K8Z2Yr0/ixtPR2Su7Yn337l2536WSC4Fu0Q6tAQR Jppfs9OnhTzilLmYuchtr0nq8PY7kD+kSz0Qz60G1piVskUpnWYL1TjbIk+XawBETinbdW 41zOHK/kZFIXZJW4EevEfgimwp/7McY= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=bRWEk8ny; spf=pass (imf01.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.179 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev X-Envelope-To: yosryahmed@google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1719289879; h=from:from: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; bh=2+STWZO5CMAxCSj0NI0Jy02Uub6PHOuwZTkIl4GXzvo=; b=bRWEk8ny/nPkkgTobgWE7Mu5yfIn4JCuCEnOfRALQCLQLRxJAhHWfPByga7LNU7KMfh81C c4R/uTYvuge0v34MgjJB1OtD2ibgyylNfWCP5+VNdcMOaFtTNVVU0onc8/JynTp1tbkKks 1+Gmjx4d4dGEeyzUy7aKpiE5tMaWKV0= X-Envelope-To: minchan@kernel.org X-Envelope-To: senozhatsky@chromium.org X-Envelope-To: akpm@linux-foundation.org X-Envelope-To: hannes@cmpxchg.org X-Envelope-To: nphamcs@gmail.com X-Envelope-To: yuzhao@google.com X-Envelope-To: flintglass@gmail.com X-Envelope-To: zhouchengming@bytedance.com X-Envelope-To: dan.carpenter@linaro.org X-Envelope-To: linux-mm@kvack.org X-Envelope-To: linux-kernel@vger.kernel.org Message-ID: Date: Tue, 25 Jun 2024 12:31:11 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v2 2/2] mm/zswap: use only one pool in zswap To: Yosry Ahmed Cc: Minchan Kim , Sergey Senozhatsky , Andrew Morton , Johannes Weiner , Nhat Pham , Yu Zhao , Takero Funaki , Chengming Zhou , Dan Carpenter , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240621-zsmalloc-lock-mm-everything-v2-0-d30e9cd2b793@linux.dev> <20240621-zsmalloc-lock-mm-everything-v2-2-d30e9cd2b793@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Stat-Signature: rhgme9z7ysu94a13qdd1ibw7nzijqhfe X-Rspamd-Queue-Id: 834B64000A X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1719289881-215492 X-HE-Meta: U2FsdGVkX188bNGJfiTg8HuCtxoejW1z1gZ+P/4TjecBsBVlwATs06MvUGyEMqnzpUgc/2Ja4520xmBpxaSFcaPULYMp70QdqEFR4Ye8OMawWt8egnozsicd6jNKeBlHevV05zCq4xQOniIvwB/NnXEeHg8sR29bgMJZn+s68UnI2AkqBHmmK5BF6nS3Q25pssLhHYm+bmKcjH3nfqKmXMacqA2F9B/B8uIXYh+38k+ZgK9I42WPQ+LACs3d1pxcCBkP5GBfivJt8FZRn7J+prPml2K3ajqbLWgUo2Q0C44kECG7KRygmr78NbzMEvMJ3HRyeaVpibfNDxrQywNmPY+XUueM1+CkTEw7vSUmCGh5X+J34Xi2I3IVhfD6jcYJfqN0wOhaMtMOidG9A0tK1vPJDILHbDRlA4SluAUbVEVHkBZQeZ31vmU+mGGTw+53eMPXUlPiVtVS3QghHlsNlFRrqcgF1I/uRAj7iGGHYqwak5V82+Wr/CnwnbXxpyMzr9AtMVwoAEyDA2JpFboflhiTS6CWsDBJRectmwpIpZQuSJEOi57CD2Rbu/eTm0gib1hJAeZ9s5kbYL8U0B1odvyqB/eTcArtMFWKJcX0srrA2v4tcNqhXyrHbHfC15z7O/cR1/orUKHYxD3bP9lawmoDEzXtkyLwfmKx0bogqEHg4p1bK0c21OH1fU/JuIywFC+mJEQEuNaRUJzrHddFxaO5EQEhWh1PDKedMg/qv4njvmOm6GzICK8bbV/VEnx3Gb7hajoWGw10y88lRkYupZxCmu+l9yQTSgJMjgHHz7zLzFRtlgb0eRlDlOxIBQG5/+ZRpYR/vng3xunskzK5hJcQTNKBWiyU0JZz69XO80A2qL44kogRhPLoileaqtQfAIVBow3jr2uGr4/Qz0rQPdkESxR5dhrgSb/+f78RCrnbirWRa12ObwQCgAVUVwINXxb5qPG9xcWNLx4XNg7 G2UpTR2r FGw+jWKnQDhcaHD92ScNM8Fi3aWZBi4JDgOMMVR5YGVQVwnE+yt4ScFE1VIr7w+iCErzToE0Sezbz/qZahCW9Xufs4FUX1jTJVTX/WtliF9xz1SekfMai+BZRcXW8Hh3a1g/+ki8JKamGn16cV+1HnoneyAUUZORp05Eb1egYzmPsnuG1An9hDZmeMyKFSxlW4cCfRV3v8yfvUgI= 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 2024/6/24 20:14, Yosry Ahmed wrote: > On Fri, Jun 21, 2024 at 12:15 AM Chengming Zhou > wrote: >> >> Zswap uses 32 pools to workaround the locking scalability problem in >> zswap backends (mainly zsmalloc nowadays), which brings its own problems >> like memory waste and more memory fragmentation. >> >> Testing results show that we can have near performance with only one >> pool in zswap after changing zsmalloc to use per-size_class lock instead >> of pool spinlock. >> >> Testing kernel build (make bzImage -j32) on tmpfs with memory.max=1GB, >> and zswap shrinker enabled with 10GB swapfile on ext4. >> >> real user sys >> 6.10.0-rc3 138.18 1241.38 1452.73 >> 6.10.0-rc3-onepool 149.45 1240.45 1844.69 >> 6.10.0-rc3-onepool-perclass 138.23 1242.37 1469.71 >> >> And do the same testing using zbud, which shows a little worse performance >> as expected since we don't do any locking optimization for zbud. I think >> it's acceptable since zsmalloc became a lot more popular than other >> backends, and we may want to support only zsmalloc in the future. >> >> real user sys >> 6.10.0-rc3-zbud 138.23 1239.58 1430.09 >> 6.10.0-rc3-onepool-zbud 139.64 1241.37 1516.59 >> >> Reviewed-by: Nhat Pham >> Signed-off-by: Chengming Zhou >> --- >> mm/zswap.c | 60 +++++++++++++++++++----------------------------------------- >> 1 file changed, 19 insertions(+), 41 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index e25a6808c2ed..7925a3d0903e 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -122,9 +122,6 @@ static unsigned int zswap_accept_thr_percent = 90; /* of max pool size */ >> module_param_named(accept_threshold_percent, zswap_accept_thr_percent, >> uint, 0644); >> >> -/* Number of zpools in zswap_pool (empirically determined for scalability) */ >> -#define ZSWAP_NR_ZPOOLS 32 >> - >> /* Enable/disable memory pressure-based shrinker. */ >> static bool zswap_shrinker_enabled = IS_ENABLED( >> CONFIG_ZSWAP_SHRINKER_DEFAULT_ON); >> @@ -160,7 +157,7 @@ struct crypto_acomp_ctx { >> * needs to be verified that it's still valid in the tree. >> */ >> struct zswap_pool { >> - struct zpool *zpools[ZSWAP_NR_ZPOOLS]; >> + struct zpool *zpool; >> struct crypto_acomp_ctx __percpu *acomp_ctx; >> struct percpu_ref ref; >> struct list_head list; >> @@ -237,7 +234,7 @@ static inline struct xarray *swap_zswap_tree(swp_entry_t swp) >> >> #define zswap_pool_debug(msg, p) \ >> pr_debug("%s pool %s/%s\n", msg, (p)->tfm_name, \ >> - zpool_get_type((p)->zpools[0])) >> + zpool_get_type((p)->zpool)) >> >> /********************************* >> * pool functions >> @@ -246,7 +243,6 @@ static void __zswap_pool_empty(struct percpu_ref *ref); >> >> static struct zswap_pool *zswap_pool_create(char *type, char *compressor) >> { >> - int i; >> struct zswap_pool *pool; >> char name[38]; /* 'zswap' + 32 char (max) num + \0 */ >> gfp_t gfp = __GFP_NORETRY | __GFP_NOWARN | __GFP_KSWAPD_RECLAIM; >> @@ -267,18 +263,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor) >> if (!pool) >> return NULL; >> >> - for (i = 0; i < ZSWAP_NR_ZPOOLS; i++) { >> - /* unique name for each pool specifically required by zsmalloc */ >> - snprintf(name, 38, "zswap%x", >> - atomic_inc_return(&zswap_pools_count)); >> - >> - pool->zpools[i] = zpool_create_pool(type, name, gfp); >> - if (!pool->zpools[i]) { >> - pr_err("%s zpool not available\n", type); >> - goto error; >> - } >> + /* unique name for each pool specifically required by zsmalloc */ >> + snprintf(name, 38, "zswap%x", atomic_inc_return(&zswap_pools_count)); >> + pool->zpool = zpool_create_pool(type, name, gfp); >> + if (!pool->zpool) { >> + pr_err("%s zpool not available\n", type); >> + return NULL; > > We need to goto error here to free the pool. > >> } >> - pr_debug("using %s zpool\n", zpool_get_type(pool->zpools[0])); >> + pr_debug("using %s zpool\n", zpool_get_type(pool->zpool)); >> >> strscpy(pool->tfm_name, compressor, sizeof(pool->tfm_name)); >> >> @@ -311,8 +303,7 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor) >> error: >> if (pool->acomp_ctx) >> free_percpu(pool->acomp_ctx); >> - while (i--) >> - zpool_destroy_pool(pool->zpools[i]); >> + zpool_destroy_pool(pool->zpool); > > .. and then we will need a NULL check needed here. Oops, my bad, will fix in the next version. Thanks!