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 258AFC48260 for ; Fri, 16 Feb 2024 10:41:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9BE428D0006; Fri, 16 Feb 2024 05:41:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 96DC48D0001; Fri, 16 Feb 2024 05:41:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 80F598D0006; Fri, 16 Feb 2024 05:41:56 -0500 (EST) 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 6E1CF8D0001 for ; Fri, 16 Feb 2024 05:41:56 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E7C12C04CD for ; Fri, 16 Feb 2024 10:41:55 +0000 (UTC) X-FDA: 81797326590.30.25AA553 Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by imf10.hostedemail.com (Postfix) with ESMTP id 318B2C0009 for ; Fri, 16 Feb 2024 10:41:53 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Pe2eo9vj; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.53 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708080114; 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=83MFj85Jw6q6w+0p9hI3fU9QbFllJZZhmeN4SqwEUMA=; b=v+FDhW7Ydpd812ScoEtCpt8xTu+6BLxbsknS+PIjltDn/AaQ4GgRp9xn6fk24kVvHlLkuo vhIQRLsTbYTyxLJObFLskhkVTM7Cz4rbYGgEeP3ettxNNTO6h/DE9uoWwOEvUMq84hQhlT apLM43IOZWN5fXz3inX+bxFVQCeqAmE= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Pe2eo9vj; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.53 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708080114; a=rsa-sha256; cv=none; b=Red12yUvhrsXKyTLpuaNJFKzByZ2DpPsbnA4orhB8YbYYHpgfW3iUnxiNYne/xswYIAt9t e1RqpGH4oYg2EaGdsTrsYZsYw7Bj7LFA2eMEhzKX9RQdt4TDcN7M+FCUqu7i5jhVfVAiHj gpiuwgRxEwENt1a0uOAly0zUswQQkEQ= Received: by mail-io1-f53.google.com with SMTP id ca18e2360f4ac-7bc32b0fdadso34428039f.2 for ; Fri, 16 Feb 2024 02:41:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708080113; x=1708684913; 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=83MFj85Jw6q6w+0p9hI3fU9QbFllJZZhmeN4SqwEUMA=; b=Pe2eo9vjwC+GY4MNf9NgoMI1VUsjuXgxalCrhCwBZZLsKfw7Stods6ZU7WVr+Qy31u jDDi7mkDASX5o6g0r8Rkm+dxTKWFG5xvPNEp0MQkCrIrOz5Ki53NdLbxJ/VvGtckjsAc pEeIh2MnXflDU8pD//mciny6LUhsoBixuJtrzqr5jNRLg0TsxhWJb0ClsFXeKpcDsO0B zHOB8v22PxmJ3fPWatJ0WuvCWFnjfOclRFatZw0FracUa33OiVA5DdBOgX4+OY8gGLO6 vUgfeHnKlv3lae/PxOhKGtjcTcMqQApVGI3dxeQ0bml2UDzXI5VpsJJ65HHpphreDL0N d5UQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708080113; x=1708684913; 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=83MFj85Jw6q6w+0p9hI3fU9QbFllJZZhmeN4SqwEUMA=; b=XlEXk5LDpVjunu08Pw+ERskkEmciIJsxU90Z7tHkt9gQk+u32lg5TQTGTsmtqcaksg 7ylyAoNJE+U++tZwhg5+2nkWItdtFfr1FIMcZ/WidK+odbafOOUCRr57OtX/cOb7SdPd cXD+WNRvIXIFhcgEL6u3g2x99Pb5mlpXpuA4eypk389ob7YKWPGd9ib9SWyQ0etZo3KF ERfxFkyJY3lBSuYZ+81LpySaJfHmV5qnI51JCyQhbQIwBXjKMzouylYz6KzRvDUXjemP 1JeDx8MJX/nbprkv2XaWktXRY++woxtI+NTx79Y+5MZSwLaHMep1ScnQZll/EXqUGbNe 2ZqA== X-Forwarded-Encrypted: i=1; AJvYcCUEKK3XPXdwONhKM3Qb7kWRMvkguRxomXV8xQusVclWmqBwxmZsjku/ahvqbgTI2c8nR5VAT4W+VxN5mpclqSaBPhM= X-Gm-Message-State: AOJu0Yxz9Vv079Kf/oopca3+WZbmUt8MRPyWBgGk7JDjt5Umdj4sKYyE bxxA76M5M1OPK8mIXP+LPmOj9OpINjkotQfHYXG1nTDGcgMuQMU/+BjVyenNbn7Prq+xXW3eCKJ 9W+R4rYecERKArg6IlXCktHs3Qu4= X-Google-Smtp-Source: AGHT+IHohGyM7aqhsbOgrJGONi+DTcnydQ/SQ/7ZiCjHTxzt7rc6dyqTE2f0MbPjJAFGOYqpnap/L8iU4kZOMkST50k= X-Received: by 2002:a6b:1547:0:b0:7c4:3218:c767 with SMTP id 68-20020a6b1547000000b007c43218c767mr5045416iov.11.1708080113226; Fri, 16 Feb 2024 02:41:53 -0800 (PST) MIME-Version: 1.0 References: <20240210-zswap-global-lru-v3-0-200495333595@bytedance.com> <20240210-zswap-global-lru-v3-2-200495333595@bytedance.com> In-Reply-To: <20240210-zswap-global-lru-v3-2-200495333595@bytedance.com> From: Nhat Pham Date: Fri, 16 Feb 2024 02:41:41 -0800 Message-ID: Subject: Re: [PATCH v3 2/2] mm/zswap: change zswap_pool kref to percpu_ref To: Chengming Zhou Cc: Johannes Weiner , Yosry Ahmed , 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: 318B2C0009 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: iupnher799sducyssx85m3en3a8yuihq X-HE-Tag: 1708080113-924538 X-HE-Meta: U2FsdGVkX18WghihZ7ELqbVvh77wYK3u2lghdwcxecTSqxxsCHWB6kUMumuQCzPncW6JgPjEKVYqSG5GllJIKbsk/b7v5poESIwTKCi7SS/xASLruMGOtYT0OKNPFuNLiHPUigrLbiZ65Q0AHJF9KZjjhLAjgnRMB2jIgbcm35/okgc/pXarOwOWqL2PLVnpRIZd+Q7IWBku5oGurWR0av1MZ6tDckOk8CF5L+u7Uqc9N860wdsE9CGzO4lUTV8VNCUIWGc8DJaZDn8rElWrIut+uTNp5NhJ4NFbdSgaG1/oIytWTEkxE+JioGQfuqwChQq32h2Gpa2vk1QJusHAFhbAkFpLbpSv6LFPC6KtkvKL2KS04mwrF3r7aSX1mG3bSjd8ZxVG3Kf5MiFWyUs0cylrQhwD7jLoQT28Tb3nIpkAXdg7ZrKffA4DLx8tn4beGD3/DEX/XiKb8MJKmGtm/tZyIJ6jtHbYWaWoG2JkxJ7R5IyYtDb62h0DDsAbzXp1JSeq09CzcYSiyn+2OzYh1FUYo1B6y+lL0KsWii3ycjCjSXbU7LZhwgpKVx9e0ONqTDZT5MVaiP8xhtC+EOCJ8cCcBwYjeOsbJ2hgypXsKrjHRyXI1TFHQx1WapRjfXFRYNqeHHfmwQ9O6qt5mCX82xeLof0v7cOvXvqfMWnQg4zRla23pyQX9PALB/9lod9Bkc8bFxhUzif2mQQR0xcndHc79FjJpPNUDZPYINWYJ6Z+3Wy4myxWNu1l7GDC2f//fRt5qlqCytyrUdCrhyQf/D9ka5xNPhWrhmN5TYUXLlROaHBXLEQWqgTAzAXb68WzCmv+tnhejb9lfogzfwbVlZ0EEWN8daYDtktKHU6/1KkNpxp+vW3Ldn+L6Ok+UpHAd7P8v5rFFM45mI+A1HYbCh9hjaEiWx16TywP75gWuArG78CezhBpEx6+VlI8DlO55gFECdvPjzl3QmmmSRu IvVyObmt 86SBOpcZyT/E6eGTT+1isA9UONc6gJyQpX7DC3NyjR1KOjrB+04xd1y5c4dFhph7hCeGesBUrdfF1RakmWlxE2DedzUgv3f9W9SVg5p0JiN/usrvHJm58yRhik19sIXerf3XqBhx7KTAecw86yqXGVbozfefzAzavaZrf9/kD+OijQ1Toik21PLNo/GxERhglSILAnLMtxDuBWkt3lr84JtJTl7Wbe4oFbaNtZP1GnzJkWiZaKs0KFxZGO8elukyVthukhlnB4ZUVjHUACQDQwtLgwqARrHfGKl+r2iN2GSi4iwRASvhmW/3Gm+0A4iy9livLhXpWLJ/gG4YZcqKwObZgXp3Af/TVY7XzmdZS4BYJtlslDfNVh3tWZZrQzLxaDJeYpwcbB2d8rVVreWOy0ofjJpnJvumuu6MONzG6wx8O0s3bFJEL9Qdr7UcncvDPq5g4np6W+pH6np7kmISWrD9ZoldzUS4Vehir 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, Feb 16, 2024 at 12:55=E2=80=AFAM Chengming Zhou wrote: > > All zswap entries will take a reference of zswap_pool when > zswap_store(), and drop it when free. Change it to use the > percpu_ref is better for scalability performance. > > Although percpu_ref use a bit more memory which should be ok > for our use case, since we almost have only one zswap_pool to > be using. The performance gain is for zswap_store/load hotpath. > > Testing kernel build (32 threads) in tmpfs with memory.max=3D2GB. > (zswap shrinker and writeback enabled with one 50GB swapfile, > on a 128 CPUs x86-64 machine, below is the average of 5 runs) > > mm-unstable zswap-global-lru > real 63.20 63.12 > user 1061.75 1062.95 > sys 268.74 264.44 Idea is straightforward + code looks solid to me FWIW, so: Reviewed-by: Nhat Pham > > Signed-off-by: Chengming Zhou > --- > mm/zswap.c | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index d275eb523fc4..961349162997 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -173,7 +173,7 @@ struct crypto_acomp_ctx { > struct zswap_pool { > struct zpool *zpools[ZSWAP_NR_ZPOOLS]; > struct crypto_acomp_ctx __percpu *acomp_ctx; > - struct kref kref; > + struct percpu_ref ref; > struct list_head list; > struct work_struct release_work; > struct hlist_node node; > @@ -305,6 +305,7 @@ static void zswap_update_total_size(void) > /********************************* > * pool functions > **********************************/ > +static void __zswap_pool_empty(struct percpu_ref *ref); > > static struct zswap_pool *zswap_pool_create(char *type, char *compressor= ) > { > @@ -358,13 +359,18 @@ static struct zswap_pool *zswap_pool_create(char *t= ype, char *compressor) > /* being the current pool takes 1 ref; this func expects the > * caller to always add the new pool as the current pool > */ > - kref_init(&pool->kref); > + ret =3D percpu_ref_init(&pool->ref, __zswap_pool_empty, > + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); > + if (ret) > + goto ref_fail; > INIT_LIST_HEAD(&pool->list); > > zswap_pool_debug("created", pool); > > return pool; > > +ref_fail: > + cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->no= de); > error: > if (pool->acomp_ctx) > free_percpu(pool->acomp_ctx); > @@ -437,8 +443,9 @@ static void __zswap_pool_release(struct work_struct *= work) > > synchronize_rcu(); > > - /* nobody should have been able to get a kref... */ > - WARN_ON(kref_get_unless_zero(&pool->kref)); > + /* nobody should have been able to get a ref... */ > + WARN_ON(!percpu_ref_is_zero(&pool->ref)); Ah nice - this is actually even clearer :) For some reason I missed it, my apologies. > + percpu_ref_exit(&pool->ref); > > /* pool is now off zswap_pools list and has no references. */ > zswap_pool_destroy(pool); > @@ -446,11 +453,11 @@ static void __zswap_pool_release(struct work_struct= *work) > > static struct zswap_pool *zswap_pool_current(void); > > -static void __zswap_pool_empty(struct kref *kref) > +static void __zswap_pool_empty(struct percpu_ref *ref) > { > struct zswap_pool *pool; > > - pool =3D container_of(kref, typeof(*pool), kref); > + pool =3D container_of(ref, typeof(*pool), ref); > > spin_lock(&zswap_pools_lock); > > @@ -469,12 +476,12 @@ static int __must_check zswap_pool_get(struct zswap= _pool *pool) > if (!pool) > return 0; > > - return kref_get_unless_zero(&pool->kref); > + return percpu_ref_tryget(&pool->ref); > } > > static void zswap_pool_put(struct zswap_pool *pool) > { > - kref_put(&pool->kref, __zswap_pool_empty); > + percpu_ref_put(&pool->ref); > } > > static struct zswap_pool *__zswap_pool_current(void) > @@ -604,6 +611,17 @@ static int __zswap_param_set(const char *val, const = struct kernel_param *kp, > > if (!pool) > pool =3D zswap_pool_create(type, compressor); > + else { > + /* > + * Restore the initial ref dropped by percpu_ref_kill() > + * when the pool was decommissioned and switch it again > + * to percpu mode. > + */ > + percpu_ref_resurrect(&pool->ref); > + > + /* Drop the ref from zswap_pool_find_get(). */ > + zswap_pool_put(pool); > + } > > if (pool) > ret =3D param_set_charp(s, kp); > @@ -642,7 +660,7 @@ static int __zswap_param_set(const char *val, const s= truct kernel_param *kp, > * or the new pool we failed to add > */ > if (put_pool) > - zswap_pool_put(put_pool); > + percpu_ref_kill(&put_pool->ref); > > return ret; > } > > -- > b4 0.10.1