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 6713CC47077 for ; Thu, 18 Jan 2024 07:05:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 01D776B0096; Thu, 18 Jan 2024 02:05:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F0F636B0098; Thu, 18 Jan 2024 02:05:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DD7E96B0099; Thu, 18 Jan 2024 02:05:54 -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 CC85C6B0096 for ; Thu, 18 Jan 2024 02:05:54 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id A68CAC05D1 for ; Thu, 18 Jan 2024 07:05:54 +0000 (UTC) X-FDA: 81691547028.28.D3FB102 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) by imf18.hostedemail.com (Postfix) with ESMTP id D329D1C0012 for ; Thu, 18 Jan 2024 07:05:52 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=AVrHdvmh; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 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=1705561552; 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=DPJ0nzLo1zKFRAGy41myENWDTx7/OqY/I6L34JXTi8Q=; b=2Vt4nQU70AwU+PduR1le+21tT/AdCtGUcqOrmyP6G+Y4VA06mQ4COt1BIhqiXaSP042u7v Sqw4n4VZTUwQIqdGO/K2PqL7J+F/I6eQDFksIbnlgbyCc8Xf0TdMIAjO/TaPbqnxkwX2jT mc8mx2gmUcZXYwkv/U+KnJf6O4j38/g= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705561552; a=rsa-sha256; cv=none; b=Njq8FLTZp+VNmWzAUPvqxPF55epPR10c4DAX1w8B9+PUDBpMJKp3e9r0u0Z9F+6Ysq1oON Bw43KuoybHJ0ndm0VzuPYbpIhun3YEHEimkCGfRHVkD7nqMhvtKaug62m2DgJRO7FKpsRL l4LbILJqsrFGOueZSOYcnLeIrzPs6dY= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=AVrHdvmh; spf=pass (imf18.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.41 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-a28fb463a28so1273976366b.3 for ; Wed, 17 Jan 2024 23:05:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705561551; x=1706166351; 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=DPJ0nzLo1zKFRAGy41myENWDTx7/OqY/I6L34JXTi8Q=; b=AVrHdvmhAVUFK5mp2Q5lk6kBJk8wyVCo7liD+Wn/I27wkibRCn2OdGCy28udBf5Q4i sdz2c1YOIxRG7Cl+Nf1GFLbuBbaHZMZ4Y/N7bgwPAX1bS+/mZprwqau2tL2jKbFpCjur 80fSwcHnCxNm5/XHFfywjj34XNSZfCzgHvOXApB+6NWb+OgQyRM9bfYNjwXkiAHI93Bj u2hb0pljHjFTAMxt7H9LJh2c+OF+xpAyzal7E5nqo8Wew9NAMedKsF163W4LxhJLxHDi qbVMHXj2lWh9R2Z3e8XRUaN4ARz2+0KPOqcu7bjqwqS1MHzkTIUoDQdR9QCsF5ZKL50j 63gQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705561551; x=1706166351; 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=DPJ0nzLo1zKFRAGy41myENWDTx7/OqY/I6L34JXTi8Q=; b=X6cUc1f1CDzktTSUtti0svcozf18FBksSmxoUadz8OsUazjeU75L04DpATqUWg3V3h foyohRsLRz89+GAoW40cJkM50M7tlczqMoR64J2VZoeIWf8AUuTK6XmLovgKS0Fd5uwZ AK616CPU8ir1sksTGQGAxo+67Lp7oUEYHrSN6ggsoqRjKg4XdXekyLmvbVcPz774i5SO p9ai8ChtmteFaBz1/xvM8PqG+GKV2o9FdRUiGjyrlnUs7pETMAvkkwqCkr7l8Y88viBG 5bBZGtwwFuiNCk9WdRDoK0E14xWyeL3gp6uURfOir6QUxwM7jzNKaeOTO3IIvyOBaF4U /v7A== X-Gm-Message-State: AOJu0Yxyg5anHm7URqHry8Z+NAwBmmMxDF1ZqeGC1uaeEBVZ+Oh2bPEn xNSH8bLwaVh4tGFxQ1gS3T4pdwsRxfE6J7s8ydE+ThgWgj2gbCt/c761xpy2U+Fgo1gAAoV1wJI lhdbqX2q7UhVcNVi2gm/DPR/WwqG3hlwgOeKD X-Google-Smtp-Source: AGHT+IE9/fsEQD7Yub4zY6FfrEq/SpKL5lA8WYU864Nzw+QpeOCHeTy7cI3Pm4K/2H0Vk2RNMWD7c77k/n3TgblEB70= X-Received: by 2002:a17:906:3498:b0:a2c:c556:bff5 with SMTP id g24-20020a170906349800b00a2cc556bff5mr165577ejb.108.1705561551262; Wed, 17 Jan 2024 23:05:51 -0800 (PST) MIME-Version: 1.0 References: <20240117-zswap-xarray-v1-0-6daa86c08fae@kernel.org> In-Reply-To: From: Yosry Ahmed Date: Wed, 17 Jan 2024 23:05:15 -0800 Message-ID: Subject: Re: [PATCH 0/2] RFC: zswap tree use xarray instead of RB tree To: Christopher 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-Stat-Signature: 69dtx69i3fybcaf5czwpe351gey681tt X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: D329D1C0012 X-Rspam-User: X-HE-Tag: 1705561552-100279 X-HE-Meta: U2FsdGVkX1+j9WDYXvZfPusPhSnvJe7PrmwhOxC7nl20tr0wR9f1TCW3CAAUv6PxGHjQjQgUuIVxxwLQHdKCGUQz8NIIgr8njtjOU+HP07MBOx2Kh+4SyPC+XUrqdaJazmSKrTkuf/6b7+SuD5vS+VbOjeTqoNuBN/vBgJwicwcuOKoJ+YZz1pBgoZ8CJwrndl+iu5vG1GcbSyqSl5JOlbvK5OAebkCylVNqwT0uJjCanAEzp4A4mkziOfOKM1RVfaqdJ5qtm+ST2oXG3AmQj+vN1xKF55FikXD3R0hzA/ZPt3TIgu7OBkrnflR4MiQmCB4A7xtKxfMgd8NSyZFl+106P2tV3b/vcbplb8HIl8zKGaSo/U2dnl2tGQzE9sTcrwrC+Rp/m4VMEVMK6ChPpSInXw7rb6/XBkZf90gs9TtJKgEvyL+EejKSuDlCtInA/UKxKBDzAtHN+Axmhs2DE9GaiDgAQxucBia7i0gJlY0lz04+4JuOB1prRTE1eJ48yRtHouqCDpt4nUxf6avOKAYqzFuOuf9NRsKDzF37zNkWjHf/hqqL0lvfoUND/Mz8uRKXp/PYG9TGMRxORYbFKy46NmEKkTMYw8VcDEFZYjEPg3kHard09BLnGD2E2tz6WtxnOcgtSYnHcQY+vFkV5fzSQHveyHVE9fOZAR4NEKFz3QtjH/g+dUQv8pXobA9edr/k9J+w6fXvAPPRjAaIFrktWuNXsM/3Rid+FpObMYwfvbT2rdQm7lfrZuq/9JntfXMSGZMnfGJ5GFlZ5P+mHJuO+tdvod2sqFZ8pvW2AtF51P4+Y6pZr1DMv+zaUyqG55I7oO3YWYIckdkFrHjw3uqaLmv/Yg6iAwa+AymvG84wKdX2gv3qzlgFBh70Jrq4i05KnxwzqfVLD7EtanDQD7Qk4Mud+yNu+iwsgsWFIvUHxJ/dCzy6Lo3C1thG7O20oI6DapDdhLJBjpA1CfS ujznTA7F g/6XxnfYt65KqgA+4GTipY07wSM3hR0Kao3PR7toLSORb/0ppAlddNtTyl/pSMV9tKOoY0/tIcFEQHUJLWvQtZDugxpjvQjd9NpMNSEketpih6BTAbnN/gpc4n3XCney/oJ6eDu1cbK7PpmUoKkyJoRJgWMtOx0juYnDcmykG/mQu52c5y5Y6bKqRMUpo6TFrAHL5bnLyS9xj/2kGbg0MN2vQR/7bMtNjQRKV 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: 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 bisecting > 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 the > > 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 part > > of this patch or this series at least. > > > > Am I missing something? > > Currently the zswap entry refcount is protected by the zswap tree spin > 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() 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. IIUC, there are no performance benefits from this conversion until we remove the tree spinlock, right?