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 66E56C4725D for ; Fri, 19 Jan 2024 05:13:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CE1366B0072; Fri, 19 Jan 2024 00:13:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C6A236B0074; Fri, 19 Jan 2024 00:13:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AE3736B0075; Fri, 19 Jan 2024 00:13:27 -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 98E276B0072 for ; Fri, 19 Jan 2024 00:13:27 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id E147D4020B for ; Fri, 19 Jan 2024 05:13:26 +0000 (UTC) X-FDA: 81694892412.24.16FEEEB Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf05.hostedemail.com (Postfix) with ESMTP id 09247100012 for ; Fri, 19 Jan 2024 05:13:24 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=lbV+3zun; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf05.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705641205; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=6DMlZREdHCOCJLHZWzZ6wV69tr/ryDaj7HxEqQ5FeQE=; b=AK+jg9U3N9D++Jckke9Mbvby++dTOcedUFH8FMnBVDfhLqezSvihFH2Q7UNro9mSg8h7VT nK6412a0m0hF1/u9/WJ5kT9/5p7ttx3mg8FleLecGlc7DJMSJHRf7EtxuLwHzPFwuwrWKd OWt674vQWASgXbdLWpYSHSj2MEKWEN8= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=lbV+3zun; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf05.hostedemail.com: domain of chrisl@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=chrisl@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705641205; a=rsa-sha256; cv=none; b=VxAWfokdmLjwxZ6ipzrW0e355tP2us/iqKONOnmfMW2bBHWwMCNATJvDvwSMPGwxTCXETF V0Em/xzJ+6oWSIC/PIHFE7OVmDm09VqN+XD6/V/IxGCocngAH0glXP7YWgogC22CiDHbW9 igtp5oJ2lN+ZpINllrlZnPYhUNhYsJs= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 215E76190B for ; Fri, 19 Jan 2024 05:13:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BDC22C43143 for ; Fri, 19 Jan 2024 05:13:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705641202; bh=Ne6s30tyKXga+DbCWN5zpt/HH+395I1ynKLHxGT4RsQ=; h=References:In-Reply-To:From:Date:Subject:To:From; b=lbV+3zunRyYYnwhJj5srk2lhNvfqikcWIHF4w1ZuWXbq+wqCBaFZVLEhYT/3sa2kr 6xw3jg60ateipQEDDLhsS2NT8Hs+mBIcWs7rbE8+YyUFrw+jhJgA/fy7mf9kUGjlQi p3aCOc3tnSZuAmoObxXCEjzmlYJ8ZcsLWZ0KwSmiyw77Dbe6Qx9bOPW3xiJEdNp5xV lNPtZkEprRA5O/pwqfsby5/+vHL8SJeSjcT6gmdRZxOnYWgZRNH/wUBlYAYOhrUxLH q2/GFYzFN3Ub2KBbRQZtZM5D+MGvAVzr2jVDK2tCgPTW54eJVUK41JRwKm5TbZFyuo lvf2I8FoVnLFQ== Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-2902264d2edso355322a91.1 for ; Thu, 18 Jan 2024 21:13:22 -0800 (PST) X-Gm-Message-State: AOJu0Yyl0Gjuyso7sVvc3jevDGQwMWCeG0Bfh36xvvgK/vf9GopZF0zO nglziSDHD02Ot+pDKmog33gzO5jXC8uEwPsPSDKDoinQzKNzJC5KkPVzO6s89qkI7VE74C+ywfK 2NQlZx76pAjs+qycnjP9dEpHEM4Qo5H7riSzC X-Google-Smtp-Source: AGHT+IFbtKlJJd9U0YKEdyOEquQDg4knOOuf/VrFmAHmrTA0QnwX7UBqM7VSQmdcEeAO41fum+dshTVNbb7S+3SZTAs= X-Received: by 2002:a17:90a:c305:b0:28f:fc44:f61e with SMTP id g5-20020a17090ac30500b0028ffc44f61emr1597466pjt.60.1705641202088; Thu, 18 Jan 2024 21:13:22 -0800 (PST) MIME-Version: 1.0 References: <20240117-zswap-xarray-v1-0-6daa86c08fae@kernel.org> <20240118180959.gwqphdgmf7r5sdne@revolver> In-Reply-To: <20240118180959.gwqphdgmf7r5sdne@revolver> From: Chris Li Date: Thu, 18 Jan 2024 21:13:10 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 0/2] RFC: zswap tree use xarray instead of RB tree To: "Liam R. Howlett" , Christopher Li , Yosry Ahmed , 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)" , Joel Fernandes , Chengming Zhou Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: nb9jzgdummpwnumzk86uoymwm8n3caqm X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 09247100012 X-HE-Tag: 1705641204-524590 X-HE-Meta: U2FsdGVkX18SPBO4Al3lY8i5EpZp7XNU4yXFIZRHvoCOLgOF+NrBPam4JP+4C+vM6apPJ861AiL6uqY0WlqGficgcrlqnAlzr5NSven3PkNIZrIeVybLkCdWbyBsgcxOCUoMgnPPrta2P5lpeelEgkeo1FyApS86ynWbnFLZR3hN3h1M7zzmwWALbeGiNmHyyQU9+EAVm6JG3B6M29RxGRD5yzoPT46lvq76GqUfcL57ylabowZ4b3/KQyAEPWqLAS3WwnmVMtqjTuRaJsRvjzPb9lQ31ivqiGGjz2cXyQog0mn87owCnGZgLyqaKEQZjiIqwMuujovBQpGAAsrS4eE1Q1c52P3wPtFcNjXEjRPVMy2HKD2f9UkVb4HRdyYHmu8Y00tsvAo0SJ7XpGQraz+XtD3HzqF7YNbMSkI2GYexIiQxQQz8Uzs5G9R8O1C1+eH/wnOOYzlZroIWYRTH3c15OV7rFev671FmhvDJ6n1gAIWCkHzzOg6TFytJo8dLr+UkxArm/uGN07aS9TCXugpczrJjOAow/uGh8u1PNbXGFXAX5kFfhS9dzoYf2yyf/yA55gWvFJSBA6pWiHBq6+vJ0GL4Nt6MN/7mdfYRMtevMMtowDgwbxc6hN2YfqzbCz8P1dvw+DcS3lpFadEfET+RE4ZA+lv3IgxaogvBbR+XrPOTjUzaO8VOvuDkxXUcz+bVt7+JSuoLRCoouR1T46W3gMV2ZMWVk6A12rLV9EdRzdSplZzJtDQcdjGnjsLWb6nn8OBYZnYYQhXNYlhgyFyvCEVTCnHW0dLaAIQ2EUv4d7J9QgG2D/GE4nMom5U5MgSIFMFutXVPGQEhra7Q9ZpV9aeR/6dBmLIHOVHkA0OD+ImxGtqF7WFH5X43JLOFXHulVC4pHX0rSJdBCZAfhpQiP6SWEowhSQP6FoJ6l6qW80vzMMBvV5hpWxjKL6OUuULV+AfcTaFOrUERSYG 544ej6UM VUu7IfTaS0+iKyJ6ORGqXAEBT/QLWp5tt8G6I/tC+wgHJDzGifKPVNH7kGz1XoQU/eYPy6EO2pSviMUraro1xeZ2wXPEtElUDRhLpeCDoXJgqXYlA96SH5yLhtEwMlGOerbwJ1EmUQsEFxr395DJH0B0sgvuIasT5kZjc73kCVWUmWTX+1LDWhVVQASKFyQubWmTB65Oz6xwyk7DFH5R1hQJwAK0Pb8bfpxM0g+xEMZBeBom0GuUx1cOssQ== 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 Thu, Jan 18, 2024 at 11:00=E2=80=AFAM Liam R. Howlett wrote: > > > > > > > > > > > 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 had thought similar when I replaced the rbtree with the maple tree in > the VMA space. That conversion was more involved and I wanted to detect > if there was ever any difference, and where I had made the error in the > multiple patch conversion. > > This became rather painful once an issue was found, as then anyone > bisecting other issues could hit this difference and either blamed the > commit pointing at the BUG_ON() or gave up (I don't blame them for > giving up, I would). With only two commits, it may be easier for people > to see a fixed tag pointing to the same commit that bisect found (if > they check), but it proved an issue with my multiple patch conversion. Thanks for sharing your experience. That debug assert did help me catch issues on my own internal version after rebasing to the latest mm tree. If the user can't do the bisect, then I agree we don't need to assert in the official version. I can always bisect on my one internal version. > > You may not experience this issue with the users of the zswap, but I > plan to avoid doing this again in the future. At least a WARN_ON_ONCE() > and a comment might help? Sure, I might just merge the two patches. Don't have the BUG_ON() any more. Chris