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 3EBCDC4707B for ; Thu, 18 Jan 2024 06:21:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7733C6B0081; Thu, 18 Jan 2024 01:21:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 723BF6B0082; Thu, 18 Jan 2024 01:21:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5EB266B0083; Thu, 18 Jan 2024 01:21:10 -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 4CAF86B0081 for ; Thu, 18 Jan 2024 01:21:10 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 10AD0402A8 for ; Thu, 18 Jan 2024 06:21:10 +0000 (UTC) X-FDA: 81691434300.06.6648D4D Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) by imf09.hostedemail.com (Postfix) with ESMTP id 342EA14000F for ; Thu, 18 Jan 2024 06:21:07 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3KjvYHHv; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.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=1705558868; 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=VJlVK4iBv+zofuCe2C1FT9Euxl2G3WgJemm135I8Sv4=; b=HX240YkHYuxIdx5fIrNhKniTn+IJzNwSpi/ANzQC34eS1dyc7QNFuEXi2H1awLvT9fi2/M hC7Pbf7ubVjhOuluL00OvsONXkWa2mWp71EiwfNF1ntjeY7fWXa13J5sjXN+POFmqC9Y62 1Es4vXXywr8/we4eZwIdodZl7A2XkcY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705558868; a=rsa-sha256; cv=none; b=3Rccr83NhYQEFEtL7BvxnG2qBMhrjNWRdeUi4uccsqG5cjU1UmErdQVYdjcxJTSoAJPDAC Ak4Fm+B+BvgNUvOblulRhKPGyloOCMeck6WgIghnByzxP1zVeC2yORkn5H0G5CKCbOuvK9 33HKGNLyehg15Wq019YXl97/QhF6RmI= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3KjvYHHv; spf=pass (imf09.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.44 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-a277339dcf4so1329148666b.2 for ; Wed, 17 Jan 2024 22:21:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705558866; x=1706163666; 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=VJlVK4iBv+zofuCe2C1FT9Euxl2G3WgJemm135I8Sv4=; b=3KjvYHHvQKR8FW3Omj073maOBMIqxIquYQgo586dOjKPqKXtPRbv4qj7ufmLQEmNZX 5Vc9Pgcgl8x3L5B1fEZmixKaAnEgKP0kfVrWN7JcvWbUiyoccVihcoppbm/dZN7lcNNr 0ltr+xF/odVBwtypShmS7ajDkvan6jferI1U/GjTXdcX0PcNwtvKKknxHw1k3nDqzJ+8 YdvUuCS4Sc2qDkCd6shVQXiY+rD6UbvGNlN0UCdbtAvKRwK8A9+5laFmpQBAIz++T+LK AooQSTsYCYs1dbHEy3ancFaUBEQh004iGLp5/ke0iUpuppaAB2+xmQukinVGXWDwnSOi MyHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705558866; x=1706163666; 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=VJlVK4iBv+zofuCe2C1FT9Euxl2G3WgJemm135I8Sv4=; b=DgmkPGIOijwmmt6UWsSUala/i/gwrhbozNC1K0QY73h3tQe3PzHcAEx+wp24BZK9CX X9TkWdOihvfd/LFVDf6sMTCYTwJl3DwamB/90slTe4P57j/R0KkHeXxms8dwCHlmgD1a YY8eQaYDcE3COhfsHLWN+HLl/zy+Hg+bnOjCSkXckj0WArYGQxd3Ww/Ow8a7GbPKSC6q QULJFxyCC0IIKj/dEkykdMJY/Czq1xnQ6nAGRsYUe0n3AqO+TfNmdIJBZ+DKu4panJ0o LyhndWTvGcw0lrsf1PpS0yNUy+VjtSSsKPWT7e1KAWac2t/E9mHNS4q+solByFT+ZZSH EjPQ== X-Gm-Message-State: AOJu0YyJtdJWMxXSa5JDVi21glOdbql4zAMzArcemyxVsKTwF3UcRNjj z/xvVlfuW5ZGqaACImV91xO3Zlzs3K8SHootMVGi/rJJliMpVmKt0TN6RoPoyLDx+VC00dkXwRm OQclrPZqdgnYgPhjfxjDOG8r4IF5sqcaheeCb X-Google-Smtp-Source: AGHT+IGHduAicQV3fcnyu3+XAb4+oQD+mNQx9HiyHI2Ew3gcecJqxh1u4e8lz19YEfvFEU0MN6dX2/LLnavDgy4XyPs= X-Received: by 2002:a17:906:f0d8:b0:a2c:872c:5683 with SMTP id dk24-20020a170906f0d800b00a2c872c5683mr152624ejb.41.1705558865635; Wed, 17 Jan 2024 22:21:05 -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: <20240117-zswap-xarray-v1-1-6daa86c08fae@kernel.org> From: Yosry Ahmed Date: Wed, 17 Jan 2024 22:20:29 -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" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 342EA14000F X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: bujztjxy4b136yfo4ju37kxqhysjrz7b X-HE-Tag: 1705558867-99410 X-HE-Meta: U2FsdGVkX1+XeQ7S5ttLWLQJa+iGlxmoP1ws7P7sPvL9jiecEQbW58FobpCBtIU7hRHDILWOv8B0LcdZ+p3h2XYYxe2grjcGNtsOukLtWZ2k4AEB7cggDDZGv9Wkh1e92sz5Nn9HikhtbdBVA5ihFpJbfV9qqdQnbtvZIAaD5HlGGmt+FDSTPW3FR9ZBA6w/B91nn16MjQJ2o1T2ChxCkOvy3GOvpAorYdfyZJr7LYZ7z+A0bpHM+jc34H1WRhKRuQx585vTccZr4T4tTypvNMPbfxs4jcO+AoBzCe7JYFSNqNN8ZT/q0twoxUTcywxVbBSrSe/Q3bhhipn4+YCG36K4rq/Heoo+9He45PxMH6x2ZoHQayxmLSiky0zEkT2p2eImQjbcGslf85yb10g0XlVPUNn4RtWXl1i1vWVscrX+kG4szE5FiIRSrdYjxmxOFhaa4Kxc2PwCFobzwj5RVJ5CBiijAC3iMED8b2heUO7/O7KTDhoK36CaVEjnIgZksC0UBE2rMBun6lIIb/BZjVOkDX5t0PmwPh3PDJxElLR0GIRaK+OrFEE1OgOIn6BpN9TzrvGb5PLXehBv6PacaDqDO89uZa1XxMfyAMHsRnfBOpqRTxDt2BE0zoQPYhR+6p60wZTBXARoC9ngxnxrwIX+Umnymb1UNCH+/YF4IhlCyOGQBcRmPdZFJol/jClWddlGGaYjAWUxvqTz1GilLOyi0gmVamcVSUlEZ6DTmGcbXL9qP8rPvNj4+TuWFgXce8bnT8zTjDtWyaSKrjpXwHSzOlwGl3cbHA2Gdci2T2iGJPVIeexDcYaNivIIqQ+jxZHjCOJLltFS303zfCQs9KcPPTHOVdg8SqpeQgAgRMUHaCBXKaSmcujCnHaCiHGr+Hisw/rfD41pL6pyosRW3jvV+vg+JH3Fw/xXqDZrVpzQOr2dSzykbfLG0CG0iorWIVC8IT6q2pweCmThaTK DC5HT6QS kmyW9f8jl1xn2EPIMEbetous3EfcHD/Z8vIv3klXNrF3suNXqMDCAF6whOHo68cnxoP/txdHzGv+nZVHCEVZFQUNVuFIMchXuMVuQPO2Vt8rQWpjHCGnY1NP7uzeXopm0Qa2lvCy/Wg18nGLbiIk4+Y7eN8nXpxDr5WsbqJLST6UoBNEvjRH6tfa+KCf0bjky4bOtTMxPH2fSZnKceAIK79uEr970zJTV53UVdO31EOeSh/ySastIaZ6vTmwsYd3d9Gti0fb5S8NPVZaCf+O2I9UUIwQp/Rg2NbZR 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 7:06=E2=80=AFPM Chris Li wrote: > > 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_l= ru, > /********************************* > * 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). [..] > @@ -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, rbn= ode) > - 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?