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 47A02C47DB3 for ; Thu, 18 Jan 2024 17:15:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A2FB16B0081; Thu, 18 Jan 2024 12:15:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E0146B0083; Thu, 18 Jan 2024 12:15:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 880D26B0087; Thu, 18 Jan 2024 12:15:58 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 75D9B6B0081 for ; Thu, 18 Jan 2024 12:15:58 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 15F09160D60 for ; Thu, 18 Jan 2024 17:15:58 +0000 (UTC) X-FDA: 81693084396.29.E0A00EC Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf25.hostedemail.com (Postfix) with ESMTP id E11BCA0031 for ; Thu, 18 Jan 2024 17:15:27 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=wyaYM6FM; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.44 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=1705598128; 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=lN7vV7/dmKAYamaryRL+nfbnN17C6XVKsy/jV+DrFZc=; b=IxaS0J30VZNN00JmGYT2pXtrs8tKTsboXpz/6+lyD1CL6YkW+aUlvtMFe9RLdcV2k4p4vD B2A0CFyO35T74HP88hdry/vKgtqcm8CCoMx5G1wrcMuFihDXO+vOn8Gm3YF+1obRa2evS/ lg6doxB7fUpSOlA8qw6aCxh3pBSTfv8= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=wyaYM6FM; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705598128; a=rsa-sha256; cv=none; b=Lv+uGDQtYz74izqZqgk2eFvAWOA3TM5hyR8gPPK4Mt54s2UCQaYoSKtzMgmcRggrzpLwgU dw3yYMDz9+3Y3711glg+YtkBcrczgqUjcybbz0kog8R4t70H2BJv72Flk6bHC4zQd3o4S9 V5a/6+FfL7070559ceWcBIONoh0hIac= Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-559edcee47eso1874630a12.0 for ; Thu, 18 Jan 2024 09:15:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705598126; x=1706202926; 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=lN7vV7/dmKAYamaryRL+nfbnN17C6XVKsy/jV+DrFZc=; b=wyaYM6FMnz+8QNNJ6RD+VRIr5p6zg6uhX+Fky+7lcto3wSL/JMGsEhmeVMyBpIc5EO gZc72isnC0biC8V0PPQJSbn3j6v3hob+wmzLhrspxBpjeUsYOBdptLSJkySI5r93XUjS E5stZcqae6EFg9wtjy57iGmhmvm5GoRrorKBtkAoo8FgGijElZffR1Ptri7QDenu56Q1 qLFMGBEteNaQXy07Pf8c8GY7+bgplEws4X7nLqPQ+GPldQZv1DiM0N2Ssrkg9o3+KJXR v4Vh3/6f5/0PHRRupJa6dA1QOne++y019vSIFYK5j6CAlhNI8FI8cjhReJqmon4vTW5T 6v6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705598126; x=1706202926; 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=lN7vV7/dmKAYamaryRL+nfbnN17C6XVKsy/jV+DrFZc=; b=RIGRLbHQ1CXktcoJjzscvES2+c9ZGBS5fp41K1Zk9jnCkWLTtjguSVcteOkbk4rzue ybLnLedT0IJvL9vpB/fYXmhv9XxQPKzAbobkacg/1k1PwcotrU0+AZM+qNGDy643+KZJ MTOQ/lOSA6nRVMwd3aXN16ECIiTf45fnvIPEFy7b63CTGe4FCkpduJ6PbrXq4DRBVe/N NzShC8cgvZ1fY33ad8a7IxdpzQ8bYtEEw7dygUlp3RfFzjVcs0ryTGo3Cm2fHBeomn97 5/4eFDYtMeC+rK1FY+q6ozwCKWKkhX2dO+zEs5umkEUjAuc+AKBcQU8xgc+QQNz4/Jd+ YrZQ== X-Gm-Message-State: AOJu0YxE5ExvBr7cX9MKQt/lwmjyz7OISPFf7eo7BWkHvmZwGWfL5ZC+ IuURnbp0MeTn9tqts1qHKw2+I8fU8jcB33YY1V70q+eF0DunHJWea5+KXFsL9LKoAjQH3PsoVJI kfyNrpMz4eHo9KekihCCx1WS07FhY9W1mVL/X X-Google-Smtp-Source: AGHT+IG4BVJJDWSpoIZFOi88PoJ7B+Bkq7LtBXjIJS3f7jogdS0H1Qb6OAuTw9O4Ar5Gz25tRxJegyW2JcD1MBHudLg= X-Received: by 2002:a17:907:948b:b0:a2c:3d75:fb7 with SMTP id dm11-20020a170907948b00b00a2c3d750fb7mr935754ejc.4.1705598126089; Thu, 18 Jan 2024 09:15:26 -0800 (PST) MIME-Version: 1.0 References: <20240117-zswap-xarray-v1-0-6daa86c08fae@kernel.org> In-Reply-To: From: Yosry Ahmed Date: Thu, 18 Jan 2024 09:14:50 -0800 Message-ID: Subject: Re: [PATCH 0/2] RFC: zswap tree use xarray instead of RB tree To: Chris Li 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-Queue-Id: E11BCA0031 X-Rspam-User: X-Stat-Signature: fhuzmerk3euxx96zequn8isde3qopkdd X-Rspamd-Server: rspam01 X-HE-Tag: 1705598127-210115 X-HE-Meta: U2FsdGVkX1901qA3bBAyMRxrLBAHoWkm5loQtF6Fvgo5rSbgTNSgezcdfkXmKLDTjc1i11JXdnZzBqcyRRJzfdADlXFEsJczvj6ds8hyBSdRFox67pOmyomuA5JQmy+VjkrolUXqibztV4Ajwpf3Dk1wlxTxebpN2fuW6EWew1wAwIcV5SWk3xB+wtmDEllH4K+MFuJD1QsgDlXFkYfeJHcfxAaHT9agw7R8rlMtWEJ2Kxz4t1GayvjDCcity9rynyfo3lzUDwwwnp18rW0qknErQzILdV3Fl3CsgB0u5BdIfueMizAyE7i90Ebc0MIwMGOZ1f142rGuRg9hIcTdyvtGOpzsiy5WlxlzfgwUKu8YM33LdDrdudzULXzHK3bkhNAGDfQ7TfzTbyrz5+783UcOZnL5WtLNuMCShzjzSVr5h1CfnHOj4J5VjNvmMl2Bw3ASfIMW1RY2954nh8kOqKDipYfhCd5Q9nx2OH3vfMD8iASFV4lplaTBD33mMWcGLDjIVXfPr7mrGK9JpmdljjdMBzeZg6xRLS/R6HA4vu6qBj/+OlvJ10NwGyJ8TMrRQpGIlCWpKTz6VmwDq4xaAFVDelb9rKjmNi7bENuGV5ubkfrb+ezP5Ac5S82UxL+uMUEVtt8a+RQd3BPvvy6UI60ZLDgqirP8tnnUHXeQaGil8PlsPdw4OoZPYPhPGTsOdScNppkpVug4VzjDOzpW2oGpdcE9D8NbIlkpl6r7gZmlSBmWBD2SBm4vLHfPMl8UPZCaNzWGHsLCLEyHAu/7QkdOv4BTD672eoAzhLr/SAk0jRRVAAQZxiuqS+2huocFtU93SuY1wygh+h1r41Bo8h+8636uURgOZwbSqulBtY8/913x/YESoStRN3Btk3XbKQBlzZSYSIfUxsjuPIBV+nfWw/Qe9FddrqsVFkw7RxaxzpQQ6z9n26zgtu9BF60TryDKaeyfBBJ5/rL+lOf mKeLiri/ 6Z4UUZSy42JF3ZU0dOcDGZ2gxFhz5TzXFV9o6er4RQ/y+w/F9lWvjbpBZtbgiVhlwByUS9K+9LzyhmISbS9S8JMTimrGPhLWkB29b9m5SLxk9LIrlVkC+C8Pb4q6LLJL3Xk9cS0HlHRS55y1aiAeN8Z34FVEuQslnvE1Nu/GS/pBzNbsF6rtr7LtlJASsojX9guTAKPvunqjw8/Q0aOa82+f8mcOCZCufXUXm35CwrkBrOIrkrVSmu4p4Wg== 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 11:28=E2=80=AFPM Chris Li wrote= : > > On Wed, Jan 17, 2024 at 11:05=E2=80=AFPM Yosry Ahmed wrote: > > > > The name changes from Chris to Christopher are confusing :D > > > > > > > > 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 bisectin= g > > > to narrow down which step causing the problem. I am fine with squash > > > it to one patch as well. > > > > I think having two patches is unnecessarily noisy, and we add some > > debug code in this patch that we remove in the next patch anyway. > > Let's see what others think, but personally I prefer a single patch. > > > > > > > > > > > > > > > > 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 th= e > > > > 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 par= t > > > > of this patch or this series at least. > > > > > > > > Am I missing something? > > > > > > Currently the zswap entry refcount is protected by the zswap tree spi= n > > > 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. > > > > I think this can be done in an RCU read section surrounding xa_load() > > xa_load() already has RCU read lock inside. If you do that you might > just as well use some XAS API to work with the lock directly. RCU reda locks are nestable, some users of xa_load() do some in an RCU read section, also for refcounting purposes. Also, I thought the point was avoiding the lock in this path. > > > and the refcount increment. Didn't look closely to check how much > > complexity this adds to manage refcounts with RCU, but I think there > > should be a lot of examples all around the kernel. > > The complexity is not adding the refcount inside xa_load(). It is on > the zswap code that calls zswap_search() and zswap_{insert,erase}(). > As far as I can tell, those codes need some tricky changes to go along > with the refcount change. I don't think it should be very tricky. https://docs.kernel.org/RCU/rcuref.html may have relevant examples, and there should be examples all over the code. > > > > > IIUC, there are no performance benefits from this conversion until we > > remove the tree spinlock, right? > > The original intent is helping the long tail case. RB tree has worse > long tails than xarray. I expect it will help the page fault long tail > even without removing the tree spinlock. I think it would be better if we can remove the tree spinlock as part of this change.