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 130FFC4829A for ; Tue, 13 Feb 2024 14:22:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 561FA8D0011; Tue, 13 Feb 2024 09:22:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E8A48D000E; Tue, 13 Feb 2024 09:22:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 362978D0011; Tue, 13 Feb 2024 09:22:50 -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 20E178D000E for ; Tue, 13 Feb 2024 09:22:50 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E874A120BFA for ; Tue, 13 Feb 2024 14:22:49 +0000 (UTC) X-FDA: 81786996858.11.A79F63A Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) by imf01.hostedemail.com (Postfix) with ESMTP id E9D7640015 for ; Tue, 13 Feb 2024 14:22:47 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=CCch7yP2; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf01.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707834168; 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=yoOD+hWlWE/17i8/Lqi/FjvHCDIa9+g3UCcdDeLmuOw=; b=IPh7nr33Li08+c99uq8yyHFOEhm1olSlvbHtwYIY2ayp1V3RN3z1jniXpiTyyi/R8I/Lxm h8yd4D8kV4E1eXWEeAmcDoVQFw4tqKXKfuRZu7cp98gRl+3levAvvJgfgljV70tfFX5/sU gvos0PRpfasBMKm15Sg9vx1LUqQlONE= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=CCch7yP2; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf01.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.210.174 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707834168; a=rsa-sha256; cv=none; b=iWUxaNQiA/1EjCY+FP+g9xh6qyDjsOugVhDm91pmfnchsL1DjcywAl9mKl7rWaMN+TL4HL HGpCZBHMxUtWbegHEjAdc+BZl32cEAVADRsImvlnn23CY7j201l8viFzTsokdDCV2yEl7C HrEEGYOLNUaeWHK4hP1z7b5/sdZSLHY= Received: by mail-pf1-f174.google.com with SMTP id d2e1a72fcca58-6e09493eb8eso2722201b3a.1 for ; Tue, 13 Feb 2024 06:22:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1707834166; x=1708438966; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yoOD+hWlWE/17i8/Lqi/FjvHCDIa9+g3UCcdDeLmuOw=; b=CCch7yP2MUkf0kRnPrRQeyi9D2znmsaSoS0giBV2CfsXirigYNxFghEwWooBtiMolR cfm6hcnqDNosH9Vqwy2cR0Cp8cGga9TSeTntg/LSULreZ6B8jG7jvlRMqtwjCDgh687e NnhdiAFEITwGXsnHtI/IEV1YXgK2+2qXBGBxl6Vy23TdyQ64Eop11kDBe2cvtAmW4dH9 L8CMs/b1g0tTyzJBZFs1F7bMod/qXVpc0M0VFuA5+R6a5J7PIRHr2KDoG8jFDNQnG+OZ 6zXmyB6DC7mGePKz3jIPbAFg4Ga8KiJcLLyiOg9C3WSiQ1knA42wjkWJ6Ypxd0os5tdK JBJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707834166; x=1708438966; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yoOD+hWlWE/17i8/Lqi/FjvHCDIa9+g3UCcdDeLmuOw=; b=GoEpTiL54aEUzATOJWXumP5BMXnB6RKOnKFTJzTHN7kZirtB0EqdSmpoyULhur04zo YbW4zAbBELVpjpZZU5dZeeOsZF7ZpoHEO9O/5PdUNnHovy2oqE0WQ48kvcjD5qDuzWB6 OzxHQX6pqbUCmuetV6Wp+SGxrjmJX/ZPvOhc5h9xiXUgGQhTJRObMW+QkBZiW02Iinuq AO7UQA/oHb+ryg+cRDe6m+GHVwyaeEtdACz7EgynIVNCdmfbDrILirPigGX5c16c3bFE srezbcoyaRUibDEKia/pOubNikdY+7V2xdoBLMaShIe3zesZd4YwcESxopIARs9Gq1ZC nvqA== X-Forwarded-Encrypted: i=1; AJvYcCUgz8ozuj4mPV0h0+RKkPX9TFN2nIHLErC5FrUUpuVKph3+8XQ+bEYEiSE8tkixduLILXxbsRX19y0fw6U/ypqL7Q4= X-Gm-Message-State: AOJu0YzOk3FyBUc7OMJNR5gxMFNgJ1xboD/lcc6QXIvFtmpYeNIFP512 vsY7mjp8L0Cm6QLh9Ab3TfSWb1IWAqDH+q9i+XCiilhVqlpCQ5nHDTMt3/jAj/8= X-Google-Smtp-Source: AGHT+IHFb+hFkNqYKVSW8GLr5IBYjAxPORH03FsRoQfyMkPGmpev9pa2Jb48PJVDq1OtWR9Q55jyUg== X-Received: by 2002:a05:6a21:998a:b0:19e:c985:f9f3 with SMTP id ve10-20020a056a21998a00b0019ec985f9f3mr3105250pzb.12.1707834166638; Tue, 13 Feb 2024 06:22:46 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCUmez5g8ORnZeCquaDa/pZEmHRpwGrwjXKMqaErxTDvZ007StaGvM1GQU14+e8TOcPHNYo8vnAQxxwDsyhXkEMqPooTRUEWiHvamrONe/ICFwtA5koW1vrBLKNFMFgEWPsCCODQp5o9DiKVbZddgoFbhlflFzPlz/jchblMgiunfp2N+C5zPMSOcT8u Received: from [10.254.125.113] ([139.177.225.244]) by smtp.gmail.com with ESMTPSA id ca29-20020a056a02069d00b005cf5cbac29asm2142042pgb.53.2024.02.13.06.22.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Feb 2024 06:22:46 -0800 (PST) Message-ID: Date: Tue, 13 Feb 2024 22:22:40 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] mm/zswap: change zswap_pool kref to percpu_ref Content-Language: en-US To: Nhat Pham Cc: Andrew Morton , Johannes Weiner , Yosry Ahmed , linux-mm@kvack.org, linux-kernel@vger.kernel.org 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> From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: E9D7640015 X-Stat-Signature: z3fybb3awboyixfpkpqsog5ym9hwc5x3 X-Rspam-User: X-HE-Tag: 1707834167-61163 X-HE-Meta: U2FsdGVkX19SjPvv55/dD1x1UeXYPUCmktoa6SAenLYM66FiFpud9ewvqQ2EiJrnbCcE8xRhI4yPT+Iinehq35beKFsA64zmzYhbZBLbn9Ar81o7/AHpNws+Nru8zb4rutY7cQKxfL2wcayWMck2iv3OuxI9WRivtg6LVhLl+VdbbbAikHRw1E0JhktKDrYR8j0/bhzx1oaCFFWquVQ7fF9rPL1r6zLK4+pY8QFP/3WMUx4C/6t5TYKcz+PzzBKlJ8brbSmbm5n58Uj1J29aAdBR8x5SPpcckl1W2KetCDl9IXTUdOmQSZsC3FVSz7C7o+hg1MkbPjwHmHnbAjm6fNAuTLRi8mfVmlLER3dOknEKNNRxR6Rm24oOFBELLX/ZvwBim2CkXreD33hYnphj+fXEV1EFkXQyZOV+rdqTREIWzaRrgXDRNjB7iFGig5JR46fjlJHX8LeGgbjtu6nIW8KK0SvK1lIDIgKyX+2x2z4vpKKm2L6e/XVQJsWGiG4HSha2eDVSR017loohf1yIh20AU/35umscmr6v6geFQW3DgSuXnHWD/fjhG9CQyDOUyRj3LAijnxZOO8N27dZxrVN8s056U4tBKiu5NDrhsz8zQmQnMhuHdQnJDWaDVxpq6BqtFRNdQkdc1g+RAYBRViCGQ+OpwLU0VylIVF6XkKP0HsvXPVWeIDdrRJ8YH8uuORU4M5lBCvT9CFNDjy2pYgF9uXheNxU3iC+Lw9QQ83LzKtj2a2KlftEyF1i88nGPTio8sP2XUuulBpAYHFVbsW24dxuhuPoEo4CSXNdyyIGTO9wycuoXj4iygAywI4gMlnT//ILjWrmcy7nfPs/2ljcpNkUNHyPX9DO7EptqrCc0Z680284Da2GKFRTVGDltw5REqAOS8vJHMyCUnv3UtgiFCLsVrQnybeati5dQQJ4K5LGRob9JHZAV0HaKTESacFDzE7sXMFM9hzMAwht MkHjxxXe wBUmMJKz5rGrIgMCJLf7fWLkY2ww1HNzkXVrVVh09vXWzwTdjoFMNY+B/B5T8e6+y5efsD3ashSTzdogZZtjbbq8sjblW+ZBvP4rtwdQNQBYn4wAr/i4/syPelcVHhgp/8IqsR7on3SK6j8xW6IJ9/Z4abevOSpTMRP23PIpVaJYkADnawek24zMVwudy1ofYefd78jw23McYdkQkVy8+gB6mxB3hmL62qYPTZ7o8Em+810qdMXj5HLGJ2rO1aEkzXXedn3zp+Tk3in0SIs1rnuoaH7YcJ/8pczsf28lbA46BvGZ8O7gS4Qxtc68WDeGyaua9PMjm4IMCSFzPFUrgfYgJGmGUEVMYsQ3V4ZXjf7EPznnu/p/gf/c3No3U9M1XQdz4mz+q2+Ls9rDoLVRzAyue/0bHKAT01ZQ+ajGziagua/6kIuJMJHmY4RtFVCt+WrIml/AA7eGvOMfEYfKZYGd5B3FD/Oy899PzCqyn0sBJFbA2+u1R+JjUgzrLJCq9cSmqh9B35TSElDIOaWDtw+n4nXj9D/VRHgEvn8cxE0v4GY0OrlOQAi+ij2ZVVuBitaH+ 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/2/13 02:53, Nhat Pham wrote: > On Mon, Feb 12, 2024 at 5:29 AM Chengming Zhou > wrote: >> >> On 2024/2/12 05:21, Nhat Pham wrote: >>> On Sun, Feb 11, 2024 at 5:58 AM 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=2GB >>>> (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 *compressor) >>>> { >>>> @@ -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 = 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_struct *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). Yes, agree. It looks clearer to keep it, which should be no cost at all. Thanks!