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 E75B4C4725D for ; Fri, 19 Jan 2024 05:24:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40CBC6B007B; Fri, 19 Jan 2024 00:24:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3BD026B007D; Fri, 19 Jan 2024 00:24:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2DC696B007E; Fri, 19 Jan 2024 00:24:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 1EA0F6B007B for ; Fri, 19 Jan 2024 00:24:33 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id E3D97A02EF for ; Fri, 19 Jan 2024 05:24:32 +0000 (UTC) X-FDA: 81694920384.24.5B958F8 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf14.hostedemail.com (Postfix) with ESMTP id E6218100006 for ; Fri, 19 Jan 2024 05:24:30 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cQLfISwg; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf14.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=1705641871; 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=KteYCQ1LJOjfYMpx1yHyXYywQeCphWRgAkGfNYm82As=; b=Bw9lENWOjcg9dCwjenaFnth6hvfwVBTn0r7rGiYSedfDrhHkDcwN2dPlNBTCmOIptKnBxT Hw8/ARAiQkFiUzAqCRoWFI6tS+FQG5SvKnSOOr8FsEZYawfCeztd263iKzc9dsysADuX5g fu0qgYg4pO1VBjo4cJmVbs0FW4CHmAk= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cQLfISwg; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf14.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=1705641871; a=rsa-sha256; cv=none; b=rom5rFDzcjWjDsE0uHTLd/1qxztD85WoOWU/WlHpWDNiu7SmfC9M/mA96G5R9N1BT9DvCn 1ez6rGRT6hbR7JhGxyrMqcpfZ+o+pVFIQs4zeTVpTZbsZG46+eM7chJZuGQM8Fk7VQ6Sp9 1ZGM5ll0zIM/oAB4sN5rJY62I474b0I= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 2C87D6190C for ; Fri, 19 Jan 2024 05:24:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88B55C4166B for ; Fri, 19 Jan 2024 05:24:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705641868; bh=GZXH9M4vRYlIBKd2B1NBqugB5mUzmY+xYn4wpQol9Co=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=cQLfISwggyTHG7Wu7DGoNQtD4oeFqTAFtiDX8sdQzlDop8PZp5eUSB8mtNCV9TgOy 1KaUFy7UBDl4XFXk4VexgOhn+3+I2EfbYWX/+NfgLmaPhSmY53iCbttzrlrWYSwLbP tZ4ATuPP+jZSKRLtwp4NCqYAF/Xf2LTWilQEWQdVwU/ZHhpLX1jjAfTGIlisnDB970 sZkSyD+wq7i9oLMMS62pMM5hJhWHQHgiwKFRNPS1CbjTNra6wJWLQ/sS3jXrO+dyaa 9KT5V/KjK9dQHR20MtdtCATT93x1H3aK6ohSB9GrkKRlTtSZ66AVk5x2bcknAv7uoe liC2DbrXA7NOg== Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-2902b0e9524so318143a91.1 for ; Thu, 18 Jan 2024 21:24:28 -0800 (PST) X-Gm-Message-State: AOJu0YwSDn0wRjbge3ngZiqJrotaQUbRMmtbfzDRTs7zcxGPo2iGiQag W3tUa94Axidh+mCC0yeglCKCfQobDJo2pBJhivT9dkuBTJ99p4FcKUF3GvIMLPIsd1bdToCkg9a KzxUmNBEEY0F7FLR2qhVeul2NBFaWuGqRAgOF X-Google-Smtp-Source: AGHT+IE8GFri0v06WYstA1NMeuaRhD8fD8knkjPgfOePLZOOetmV4TfCs6TKuy2lMnpiYhZ5n76GkLIIdOlLn6IyHII= X-Received: by 2002:a17:90a:b00c:b0:28c:a722:c587 with SMTP id x12-20020a17090ab00c00b0028ca722c587mr1638769pjq.42.1705641867801; Thu, 18 Jan 2024 21:24:27 -0800 (PST) MIME-Version: 1.0 References: <20240117-zswap-xarray-v1-0-6daa86c08fae@kernel.org> <20240117-zswap-xarray-v1-1-6daa86c08fae@kernel.org> In-Reply-To: From: Chris Li Date: Thu, 18 Jan 2024 21:24:16 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/2] mm: zswap.c: add xarray tree to zswap 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-Queue-Id: E6218100006 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: amqsybf87dhsqqm1e5sgkwdtcdy9j97n X-HE-Tag: 1705641870-228975 X-HE-Meta: U2FsdGVkX18TyslYxznAXm1pypRCd+biXrkLspQzs6SMQAhSN6VzWqCeySxnM0WvA4eNePwj2m7EtOiQq/MOc2GU27MdbF8QoWp7NpvLsvux8raawGk6ULHnT6cn6TnzsJeQUH+8q1Crm821J6DZgEMaU+DrNtDd0qR/HpaduTC5BOiRVtcUeYC5V2RBtztVRWvfjhqPG8coocKbSvZO2IFfqX0uiIaPOwU0uHP374CqCFgw2Bqmq7js/3BdLKlTqxUXjfj88wYrPqxvZHUvEAWsuAEEL4UphGmHs4G15IRd2Iw7shpYQiFeR32sh11zvbUzMdbG4qqpPnzGcCzVLNvOHO0NXTetY/tfnzZW0JQjIDGR9rrXl9KnmTf01JmJCeDJ2b4xrRNLWt3VgrLE+4z2RLfoKgMZr9rf+12iU5h5KGS72EihaETcDlhgBNRH7FvRKYUAu0UC9k77p7fhQooV5q5pzcShMxPXpOGwmSmULFF9dzsaTZCqSLFB/XBC+Prtw8g6ljqVuoxhQ+SaZsnhPVe9dDVKpew4mXWqAox6K0G8Sny7mJqoFO5D1UKSUHs+QK30NbjLDDl0T6OT4KNmGhyFSNY5prC+ddgzNnAC7EZMnK8ZLX3nG8HuS6p13cITLKYmHtcdkJ4Ro1Fg6o0x2FTMxP64S8r9QjA3hYCnqaRHIXj6WHX+fq7PvfCIlZjDeKykoQ+8FW1HV80TFSXviMONY3NiUecvAXOcsJIy9MJJHm2OJQvBn0xs6rMPuWGidd99vA2J4/bhFK4I1Y+OIRcy3PxZILmxXFrg9poju5tLtTHZEuHi+s1aHUPwAeWEbQ2niH6DrMGvL4/isaC0+ifRtaDcAp/7f8Bks1Dy6wteLyt8nXWS2kj9pkKnl59/7iVJKrk016s/+hxpQPG4tjtCMwdr86J3qMU2xKxFNN6WV4rTAv4AoW7chpdH3YmFewk7MbFQ87lTYs6 gb0sLrnt HDbNyvVY/q4tEg1Ub5Zz8H9hGp9HIclIPcw6at8mzRuNpBTFAqB9HtH6LE7f+PiufEtJKONhGFKiCyTEjvVH8tCWFNEndZs9bAIzWBDQHdnLzZahgmCT/1qfcU4an4yMKDm0kAJSLvIY3ngfQXKJyy3u/TfMnKRCNkcoEAIh0fIMTIB9U2PPhCo02X/j2fdd2Kfp0/7IoZj1IILOFkZbpcURu8/w6L8hwtIhz13QgDvLP21Z6yMYtveX4wuNGjOuYuWU5O/gm1WhPRavwqQecWt7D5BOR9eZywpjw0H9lXxZjrEI= 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: Hi Yosry, On Wed, Jan 17, 2024 at 10:21=E2=80=AFPM Yosry Ahmed wrote: > > On Wed, Jan 17, 2024 at 7:06=E2=80=AFPM Chris Li wrot= e: > > > > The xarray tree is added alongside the zswap RB tree. > > Checks for the xarray get the same result as the RB tree operations. > > > > Rename the zswap RB tree function to a more generic function > > name without the RB part. > > As I mentioned in the cover letter, I believe this should be squashed > into the second patch. I have some comments below as well on the parts > that should remain after the squash. > > [..] > > > > @@ -462,9 +463,9 @@ static void zswap_lru_putback(struct list_lru *list= _lru, > > /********************************* > > * rbtree functions > > **********************************/ > > -static struct zswap_entry *zswap_rb_search(struct rb_root *root, pgoff= _t offset) > > +static struct zswap_entry *zswap_search(struct zswap_tree *tree, pgoff= _t offset) > > Let's change the zswap_rb_* prefixes to zswap_tree_* instead of just > zswap_*. Otherwise, it will be confusing to have both zswap_store and > zswap_insert (as well as zswap_load and zswap_search). How about zswap_xa_* ? > > [..] > > @@ -1790,15 +1808,21 @@ void zswap_swapon(int type) > > void zswap_swapoff(int type) > > { > > struct zswap_tree *tree =3D zswap_trees[type]; > > - struct zswap_entry *entry, *n; > > + struct zswap_entry *entry, *e, *n; > > + XA_STATE(xas, tree ? &tree->xarray : NULL, 0); > > > > if (!tree) > > return; > > > > /* walk the tree and free everything */ > > spin_lock(&tree->lock); > > + > > + xas_for_each(&xas, e, ULONG_MAX) > > Why not use xa_for_each? > > > + zswap_invalidate_entry(tree, e); > > + > > rbtree_postorder_for_each_entry_safe(entry, n, &tree->rbroot, r= bnode) > > - zswap_free_entry(entry); > > Replacing zswap_free_entry() with zswap_invalidate_entry() is a > behavioral change that should be done separate from this series, but I > am wondering why it's needed. IIUC, the swapoff code should be making > sure there are no ongoing swapin/swapout operations, and there are no > pages left in zswap to writeback. > > Is it the case that swapoff may race with writeback, such that > writeback is holding the last remaining ref after zswap_invalidate() > is called, and then zswap_swapoff() is called freeing the zswap entry > while writeback is still accessing it? For the RB tree the mapping is stored in the zswap entry as RB node. That is different from xarray. Xarry stores the mapping outside of zswap entry. Just freeing the entry does not remove the mapping from xarray. Therefore it needs to call zswap_invalidate_entry() to remove the entry from the xarray. I could call zswap_erase() then free entry. I just think zswap_invalidate_entry() is more consistent with the rest of the code. Chris