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 D4227C48BC3 for ; Mon, 12 Feb 2024 18:53:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 399446B0083; Mon, 12 Feb 2024 13:53:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 349906B0085; Mon, 12 Feb 2024 13:53:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2115F6B0087; Mon, 12 Feb 2024 13:53:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 12F5A6B0083 for ; Mon, 12 Feb 2024 13:53:22 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id CD643160761 for ; Mon, 12 Feb 2024 18:53:21 +0000 (UTC) X-FDA: 81784049802.02.8714CC3 Received: from mail-il1-f171.google.com (mail-il1-f171.google.com [209.85.166.171]) by imf16.hostedemail.com (Postfix) with ESMTP id 0E45718001A for ; Mon, 12 Feb 2024 18:53:19 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=hMA9vlKd; spf=pass (imf16.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.171 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707764000; 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=X4QFqP4yLnP1kaBdv6CTyW0dZP0E/ZSe+IpLxw5UGMo=; b=nKM/AjNCB/uS2Mlq56olX6MQ2CjSVU1kwoyx0NtagN/kd1qlOB7C9LRUcgAgRLXMtz24fY snl2dMBlIjgfEkR1wGfbGxXY/VmKpCsKtwVsbDXYrvtFIjdRIT9q9zXulTKdKnaTALCyHN KLnAz66bVGb++EDc/YDESlzawD9SIwg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707764000; a=rsa-sha256; cv=none; b=b3WCJwgJleNKLrYSdvvaUeqJBdT5/da64PxAZrGU063QopM4Ofaf5AFa9x2uRZ9O84Vp4w TSHVfnkiLzC78Ql5Z8MVkgoDj6rAJuqLetnTlwVEU0yrtwCDoOJaIY+zfroVBWRFcvkYXR cJ8ersnAN1+o423SA/5IQluZNYGBifI= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=hMA9vlKd; spf=pass (imf16.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.171 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-il1-f171.google.com with SMTP id e9e14a558f8ab-363afc38a1cso11315035ab.3 for ; Mon, 12 Feb 2024 10:53:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707763999; x=1708368799; 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=X4QFqP4yLnP1kaBdv6CTyW0dZP0E/ZSe+IpLxw5UGMo=; b=hMA9vlKdodq/5AHlBm8j2DeXkyWMK/t4Uc0bq8qdx1n2jdJLRmbGF40Nh+bNWN5xnD iMlT8kYkH4IvCOeXL0fgwjTVf5pch1yLPTJXoRLxdYIWErHN1W5EbxUJ2pB9Am3PUQ7z EKV4BNDBwe8qRCjiDoPZLZfSR0Kvv7kxfnISEb1aZg7Bcq7R6l+IQxq/3BhScoAUdc91 GXTA4khZHWDHFEuQBTgXarl7GAcA3wCqwhmJo4wWpq0/2dgIQjANgFTzE58BX1fG7AVx wywPO2y42DbMCyMa3s0BbVk/Lh/D4RA2VULfEKih3h6U7aWEcRLXmn62hRFiTOan6pBB qdIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707763999; x=1708368799; 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=X4QFqP4yLnP1kaBdv6CTyW0dZP0E/ZSe+IpLxw5UGMo=; b=q8nr2uRZdWlho4ONxp2OXEcwHU3G3u0iBvP8dfDSqBxmofSVUVXOhxRIgI471a0hF4 ZsCZd9l0odfNedcy3vOKxFkLix+J/lzTIt//Ct5VBH0SuFRszikEoXeGCqtO5F+z2rMK J95NSnSectwVnIY+S/TFqkPdp9PZCbLLfoLmMGH8XbXANcMTa7gE26UW6YtdR1BEnWRz aseXbMXUv0wkDqH6PvNzaOud9KXnqOGp+TRU4me4nAjpOz9utfJ3/JaL+EjG2J8sCy2v 1uZZ2yZx15KD+qFDzVgOiMg3N5Hh9xsiWSVBGy8IspfhnSdmciHu2BkAqNGx+Usq3ETv mIcw== X-Forwarded-Encrypted: i=1; AJvYcCWzrjK277rG6cQyDiaRfK05LfICrXsvc+XWdPuo5QAaLiUNd+X3FAzzyxiqrBgc90gtar5Rr7ayFJqvOvUIt3t07qs= X-Gm-Message-State: AOJu0YwVPqXI4ggSvBiB2wS6nZK2oDJV7NvbNu7qXEeIE3s5v4Q9xOlc d185Eig4Zm7go0gbttJC7D8ykz6u2fphOdOqfjYOWbscfmt1aNk3l/3oErjDtS0F0khDWEOPDMr GbVnC5IxRFaldbM2PxP0GhlQ4H6A= X-Google-Smtp-Source: AGHT+IFdFWbMxuNMt/eEaKf2Wqdoj6hgK4E48dPopsQNW+phuc0U6MUfTSWDkx6StAauk9J7g8OjclwwiEeJ9nbe2kM= X-Received: by 2002:a05:6e02:1a09:b0:363:d96f:6850 with SMTP id s9-20020a056e021a0900b00363d96f6850mr11605169ild.12.1707763999051; Mon, 12 Feb 2024 10:53:19 -0800 (PST) MIME-Version: 1.0 References: <20240210-zswap-global-lru-v1-0-853473d7b0da@bytedance.com> <20240210-zswap-global-lru-v1-2-853473d7b0da@bytedance.com> <900cd5da-da96-4107-b5f0-c7d975a8ba97@bytedance.com> In-Reply-To: <900cd5da-da96-4107-b5f0-c7d975a8ba97@bytedance.com> From: Nhat Pham Date: Mon, 12 Feb 2024 10:53:07 -0800 Message-ID: Subject: Re: [PATCH 2/2] mm/zswap: change zswap_pool kref to percpu_ref To: Chengming Zhou Cc: Andrew Morton , Johannes Weiner , Yosry Ahmed , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 69cbpajd9drnn65j53bwjseu5wx7hjj1 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 0E45718001A X-Rspam-User: X-HE-Tag: 1707763999-55891 X-HE-Meta: U2FsdGVkX19Tzbc04hmmdZF3tuzhZpyizaCnt14EhJ9onFZIuoq0pFgLIxbyCaVYvPuGuIL6Cyi8MYW53juRb4Xl7x52pjHTfMmyhUXdkegduLvZY7ezkUXhHgE3xxvJiBXmj5KsQ8YEH8BFHAMO1FoQKvomV8u4unl9ey9kSnCEWNDYS0KVZpjvsEDTdvCtBjTx+tKGIa/ZNFKeqLD1Er5zRRsvR9DnC7vxIb88JjXIecYHN2SR8gh/mMUWfib+YbAIHZereem0TvOKb3pZ4b3fI4Jh0hDQR3+DHpYGFHPWEOuZQiasngsS8Cx+DcBT1v4abidqa8Zc+1jyzHfXwSwPcSkQ0IntwnUL1d3XTiMYmlvfugcR26QQ1S8AAXInwHPLflySpDogyglQ0Scsur7dva3x16Rlu831f9wc6J7KapMp+G6RL2l7qCyPRTIzFA9JmZ19G6zsYXp94HZlC0slCZinH1Z4E98T0aE8AfuWAGOs+m4/TsRj1ftLbh2/nKDbuZjAgeHZE7+FixX9yv4FxjJMCG7oy77OTTRISZdaVv5bMMg0ToczXPmQeIkkpaX4iBzSvAPVT7b90aCw0qJOMsvYAyN+iX/Rzqh3F5yhYa4x/vdzhvOtihwuUpNS9TxZQ1Mv0yki2YNOuyy7vI+MtaWS2r097HqdOC57wJqf3Jr1suRjxtGQd7msm6RO/1Obbw2XfOOgHA9Icd1Ng8Fp9keH8x5faLoh/ucmO7Q+8CjyOkV+tVMWGULLGZ2ncwmrcpZERQnAlwku2Whv/maU6kwG1mduEeuL9yt7JwyPB+slS6K9Nv4xHy2bnk/leaABeEOKNANmU2G2Bie7a2jDqGbVNNE84Bvu3tBAyA+ANgduYnSK9XmLhc0DnRWzJEERIrvElyf9dmMP6O3U28C239etcor7+NY5reVlX7APUR2AeNv3KKh8TMrzp1FrpPediI+i3Yynfb+uGde lPe1m9LP 291NE80hNdXNVvh0W77tMdjzydc0PPfC2goKQut9axQUNh3z2zm7YzD6BBER7NcBrcdltpZP6DpDWx/fR4fsTcaE6m8yEob9WuYh7sMx+sr2pWGzU9e+Hciww6UUNPpRgOBeYecQ9y+8N3tgn0Jd47BhvtXalGA0yd70Kt2JRcDUzqnwluGsikmEiTLBb5Sl4qIe+ESNkP0DddgvdlOZeGMrxh64yXlcpNwGXon1Bl/EQmajJCySnuT2IW+bhG04CPvcKJYrMibz1vi+tcif7LV4YIOkZC5UXZ1gCZnchXA/uOGIr5bUAGeVLg7ba78Jrc7WBuLHuaU3IE12eca9r7tmGGAWoTip1JnAjpSfJ3caUw4LMmhwsYU0rvGNR3dVQjMT+wkJxtnQGSs/+W1j+iYECrjvgwfN6Ft6sBuARJO2tdgExKEfmLDGi9C83eJDza36KzNKvFWp8fkPQsdR7omhshk9JKziTAMYoDk6BjzZEQiuqdj/ZcNO0qA== 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 Mon, Feb 12, 2024 at 5:29=E2=80=AFAM Chengming Zhou wrote: > > On 2024/2/12 05:21, Nhat Pham wrote: > > On Sun, Feb 11, 2024 at 5:58=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. > >> > >> Testing kernel build in tmpfs with memory.max=3D2GB > >> (zswap shrinker and writeback enabled with one 50GB swapfile). > >> > >> mm-unstable zswap-global-lru > >> real 63.20 63.12 > >> user 1061.75 1062.95 > >> sys 268.74 264.44 > >> > >> Signed-off-by: Chengming Zhou > >> --- > >> mm/zswap.c | 30 +++++++++++++++++++++--------- > >> 1 file changed, 21 insertions(+), 9 deletions(-) > >> > >> diff --git a/mm/zswap.c b/mm/zswap.c > >> index 7668db8c10e3..afb31904fb08 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; > >> @@ -303,6 +303,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 *compres= sor) > >> { > >> @@ -356,13 +357,18 @@ static struct zswap_pool *zswap_pool_create(char= *type, 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-= >node); > >> error: > >> if (pool->acomp_ctx) > >> free_percpu(pool->acomp_ctx); > >> @@ -435,8 +441,8 @@ static void __zswap_pool_release(struct work_struc= t *work) > >> > >> synchronize_rcu(); > >> > >> - /* nobody should have been able to get a kref... */ > >> - WARN_ON(kref_get_unless_zero(&pool->kref)); > > > > Do we no longer care about this WARN? IIUC, this is to catch someone > > still holding a reference to the pool at release time, which sounds > > like a bug. I think we can simulate the similar behavior with: > > Ok, I thought it has already been put to 0 when we're here, so any tryget > will fail. But keeping this WARN_ON() is also fine to me, will keep it. Yup - it should fail, if the code is not buggy. But that's a pretty big if = :) Jokes aside, we can remove it if folks think the benefit is not worth the cost/overhead. However, I'm a bit hesitant to remove checks in zswap, especially given how buggy it has been (some of which are refcnt bugs as well, IIRC).