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 EFB7CD0EE19 for ; Fri, 11 Oct 2024 17:54:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 674A56B00A9; Fri, 11 Oct 2024 13:54:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5FD3F6B00AA; Fri, 11 Oct 2024 13:54:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 477C76B00AD; Fri, 11 Oct 2024 13:54:14 -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 260856B00A9 for ; Fri, 11 Oct 2024 13:54:14 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 6F71BA916C for ; Fri, 11 Oct 2024 17:54:04 +0000 (UTC) X-FDA: 82662070386.29.F14D0FD Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by imf24.hostedemail.com (Postfix) with ESMTP id BBB16180015 for ; Fri, 11 Oct 2024 17:54:10 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=nDXUiPP6; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728669113; 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=NqgP/1pCCpSKuXinIt97h7bzJZWYuOT4pkGtBuCYT58=; b=c3qFg2j8SeVjYU1Uq2MGgdXeoNp6eJfyQmI5wK4yK8rZ29Gj46/6mHzDacqK2lQ7/VVu51 i21hu+QSmaeksPjxa5/u5X1idDNmCxAgGUIA7+nN60wKaqAPXE/f8QdOhZPuCvCA0QeAPT mFSHyg5ise9hvX3LWQdNORdf0VWpFns= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728669113; a=rsa-sha256; cv=none; b=YcsUvknOul8bG99itfGZZGDIUuo7ippY3uLY5y8JrU4NYifxB4xcUWCpFCJv/6kCqNASo0 4rQFYsPyG5cuj4Y2rfAB/kdheDYU3UZHclJMcq2geoWY77v58y4t9GJZTRugkvdpWSBWYi UMJh2kEI445/uKLIfpKJEZcbjRSXAPA= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=nDXUiPP6; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-53995380bb3so3205519e87.1 for ; Fri, 11 Oct 2024 10:54:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728669250; x=1729274050; 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=NqgP/1pCCpSKuXinIt97h7bzJZWYuOT4pkGtBuCYT58=; b=nDXUiPP6GLwwVKy9mY09LAAxwJv4T9RV0cdASbUxr0pnPzR57aV+X34zizpSmtPwGx gbWjqxUjSgw0TRLEg0veHIeCKXlkrx5vKezIKvzQ2rjLbem8mXmtYhGrV9Vu3k/Ll3y0 vnswQm/3EPnng6XsmumfdSR1NfS5SLYKMcZ0xcR3NofxRfqvom3bDkCdCQUbo3VrW/oZ gv+wksL4IUTjvbYeiNhhkNxcGFoCnB4FoErX2GHyRHD+v2Rwhtzrs47z/J3MtxDs+Kor CzONG28DYz+dLjQY5et2NUyfoa/uWpxtvoVRgtyIw7UTaR3MHESgIUqlf0eaAbsUqPjW CkqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728669250; x=1729274050; 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=NqgP/1pCCpSKuXinIt97h7bzJZWYuOT4pkGtBuCYT58=; b=U66tqtauRRmRwX+OKnUawandgHWnexlUGI/AamZJaixKouI+cUzEHt5KmVOsUdCc49 f/dbG3OGeudoJJ5hyGgmp5/FfU/HHZqVNnDG7itzTlyQyw+OQU1h1o/DE7P/VRZShYsq VNzXDg9VrTDBxwf90LU5WPyimwSOnHnq66yBwIK3y1ao5xp4uQeRJDiWKeDkyDOKjAO7 RBYKExWRDUB3Ar0jDaFbBC+KSWE5VcnfvIm3xG1YNvP2envYMq86H6ucDFZM/CYzX6bs fuLsTjWt4GcgNw2V82XmL/kdn2VXlnV7fx2rs1VGGH0Jc0umfQJRCJIbplkyXaMsK74j qOoQ== X-Gm-Message-State: AOJu0YxgoHY2oKLWwZZrgy9UZmO4Ezr4ak3PeuPcHM4DB75kDPqi3U7W u1YJyp0phYoon5zks1mDcptI7IpzzZFzuaZzqg2hYjYMs8RG19pRLqrjdOL/gTZRil9uLe4R+78 6bkZBh9JaPBiiXTxBOryyX0i10KBZI6NykFjn X-Google-Smtp-Source: AGHT+IE4/pgk3+N8inlVBOFRVaQm590gXmHKPwVD060RWGiTTrhhVLw22lDhsmZT4Xjhs4+zlSvTHED2cAY3FKDysOA= X-Received: by 2002:a05:6512:1588:b0:539:93b2:1383 with SMTP id 2adb3069b0e04-539da5661c8mr1996729e87.46.1728669249759; Fri, 11 Oct 2024 10:54:09 -0700 (PDT) MIME-Version: 1.0 References: <20241011171950.62684-1-ryncsn@gmail.com> In-Reply-To: <20241011171950.62684-1-ryncsn@gmail.com> From: Yosry Ahmed Date: Fri, 11 Oct 2024 10:53:31 -0700 Message-ID: Subject: Re: [PATCH] mm/zswap: avoid touching XArray for unnecessary invalidation To: Kairui Song Cc: linux-mm@kvack.org, Andrew Morton , Johannes Weiner , Nhat Pham , Chengming Zhou , Chris Li , Barry Song , "Huang, Ying" , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: BBB16180015 X-Stat-Signature: 7jynfze6hf159e8tw36ccbdjywr1s374 X-HE-Tag: 1728669250-784821 X-HE-Meta: U2FsdGVkX188Om+OsU3VkoJ/6FhhTGGIy7B0XiOkc/cN1wPpCIFYyPoirHyUOFgoXMZZfkiAtWK3Jk0YhuTxoG7ilShPuF0WTIxzjd0bj85yV8G7YIFIXgZ/CrBvWjywTZNyIbjYDHMeauqWE0ZlT8LpzmKxTh6Qo5nWB1QaoCNzDiH3W8JzsAKxAG5fAaq106GuPsDwuggfP9sD3vENJLoQi40/VDJjwo3xk8bdSEJLR30RTMDFQG/lTnC9KnwGCqbSr6Da+V/zCfDzaKPsdGZ/1LTGonp0FFeB+ed4aOl7xZzeukj8yIHcWIPnjWQhA9+JJG/Eqz3MbWLC5nOe8qCRA5/YNUT3brJbPDdmHK9kB9wg6OqykswSu46HKKcihFfJ3nO+MWw+o7LltDy5F6WQTE6aXt9bjfoPrSZtmM4/hWODOYW7i0EBfYhdazAy/v9ujUDgui0rNtzcrzRpj6IeXpTJS029OLp368mrJCSYsEYANh6E6l0NsEkyqBhGNl8jZd7KbbD3sxgpMeT4AvpSe2W93cxkyfEJXgQq7f3JOeIfMWYQvfruYeMgbue9XOwiP/4c9eZfENL4UatnKZ/P56UNigC9O2zmDGGUYCReii8mBf5h9zDDS6lGgWcUzUjIe0ctCPFmo253dJqopJI6YoPFQR/FpiD4jymwJ6pbfwKaiwvm+MeOvDDfMWBlseiu/XEZTa91g8s30IK7y3s9gNijqmXrXNEFyA5VubibFxfGnf7LxmMbWttuOPZVT2zBQHTyhz9y4hLtVpxfcY+PgqQSrfWlwDIoj1MQQu355LMpdiOAUmrWhV17oWSpdrStJo+PfZhdCqBajRXoZVD7qtX8aJY9aDV+4E1Yznen8Nz1mMmGHQqf8Dhl24vsN/OHeRy5AI8h3xJwVoPL27DTI3v0Y1NatT7iDEyjDfQ8+ahl9Crq0o3GX14hFbCxB7uxS1PyS+QCS8cO/j8 l3VWhrD3 rctymu6dCS2GT1gPAzOccbM6AOjf3CBeQ2rv0JYhtcNHDE2ZFlXdLl8sVp09Olx82MFJz8MlLc3fBLTHmesWiNG5kUA/4JYCzIiZ0m9FEHehn1X/2jykw7ZHsqiJtU0spMA96LsKXqMrKhDeNeMIiTJZ6Qhzsvzuotj+OM/hDkU03coZkHweMOsC/0ZRxcf0igue0toOV1wH8lFPGH0LCjQ/XuuqdDOq0kp0fRb+K6u63RYx3fOY85yL/CbM/nEpFuPMz5c8Dj8IEaqw= 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, Oct 11, 2024 at 10:20=E2=80=AFAM Kairui Song wro= te: > > From: Kairui Song > > zswap_invalidation simply calls xa_erase, which acquires the Xarray > lock first, then does a look up. This has a higher overhead even if > zswap is not used or the tree is empty. > > So instead, do a very lightweight xa_empty check first, if there is > nothing to erase, don't touch the lock or the tree. > > Using xa_empty rather than zswap_never_enabled is more helpful as it > cover both case where zswap wes never used or the particular range > doesn't have any zswap entry. And it's safe as the swap slot should > be currently pinned by caller with HAS_CACHE. You actually made me think whether it's better to replace zswap_never_enabled() with something like zswap_may_have_swpentry() that checks xa_empty() as well. > > Sequential SWAP in/out tests with zswap disabled showed a minor > performance gain, SWAP in of zero page with zswap enabled also > showed a performance gain. (swapout is basically unchanged so > only test one case): > > Swapout of 2G zero page using brd as SWAP, zswap disabled > (total time, 4 testrun, +0.1%): > Before: 1705013 us 1703119 us 1704335 us 1705848 us. > After: 1703579 us 1710640 us 1703625 us 1708699 us. > > Swapin of 2G zero page using brd as SWAP, zswap disabled > (total time, 4 testrun, -3.5%): > Before: 1912312 us 1915692 us 1905837 us 1912706 us. > After: 1845354 us 1849691 us 1845868 us 1841828 us. > > Swapin of 2G zero page using brd as SWAP, zswap enabled > (total time, 4 testrun, -3.3%): > Before: 1897994 us 1894681 us 1899982 us 1898333 us > After: 1835894 us 1834113 us 1832047 us 1833125 us > > Swapin of 2G random page using brd as SWAP, zswap enabled > (total time, 4 testrun, -0.1%): > Before: 4519747 us 4431078 us 4430185 us 4439999 us > After: 4492176 us 4437796 us 4434612 us 4434289 us > > And the performance is very slightly better or unchanged for > build kernel test with zswap enabled or disabled. > > Build Linux Kernel with defconfig and -j32 in 1G memory cgroup, > using brd SWAP, zswap disabled (sys time in seconds, 6 testrun, -0.1%): > Before: 1648.83 1653.52 1666.34 1665.95 1663.06 1656.67 > After: 1651.36 1661.89 1645.70 1657.45 1662.07 1652.83 > > Build Linux Kernel with defconfig and -j32 in 2G memory cgroup, > using brd SWAP zswap enabled (sys time in seconds, 6 testrun, -0.3%): > Before: 1240.25 1254.06 1246.77 1265.92 1244.23 1227.74 > After: 1226.41 1218.21 1249.12 1249.13 1244.39 1233.01 Nice! Do you know how the results change if we use xa_load() instead? Or even do something like this to avoid doing the lookup twice: XA_STATE(xas, ..); rcu_read_lock(); entry =3D xas_load(&xas); if (entry) { xas_lock(&xas); WARN_ON_ONCE(xas_reload(&xas) !=3D entry); xas_store(&xas, NULL); xas_unlock(&xas); } rcu_read_unlock(); On one hand, xa_empty() is cheaper. OTOH, xas_load() will also avoid the lock if the tree is not empty yet the entry is not there. Actually there's no reason why we can't check both. I think the benefit of this would be most visible with SSD swap, because zswap_load() will erase the entry from the xarray, and zswap_invalidate() should always be able to avoid holding the lock. > > Signed-off-by: Kairui Song Anyway, all of this is kinda orthogonal to this patch, Acked-by: Yosry Ahmed > --- > mm/zswap.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 7f00cc918e7c..f6316b66fb23 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1641,6 +1641,9 @@ void zswap_invalidate(swp_entry_t swp) > struct xarray *tree =3D swap_zswap_tree(swp); > struct zswap_entry *entry; > > + if (xa_empty(tree)) > + return; > + > entry =3D xa_erase(tree, offset); > if (entry) > zswap_entry_free(entry); > -- > 2.47.0 >