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 6F3CBC47077 for ; Thu, 18 Jan 2024 06:48:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0A4236B008C; Thu, 18 Jan 2024 01:48:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0541B6B0092; Thu, 18 Jan 2024 01:48:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E5E026B0093; Thu, 18 Jan 2024 01:48:27 -0500 (EST) 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 D5B466B008C for ; Thu, 18 Jan 2024 01:48:27 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A65314056A for ; Thu, 18 Jan 2024 06:48:27 +0000 (UTC) X-FDA: 81691503054.02.127DD64 Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by imf29.hostedemail.com (Postfix) with ESMTP id E786A12001F for ; Thu, 18 Jan 2024 06:48:25 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf29.hostedemail.com: domain of christ.li@gmail.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=christ.li@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705560506; a=rsa-sha256; cv=none; b=Q3gjAksUHr/6Wctv4IOJ9zMkGL3yzl2lVo3Km/GTAqozoNoEaqB9zogbNZdYEuaDkqF5Vl 9w9spmTLgYfEy7KKNdUesUDneKZc2iE0SOL1QC4O8lTdFUpAWiXlZmC6LBDNA3OTavYrCZ DB4RG/1MC87v+qL8ElOqQZLXE2qiN9U= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf29.hostedemail.com: domain of christ.li@gmail.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=christ.li@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705560506; 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; bh=8jauxg95i132eOZCuJr/hrW8/+eD3j4e3f3VPpTT5U4=; b=nYYgKZZPUMVyvxB+VGGqHeTr92sYO0fje8iTej/z6wicHvRyjt+nElFnokVUVH4GWiZ91F nQ3dfHiq4OUEPC5zx848XUBz4Y5dXxwF6ybheGVFlCV0j71MqvGuswgBZz52Y41XOmHr1z hkpp7OZPmBfSboEhbhRUduURpyZOH4s= Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-50e80d14404so354978e87.1 for ; Wed, 17 Jan 2024 22:48:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705560504; x=1706165304; 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=8jauxg95i132eOZCuJr/hrW8/+eD3j4e3f3VPpTT5U4=; b=D9LNxu3ZgXJZ5csGO2soASGkV3vbzvutVomUw5TpOr/oQbfWqzZtuc3vnuUFYMmm3S Zw2JfixeO3SB4D+l4+6dbz2tHa2pHuRfTw62lee5PphoF6wJntdbBtiSxHd+cMPw919H GLhSpd+cHcHox57ZItEXSZW5IYutiSEcesGHOHArbJKiHkG3W+6KpUJ9ayUh3baDWhzN UHAJIFyuHHfApgR8Rx72jwxRY7uDbSMc1MZpdzqu+ZItuDxc0SCDH8D/AlhuHhBiR+2R zdv7wrmeNPaBA4ZRxUFljD5visi57aQOL3NUy2lJ0sGOeUEdn6Ey3I0nzulDdlgmaw8Y H7xA== X-Gm-Message-State: AOJu0YyBFSswzqV7LasDMUDwD8Gf52GZaNNNrfKAHmxJ9ocTkEP8E+VK 9IFk2O8mB1422sqFMinabZhMYQ68KOXAAtGg9Ewhsyu+OYrdQH5oZCLmJwzJepHRWkrWQXJY2mJ /leRAyRgDxfZIwxaGnFjnZ+/xoA== X-Google-Smtp-Source: AGHT+IG/s7NbNdQFIeBlaCU1kPMbseGVc9UtT1SjMDdmYxpotmYqdS8MMRCcYq8pd1e50ALT7BqfOJgQD40bhxgY0rc= X-Received: by 2002:ac2:4283:0:b0:50e:ed79:d94a with SMTP id m3-20020ac24283000000b0050eed79d94amr246958lfh.30.1705560503950; Wed, 17 Jan 2024 22:48:23 -0800 (PST) MIME-Version: 1.0 References: <20240117-zswap-xarray-v1-0-6daa86c08fae@kernel.org> In-Reply-To: From: Christopher Li Date: Wed, 17 Jan 2024 22:48:11 -0800 Message-ID: Subject: Re: [PATCH 0/2] RFC: zswap tree use xarray instead of RB tree To: Yosry Ahmed Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, =?UTF-8?B?V2VpIFh177+8?= , Yu Zhao , Greg Thelen , Chun-Tse Shao , =?UTF-8?Q?Suren_Baghdasaryan=EF=BF=BC?= , Brain Geffon , Minchan Kim , Michal Hocko , Mel Gorman , Huang Ying , Nhat Pham , Johannes Weiner , Kairui Song , Zhongkun He , Kemeng Shi , Barry Song , "Matthew Wilcox (Oracle)" , "Liam R. Howlett" , Joel Fernandes , Chengming Zhou Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: E786A12001F X-Stat-Signature: uaumhptmfxn33zknq8yjkx7bhi9c8wx1 X-Rspam-User: X-HE-Tag: 1705560505-824446 X-HE-Meta: U2FsdGVkX1+Bdf/dsCf3rHHx+s7WBKhK3c+9RAeoN1FHURdda6guOdDrw2hQpid0rWFslWbh8WxMfqgWk1EkJHqycyxiZUnZXL759id9v0tuh84juru30ibS/ji+uZ9fvOUSIaRVoTHvieNZdHak3lv6pEgP7vPIWWY4csXwNxtVRnllfoemHqcwo9BXCbP3ntZZttAw4/0BK7ergSsQm42nyGn0AHmRxZBPn4dlto5PMh59rjwXaSmqwKNB6whlObKRaEJNoO4bk6nrFUUpXXqqY3IjpeYbMkPbRB5VobmbP3i3WtdnMzkQOOqGkanxLgeh+fl7Q8/sox8IxRIsF3cAzgi4xFtEY5LneMJQX908tdvfY3ng91hX0ontp2aSxZiAYgmwGxWKfFldYniKJRr87E8iOHCWs7ZtpZqF3YtZxuRu8l13rktiEO4aj6P35S1YRjD2JXD2UtA4Qv8n9RmEmvOVHcvFev0nz3KdvrjkChEHcYq04gTBHgRl/7mUSwX2w0tyjT2ihzejduDn3/+XRiQTucpUDtdB9naSOu4FVmVMUQl45N2lWrdlOzl/+n5fX+UO7Q5ywbmgzVF5mMrUgWAeV7MJ0ewGTi8OEBuJad+hnLb/cepCWeXGgkjv9wU6EPr+u/ZM6qp1FqIMkn6ZpRZ/029zJIvjFD+VkHRXBYvpEEObvb7C+QECzIe+kg8fu9ljXD+MactPR/bP8mVaxU/G04QZ4fL57bOIH9GeIatnZSKRC0yuaNeVf2CfwNA8sN2mpfQLPRH+7QWDg4xl6Vzxrke4bV74Zf9By5oZsL8cw4s2Q+AuGG6MbYPlpraSA51zci4wUpmxVJr2LQvxldGtbOHKvRKom6gdc/wW6YZ0RI1DO3Vd2zFghIsE3C7RI6LfKxsxeOsoJs106Vaqn/fR+ms+6mOutkq6HK4uu43HBcYh4Srh/wW1wUknvu3hNLcPxsfQPIIA+4k dNeZTpy7 ewNb3LltfQH+NPa2WcDGrv8UZ5tEBWzO7P6izNCWpCylysMAsvY2HcVrVZ/A8HBSyqgyF/NV0N4BO9eLmqK9vqFEM8bpvzn11NFGOMydN8vPstl5VR37W0iCV27wcFjNAyHIWjiV1UplvZn9x+Ju4yTRy21GSr4kupVQf1+sdgNtBFLBeQgKMSpfGeNRVD2K1E6GIslBMwQplDAlFJq0glEF+FPrmlbW6m7QBJQ1BN9rp77DCJafurXXu3dO63nBOTJ+rcp0mawUdFRkOtRS7hXyw4GVgtGRofKUGw/ODQM1aBBh+JNgoesTTZ3faC6SnELzp7v9gyCvizXJbEdPhZDAPtEeBtIjaQYQNDgtDi1LSQRNUADeYG8yiAcwJKHoEJxcBug9ObFQ+xZEhK2SZtEwZxw5syKX2upeoED+tKq+9tXC9GbaN0fMAVw/WiSZKVWALp5er/HRHAIg= 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 Wed, Jan 17, 2024 at 10:02=E2=80=AFPM Yosry Ahmed wrote: > > That's a long CC list for sure :) > > On Wed, Jan 17, 2024 at 7:06=E2=80=AFPM Chris Li wrot= e: > > > > The RB tree shows some contribution to the swap fault > > long tail latency due to two factors: > > 1) RB tree requires re-balance from time to time. > > 2) The zswap RB tree has a tree level spin lock protecting > > the tree access. > > > > The swap cache is using xarray. The break down the swap > > cache access does not have the similar long time as zswap > > RB tree. > > I think the comparison to the swap cache may not be valid as the swap > cache has many trees per swapfile, while zswap has a single tree. Yes, good point. I think we can bench mark the xarray zswap vs the RB tree zswap, that would be more of a direct comparison. > > Moving the zswap entry to xarray enable read side > > take read RCU lock only. > > Nice. > > > > > The first patch adds the xarray alongside the RB tree. > > There is some debug check asserting the xarray agrees with > > the RB tree results. > > > > The second patch removes the zwap RB tree. > > The breakdown looks like something that would be a development step, > but for patch submission I think it makes more sense to have a single > patch replacing the rbtree with an xarray. I think it makes the review easier. The code adding and removing does not have much overlap. Combining it to a single patch does not save patch size. Having the assert check would be useful for some bisecting to narrow down which step causing the problem. I am fine with squash it to one patch as well. > > > > > I expect to merge the zswap rb tree spin lock with the xarray > > lock in the follow up changes. > > Shouldn't this simply be changing uses of tree->lock to use > xa_{lock/unlock}? We also need to make sure we don't try to lock the > tree when operating on the xarray if the caller is already holding the > lock, but this seems to be straightforward enough to be done as part > of this patch or this series at least. > > Am I missing something? Currently the zswap entry refcount is protected by the zswap tree spin lock as well. Can't remove the tree spin lock without changing the refcount code. I think the zswap search entry should just return the entry with refcount atomic increase, inside the RCU read() or xarray lock. The previous zswap code does the find_and_get entry() which is closer to what I want. Chris