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 8EC2BC4828F for ; Sat, 3 Feb 2024 05:09:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 246606B0075; Sat, 3 Feb 2024 00:09:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F6016B007D; Sat, 3 Feb 2024 00:09:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0BE246B007E; Sat, 3 Feb 2024 00:09:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id F06D26B0075 for ; Sat, 3 Feb 2024 00:09:55 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id BA7484017F for ; Sat, 3 Feb 2024 05:09:55 +0000 (UTC) X-FDA: 81749315550.22.DCD7272 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by imf16.hostedemail.com (Postfix) with ESMTP id 010C718000B for ; Sat, 3 Feb 2024 05:09:52 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="WvYi/smG"; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf16.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.216.44 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=1706936993; 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=cmu0vpvqC5Lai0mTFBcrp18f/81rnOKK+LVNmR+nrhI=; b=DZEdTgSaSlW2DCL+DbYN5ADnVWQe2ngmuXJGt4G88TOkHRt1GR+R61Vy2BzLhOfm4rbKTJ w3TLaa77Pe+eeXziSRUjdY3JQCotntdTSAoWLRDcg7bFkWlCgmCNW4dx0IVHczDy5HVGyn f79Hr7TExrSlfG2WUHTdOTFuluRL1U0= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b="WvYi/smG"; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf16.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.216.44 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706936993; a=rsa-sha256; cv=none; b=Yg9qynmB9GYhuvaiT1rb3hdm4t+fMXrxeMoWMKB3CeAezGwfTdEL/mkQ9uydh0t2pxlAKR rEb5iKnjArg5TN8ik3k7bzvPg8dx7l0vqCRqttrVN5C7IHcChpnPYs5KoXl7nFMFHQ6XBC PnM3v467AH6dX0K4sNxhZOxSZkkuXIs= Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2962c769b5dso1604216a91.1 for ; Fri, 02 Feb 2024 21:09:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1706936991; x=1707541791; 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=cmu0vpvqC5Lai0mTFBcrp18f/81rnOKK+LVNmR+nrhI=; b=WvYi/smG79gTZp9fTh+iZeKNI/1T8M3PJjlCE7gC7LUMUZpfQJA8RmZMpcR4Uas+a3 +4Am4PpsT4lGswRqUpJayENycMsnOlittsj+j96XzupsJOjO3jxr5N1DzsSR4CLHVIuj e2oQRCrqqEPW26BUoAeNVgH38p2cW4cQoNSoy+3hWvl+FvlkKRTK34kh06vLn3wdH22Z /ljcodVFIocMoewenuy+YyiyqJ/CzqiY8d9UGkNlo44vbGDKX2RRrmM8pwdWwtypNe1X uI8r4HCDBKfUkmgndI+sP11kVWCx82efGFtqGtF3on/eWFpWDF27a5ieDez8IZmK7cmi 3vhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706936991; x=1707541791; 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=cmu0vpvqC5Lai0mTFBcrp18f/81rnOKK+LVNmR+nrhI=; b=e//apFD6myXF2EAOEalFvnD10KOKST7FYR0BHp32oYDvqc3+WxlQ1pcAEnZb4WH8Uo YO/PJIHP5UUjm89rl6fiIWgAVXT3cGOEZutk4l/i6aCQ9lP6Xq7zOdcC6oqnQ4wmlNau DvnCJMO7Tv/r+0c6MEY19BfxIdh1w9+0Woma34Gye6TzWl+Fbs2lKmCRIsOUj2YvnnTG qhUjdqraJddmjYEle8/RixGnuhoDD1qJ5rYErPom/KDdDEtXxo3Ypx/nPXjwNFYSxdSD jhazHh1zxjN1IMIQgKQnGpxsBhzY1PZAxWkk5qGKq7Fty/HSMMy+vOzsbHpErJTb6XQX uKaA== X-Gm-Message-State: AOJu0Yz6Lms09fCuB0lyqL3Z1mr8PKfEX8MsJuIdCy2RU9h1wRpNN2Pi mzyc5dkNx7VQzq2U3u9hNZC3/UeWk3J7G9KzVeP5fLF7I+B0Lwpqlkv2q3gdY7yTT6VuDyORM9R g X-Google-Smtp-Source: AGHT+IEgQJ6wiMQz4b0H0J2ytFxo0Wr/pgHZeyut0e5YLIK++zDDoPa+veu0Wo5vwp//4ugD7FsBgg== X-Received: by 2002:a17:90a:fa8f:b0:296:6229:aa41 with SMTP id cu15-20020a17090afa8f00b002966229aa41mr1024126pjb.2.1706936991571; Fri, 02 Feb 2024 21:09:51 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCWQLXbkQlA+wwYDgyyy1Ilfi0sg0IXHZ6Hir8OqaDzNjK7+6sX4fmiwiOAOZILsqi3+tjtCnrI+uShzhkM1GWPEPOhB6tXIpG0J9ACNVFwqQ9Y0NOc2qqbo4eA4cwA3ZNcjdF7/PYLxI9b4hd+w52XwCOxcd6KiIEYhJYvLG4yPQ3iSwtiJjUX1B+xCcAxirGOf2Q== Received: from [10.4.12.121] ([139.177.225.224]) by smtp.gmail.com with ESMTPSA id qn14-20020a17090b3d4e00b0028c89122f8asm950351pjb.6.2024.02.02.21.09.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 02 Feb 2024 21:09:51 -0800 (PST) Message-ID: <10f4fb80-a385-4c1e-974b-24057922ca62@bytedance.com> Date: Sat, 3 Feb 2024 13:09:46 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 6/6] mm/zswap: zswap entry doesn't need refcount anymore Content-Language: en-US To: Nhat Pham , Yosry Ahmed Cc: Johannes Weiner , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20240201-b4-zswap-invalidate-entry-v1-0-56ed496b6e55@bytedance.com> <20240201-b4-zswap-invalidate-entry-v1-6-56ed496b6e55@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: 010C718000B X-Stat-Signature: yqkpurpjndpd63mhps48cx76ua55kxr8 X-Rspam-User: X-HE-Tag: 1706936992-902024 X-HE-Meta: U2FsdGVkX1/nwwqAOnsoTn8iGYpdehamDSHgzbTmXj1HI9b73WoNJepp9GWj6m9s7yWiOjn77IXEJAV8ankiNZPTUU7dwTcodZfeeWkjUKioDWHOW6+gAQLJDDvM59N8oJ645JvaIDAgaX5qUDr+xuaod1HpqMuQfmyNHKaYbeyAuGnm1qxK+NGMG2tfgW+LgFSf6T/xDp2u+mo3UVp+uHgxQaa1x5LxdI/ITzdgijWkxwD19b0Deb5VHtkIJF9Pk3w62pjjKTgqOvNUjC6bFECnuDuhySEO4CrJ3ysZ/zl3FpmLfo8cqrN33hrLa0YpH9qkzC75ps6vNI3tVAra9MTbGx2oqARK58igwKtMChhn90umgchRy6ip3lKcovx2IiRswTbGxAar395wyjkBNVAdAsI2nqn/tQk7Dw4WQC6lb+gLUyvG2vH72gFO9ZnMF3plfZRiZ+P2tdrdxowtWJOU20uOvd4VQi3wxIjUkOVfhqvkBriImg8vYPeb09KnJ3DKNcHt/C5gnXLVr42A2kXpn0UH/eRiKppAedlqXuzZzPFdi9IiJQns/+VCi48b1pUtwbZRYwd/qOr22kvhVUkFnHFg+Or1nRK4imsJjZN1GmzGU20OvsWQ8yesIv7BnTsZ1n/4gqZn5M5F7egRC6mN0m6a1iQNJCOFsOaa15cOzPdE6KIYuPNz4hBzlEp7vb7NZcrVCRkm86LF1MTNR6TggJYmrwG5UuLM8Akw20vE+zPqrwWg3WuWzlWvhDXT8YaMF8NHUgAlJOOZR3Agk1gNQeDm92e/KXEnlf3EK5eYdqqtVJdPvkbXVdWz56WLV6Q/Ycmi6lG/dADNqMrbLuSXwguTRFclvaE+YZ4+zmeHiUUKf49bTdL/YijsBUKhfO26wLi4mrEq9HkbRLcH+Gn04/hmvgfYd6Y+lIueNCf1tICvIgbYKBKOsBkRS8iZCW6T8+lPzXEgGCMDxeh hLkGg+Vn PXuC2HAH1M7KtQargaXAbzVY2bOOkzQiQ4zg5yCGcdWAy97SpDZiMqOxqikFeKWPWYd6lS4qwEveFjB23Tq/QIq2Dp6jeJGEcOWbRAKqXjovCWKkP+m/fElCznA3LJi+u+ToJZpHU/o7JurS/TFsOQycLKoFo+Jc6vsey22MLUuXaLNput//wSZVKzE2qyiSU5/JFQYRr6lueDo7MzjXD/a5OU4AqQKwNH+seaH9HBGxt0srWvoNMR7gDbL4KkRA/eEAhi14ePSqXBsNYDREGbZMpSQTEXxw24XghHJg06+ACN75s9o2cAtbMMSrQ7fAQ+ivIv+ZjWfP0hqi/6usq/Coo1E4IfGjMfBxXjXrE3jbgrNWx7ZoTu4Jtuopqq6BBWvOu/j6yXQSC1astV3arhV/TqC2HIqkjUOTBNKyWQZUvkknwKAU3uQcsvCAzg67X0UOx28E78+2XFWQXNFLRG2+utbWrEJk5DBRxC9728+F49Pv1JAFVcc/zZFdKM/2N6eCo 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/3 06:44, Nhat Pham wrote: > On Fri, Feb 2, 2024 at 2:37 PM Yosry Ahmed wrote: >> >> On Fri, Feb 2, 2024 at 2:33 PM Nhat Pham wrote: >>> >>> On Thu, Feb 1, 2024 at 7:50 AM Chengming Zhou >>> wrote: >>>> >>>> Since we don't need to leave zswap entry on the zswap tree anymore, >>>> we should remove it from tree once we find it from the tree. >>>> >>>> Then after using it, we can directly free it, no concurrent path >>>> can find it from tree. Only the shrinker can see it from lru list, >>>> which will also double check under tree lock, so no race problem. >>>> >>>> So we don't need refcount in zswap entry anymore and don't need to >>>> take the spinlock for the second time to invalidate it. >>>> >>>> The side effect is that zswap_entry_free() maybe not happen in tree >>>> spinlock, but it's ok since nothing need to be protected by the lock. >>>> >>>> Signed-off-by: Chengming Zhou >>> >>> Oh this is sweet! Fewer things to keep in mind. >>> Reviewed-by: Nhat Pham >>> >>>> --- >>>> mm/zswap.c | 63 +++++++++++--------------------------------------------------- >>>> 1 file changed, 11 insertions(+), 52 deletions(-) >>>> >>>> diff --git a/mm/zswap.c b/mm/zswap.c >>>> index cbf379abb6c7..cd67f7f6b302 100644 >>>> --- a/mm/zswap.c >>>> +++ b/mm/zswap.c >>>> @@ -193,12 +193,6 @@ struct zswap_pool { >>>> * >>>> * rbnode - links the entry into red-black tree for the appropriate swap type >>>> * swpentry - associated swap entry, the offset indexes into the red-black tree >>>> - * refcount - the number of outstanding reference to the entry. This is needed >>>> - * to protect against premature freeing of the entry by code >>>> - * concurrent calls to load, invalidate, and writeback. The lock >>>> - * for the zswap_tree structure that contains the entry must >>>> - * be held while changing the refcount. Since the lock must >>>> - * be held, there is no reason to also make refcount atomic. >>>> * length - the length in bytes of the compressed page data. Needed during >>>> * decompression. For a same value filled page length is 0, and both >>>> * pool and lru are invalid and must be ignored. >>>> @@ -211,7 +205,6 @@ struct zswap_pool { >>>> struct zswap_entry { >>>> struct rb_node rbnode; >>>> swp_entry_t swpentry; >>>> - int refcount; >>> >>> Hah this should even make zswap a bit more space-efficient. IIRC Yosry >>> has some analysis regarding how much less efficient zswap will be >>> every time we add a new field to zswap entry - this should go in the >>> opposite direction :) >> >> Unfortunately in this specific case I think it won't change the size >> of the allocation for struct zswap_entry anyway, but it is a step >> nonetheless :) > > Ah, is it because of the field alignment requirement? But yeah, one > day we will remove enough of them :) Yeah, the zswap_entry size is not changed :) If later xarray conversion land in, the rb_node would be gone, so one cacheline will be enough. struct zswap_entry { struct rb_node rbnode __attribute__((__aligned__(8))); /* 0 24 */ swp_entry_t swpentry; /* 24 8 */ unsigned int length; /* 32 4 */ /* XXX 4 bytes hole, try to pack */ struct zswap_pool * pool; /* 40 8 */ union { long unsigned int handle; /* 48 8 */ long unsigned int value; /* 48 8 */ }; /* 48 8 */ struct obj_cgroup * objcg; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ struct list_head lru; /* 64 16 */ /* size: 80, cachelines: 2, members: 7 */ /* sum members: 76, holes: 1, sum holes: 4 */ /* forced alignments: 1 */ /* last cacheline: 16 bytes */ } __attribute__((__aligned__(8)));