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 8D4FFC4725D for ; Fri, 19 Jan 2024 19:30:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 01A696B0075; Fri, 19 Jan 2024 14:30:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F0C546B0078; Fri, 19 Jan 2024 14:30:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DAD136B007B; Fri, 19 Jan 2024 14:30:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id C7FE16B0075 for ; Fri, 19 Jan 2024 14:30:22 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 8C33E1C103F for ; Fri, 19 Jan 2024 19:30:22 +0000 (UTC) X-FDA: 81697051884.19.AB83579 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by imf28.hostedemail.com (Postfix) with ESMTP id C6337C000A for ; Fri, 19 Jan 2024 19:30:20 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="vOkJE3/Z"; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.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=1705692620; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ugyBhCYqe/JSP/toMybx2mnqVSMpXtGwbZ5vXwky8K8=; b=AIn8FfuanqJNoRF5GiueYQGmAJhHM+6XgrOdRSTwzp/FgC+afhE36FE3kZRLrLPunJLk+d UTKok7DBNrZYMSkUBvzTGge2a55utZu+ofNNqhSS2SsCzqVfqMNWihJI1JtPNKhhz501Hb J0/67wVv9ceGh+hghO2eh3wKyfgOMfI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="vOkJE3/Z"; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.46 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=1705692620; a=rsa-sha256; cv=none; b=FjbQSSlKuBkij4V8P0tx6TWOSeMjrjflS7AUzzQDfzYaX7R+hSaT4Du5QVDrhGCQw/X07s 6E9wT1NeUvace1jzXt1cWUbvQkBdj3gPJYRAs9xADsw6EZcka5u7ZbNDBny9uqLKkQtqBi zGMC5rdt4Nqj/cKJVExPQzxjxCtpKjw= Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a2d04888d3dso125289166b.2 for ; Fri, 19 Jan 2024 11:30:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705692619; x=1706297419; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ugyBhCYqe/JSP/toMybx2mnqVSMpXtGwbZ5vXwky8K8=; b=vOkJE3/Zw2PEBHTj4cyzRW7Ih+BEYLrQKS+M6Y7ZSE9r3jNrEx4mWCqBqELw19RpGt ms4kuu+iVbV0itjSkQyGewIih9/IQehAY/jSTVJkxdVa0F0LmrOsOmxTBMP9WzSeGWSN FBRIqWP/7V4IBZJPxmIUT9pwlQEiVnXW+FjzZ+3IpzZtOeZ3ZKrj38Tl+VyI5SVnmyqB djE2yIPn/0z29ka4zlzekqVrGWEwxZYf+j+1dw/64voAR4yPI3fnDckX3DpDcUao9FGl kCFilDTsIK+h9JYeoNKsGwG1hJezBAvj6vPjdcJGJXq/TwHVUS9l5PbRFG+frljhp5Wj PQxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705692619; x=1706297419; h=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=ugyBhCYqe/JSP/toMybx2mnqVSMpXtGwbZ5vXwky8K8=; b=MzSdiuluoJ/g1SkNZbS22oIGFC/a2S54nh9+vxSJ6ZCCgcJUywOmFBBO+ogDd5N9W3 /OD2GfxF52RYxHXxH38MSWJQ5drMErI7Geic112Zq+dZvOpHX0vHC5aIM1yJ+HuJNL2X xboNzGOniVgCtg6VikxqtcBaWFs+6cqZg02ZC+oAIswKF7XSEAM+xq7+nHZyMGoN4+Q2 Z+m8xvyj1UdFxbS5vMwmTxjhRXo7T587Gm+mo3HKIw1WP7P0y2xr+AcpJb5l51Tx0mb/ Bi7XjMQeHOHPoaTppkU4UWc0H6TLjCzrRhQ40V5FoXvWot+3Vn5H7v6L8XkDRr4IWmNM BkNg== X-Gm-Message-State: AOJu0Yye8FpI2N/Xo781LpXc2PoIxopzyfl9l0B2DDZMhlTSxSs+hew3 Uj/mm1zm3JZ1H4I2yNtBN6fTzHQVioxDmbuQzI+27W5H51t8dOJOgtFyE80Q62Z1kEEacfW5Cdn 3RG8tHG05CFQZhnIue2hHhg1L0P9RxCZT1u3R X-Google-Smtp-Source: AGHT+IE1cNh2Ok1Hw/xx/XTYjRdo3LIOnQNeZKcP5EuYjLQKeenERouSCa8xmrUhaf7ZVukdoVG9YNoA305yp3SzEtw= X-Received: by 2002:a17:906:2b85:b0:a2f:8533:af79 with SMTP id m5-20020a1709062b8500b00a2f8533af79mr139810ejg.36.1705692619040; Fri, 19 Jan 2024 11:30:19 -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: Yosry Ahmed Date: Fri, 19 Jan 2024 11:29:42 -0800 Message-ID: Subject: Re: [PATCH 1/2] mm: zswap.c: add xarray tree to zswap 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" X-Rspamd-Queue-Id: C6337C000A X-Rspam-User: X-Stat-Signature: 7ktyq3ys984zstibu74dmtaj1tkfcwxd X-Rspamd-Server: rspam01 X-HE-Tag: 1705692620-628527 X-HE-Meta: U2FsdGVkX18CFQKNBY6Bmpb9chU9SJ5b2ZN8ZahgDvEHWigSNOksDWhCyIeq7447S7pHE8PZZH+/kuT+JAIf2j89ogWFlrJsvmDMFLYof/cgXNqO8r5Mgl6/f2TNdfELR7HHIuicd6AgAsyfP2yaW6MJVON1UvFNjl4/RW040h8MFkPf9V3/2f7mHtDVKPxA8qmfBYBaBDPCsttJkF57bQpZO9YCxjIjHna/c9XBxwEyiatBtHPS7kes8ga3bvVZcjP4/vGmFv6s43dUBQjZeuWQ9Mg7DsBqSglVPAsv13tb6irK7k8xN+ilXeOSep/IOi+20elGr962PaOy+W9ZW5Ic/CyhHBEDG+7JztWACK2bMYipTW+e2aZEI4w08GaTHZLRLl5SUsCqVXQTM47/q/BoKF5rIzC80F6nPaU4GsJPpmicahug+CBjIalm6woys7uJsu/6rFOmv9dnikp/kgSclDk8O+zFkFOufQ/f9Rhhi5jl4r05T3gVBrVcVlS+gvS5if5YqVf1b/I95GQQhNe1RGHdwXStEbjuWvKFXCgRIQAc0VK3Stsoy1yc/yKWPnZTMEWk9KsDBktuf/Ct5gcul2wzwdWDSWcP0oEQMHKjusqSdFcvlxqFrgsVFX/qS6oIRWJ5BhbEHmkHYG+YBes4RHxQJdysw+PMGm1ovKZBB63np3GUAm4ys6jb/gYSUI9PG65Q8qHmJnVokKT3wDiJ4LZKAmrfSKqTZa/tRNN+iETGCu22YcDHe4mfR58+bcQ8pG10VBUvc6EL/Kx7Cq7rWTK/Bni7W+vnJwFKMt4DP0CfbBApzK6FEN89Pu35o8lFf1ox9PP8vHllnqIEaqT1HjpUeayNTxglzaJPhs51YpNar047CqCHJA14RxpALgaFNzlbt1FCXPW8UjqJaVwkAhvngUATNOfmJMt7/NrQmcgOg0IpOoinJyGEnK6zZbfcE7xFVbCF3KYJd2m Z4L8/bKj 3COYmvbbZHbBDRhpmQjq5VG7sakauI/j8oDOJUTj4cB2Y0NASc3+N/Uk1WmIooxJbU1ylyBybzacpFz4FmpbVAJNjlfhWDz0eJOJRI0iHJk+jq4mZsIMxcx3S9cAWERHzXy1wljJKXcexpUV/OMbwno7iq3kGhyK2HXaogsziNlPCAFfrMjFpsqbAed7iPfBY1qj1yR7f/0nNq+WDgHCl37R3RICtt3U+c8ib 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: My previous email got messed up, sorry. > > > @@ -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_* ? SGTM. > > > > [..] > > > @@ -1790,15 +1808,21 @@ void zswap_swapon(int type) > > > void zswap_swapoff(int type) > > > { > > > struct zswap_tree *tree = 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, rbnode) > > > - 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. I see, but it's not clear to me if the xarray is being properly cleaned up in this case. Do we have to call xa_destroy() anyway to make sure everything is cleaned up in the xarray? In that case, we can just do that after the loop.