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 067ABC6FD1F for ; Wed, 20 Mar 2024 18:34:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6E6C56B007B; Wed, 20 Mar 2024 14:34:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 696856B0087; Wed, 20 Mar 2024 14:34:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 55FD56B0096; Wed, 20 Mar 2024 14:34:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 42EFB6B007B for ; Wed, 20 Mar 2024 14:34:52 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E1E4A121263 for ; Wed, 20 Mar 2024 18:34:51 +0000 (UTC) X-FDA: 81918268782.22.3DF2922 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf17.hostedemail.com (Postfix) with ESMTP id CB6D740019 for ; Wed, 20 Mar 2024 18:34:48 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=N+VNGL0Q; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf17.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=1710959688; 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=jtorCvnCbO4Me6/2//S96cnRJdC5unscATfHos9lmjQ=; b=kd7GK02vS6WwpLjqS47mTFf7TLESf2F1i43yHqEfYsUI83XqDsSLeyXzDZCIxykrdJW45g a2A2o44oMDbPxyaTU3IxcFMgktCsffwCmMmFUg0hPIZrZgzTnh3Zdm1xqRMcpqj8jV+Uds lO19vxCGRFSMZ7ygkt7bucfwQi5JE6o= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=N+VNGL0Q; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf17.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=1710959688; a=rsa-sha256; cv=none; b=jQFgCFpv1d0W+hd0A3xqJnUoNk0f4hfQpFfaVyscklt9imThC/GTYRDgBy36lPGajut2Sc wWgd6TA5R7ghkKcEkdarilCNMHxC1tAnpHxCBom93rEx7tV/YDTj01yizxkomgUQc8KyyO fRrSZp8kEWT2HD37m2Fcx1+ujPYzZlk= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id A068761092 for ; Wed, 20 Mar 2024 18:34:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2C5E5C43141 for ; Wed, 20 Mar 2024 18:34:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1710959687; bh=yROABpQhiEj/haGfBqdFlpMyY4R0CsHS6BSPalzpIq8=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=N+VNGL0QrxV0tp2rFWogoY4tCujasKU6QXpwdsJWcqjBy9lYTIcrXllIStlAXRCy9 YoLZU3OSmXB/+bVtfAoc9Oakh0qoe2ufffKD+oFcXY360CLLS3cwV9/rinzodv00qK sUwhF0EmBqPZbEV1iDnQNqY2fWLfuGY/jbof+sXrUjcUONdCQNDqHPVfpevbJOtHX5 Lb/Soj1NbS0JJuLxaYR7H5sxmnNDhhes1XoA6+LlqP0fkzHbeamp8q9GhRySIuT70B pHTMGdJajHzQ5FnqAj0n0ejZWIsXKnfWbIi+WKeW7fkHid7C1PoCbD0BrMMtnThNri +F1pj5HGunVnw== Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2d4a8bddc21so2668681fa.0 for ; Wed, 20 Mar 2024 11:34:47 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCUMsttR1jFj1OioO4s9fcg6Cc+tlvc7hq6R8yqdkBc8ldSW1a1nDDIMnky/UrTH9Pq883PqEgnr5Yx+bIoNWMQ97AE= X-Gm-Message-State: AOJu0YyDfzMV1gljlNeKWDdNPHi0e7EpjtcclSZWbIQbuVlkou1oPlzN W/EjEAIQhlFwCbQ4z5t7N60C/zXPVAc+jTQiBCKLZoiDaPnHhDwSpg5/2Pwu9Kk/tpTaCAp0XNt tlmQXaPP/x3fKmw5c1nWZursQtw== X-Google-Smtp-Source: AGHT+IHkqi9pXNyzThnVJEGeMXXXOSr1eyWI1DbgoGjWraxm2MOHjnPHNvLIqJ0cmIil3lbeRoTlMDB99qQ55LHaKHE= X-Received: by 2002:a05:651c:c6:b0:2d4:2b05:a671 with SMTP id 6-20020a05651c00c600b002d42b05a671mr11089276ljr.32.1710959685792; Wed, 20 Mar 2024 11:34:45 -0700 (PDT) MIME-Version: 1.0 References: <20240319-zswap-xarray-v7-1-e9a03a049e86@kernel.org> <20240320100803.GB294822@cmpxchg.org> In-Reply-To: <20240320100803.GB294822@cmpxchg.org> From: Chris Li Date: Wed, 20 Mar 2024 11:34:32 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v7] zswap: replace RB tree with xarray To: Johannes Weiner Cc: Yosry Ahmed , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Nhat Pham , "Matthew Wilcox (Oracle)" , Chengming Zhou , Barry Song Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: CB6D740019 X-Stat-Signature: jwfcriqt4okraq7mro65cjxd67mztjkg X-Rspam-User: X-HE-Tag: 1710959688-962798 X-HE-Meta: U2FsdGVkX1+yDEkN8rFjKitbqu9GSj8vQ4PODW90u5uMFo2VMpBSphIrGnoWVui2NDoJ4CCpc9nnLbplhAfdY5ON8+ql8Mu4cimZTBiBdkfC0xTysU9NCdfZd2mqw3M2yrS1447X91i8U7MQndXRdYicZlm622+eIB5+m4SeUAtNUOgMPe9oMDdpPCQrEbqk/rVI/jgKhrr5XBoNXTkuN2nsFRL2/oDq/g1DhT23xPTQ1/JcbQ5mjz0TfKq4hIjPxCsWDwRheQkh5LzwuIuWtYlrDUNUmuDT0Zjdpe9rIQG9z/dM6zga0nG2A1Hww+BOanxc4LhUzdL1tI34iF1bNUgYIZtotK4FANzJr+UlrwAfQ5zanIU7DbBiDyc5dDpPj/psjAdCUVAmt67cE7X8e70flImCsMV+i+qgp1ZXJe+ear/vIQEvlu2mIgXO12/rFUJjm8l+llZMTP1xWGTldrLlcXIxdamofxGCnqDL4W7udNXxFZ1ozqTCOMteZ2gC0zv/lP/ddemb7S3/Mz4knkywuLoEhMZNZdY5d9cAkQ5QQjoXam4xLxTzkAkeCnJPfGvP3C2exPsxbJX4zdimXilJR3eUmqBVBAcBU9jDTOMzzcbu+tbnZnPNU+r/tpScZo6bl3S0Vfa8appGMon/7gIn1/UR1zWAkirMhzir+htsNW9AMk534oGxFxCnyTm5rwjQZr/VY1teQrvuRsa4a4ioNKvqIH05gr4lHB4l/WCc8PZQekeShE38jtEvCiVtNqDlnWvLIZkoCfB49RfaL3NEQX5rEo4c3LPZZGe/tqm0cJd6JKtKtVFWJOs7IYm4isnA+2273LIOrwlKVnfsz32rw5HVyq/FSVgrISnw0oMJXxZtfrjmNH5jVNqXAnMIX3cv3mc1m6nLmC97YbljX2oRqRJ/LYHt0ulEvIQOn5tJnWqRXOAb6VsxqtXiJOOuAOnFN5QpKlpxqnrf7dU rQ+E/nWx 8y/JTI2OPtfXugLc9LnrHD7ibdsw/0VCtnfJgNs5G7YgKvmgFJTvZssR13SB5HvPNUSMhb0P0qb90pWwpwh3YUqQr4ZhfmTV5ov6+wPmbE1+XVYcKKbQhlFVb0t9Dkor9o0mgAl75Ls8cn8SOIGcckfo+8IkqBDY0nspg959pkTDDnUNwFnl82HnuJyTDYVzEIiPMmJWTl0x1oLeUQlejaMkvFykmmHEz4cZGor+kL6GLD8sOAzdpMEChbCR8YG7y31Qba5ug7Ae/cDCMu0GY5eHXsg== 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, Mar 20, 2024 at 3:08=E2=80=AFAM Johannes Weiner wrote: > > On Wed, Mar 20, 2024 at 07:24:27AM +0000, Yosry Ahmed wrote: > > [..] > > > > > - /* map */ > > > > > - spin_lock(&tree->lock); > > > > > /* > > > > > - * The folio may have been dirtied again, invalidate the > > > > > - * possibly stale entry before inserting the new entry. > > > > > + * We finish initializing the entry while it's already in x= array. > > > > > + * This is safe because: > > > > > + * > > > > > + * 1. Concurrent stores and invalidations are excluded by f= olio lock. > > > > > + * > > > > > + * 2. Writeback is excluded by the entry not being on the L= RU yet. > > > > > + * The publishing order matters to prevent writeback fro= m seeing > > > > > + * an incoherent entry. > > > > > > > > As I mentioned before, writeback is also protected by the folio loc= k. > > > > Concurrent writeback will find the folio in the swapcache and abort= . The > > > > fact that the entry is not on the LRU yet is just additional protec= tion, > > > > so I don't think the publishing order actually matters here. Right? > > > > > > Right. This comment is explaining why this publishing order does not > > > matter. I think we are talking about the same thing here? > > > > The comment literally says "the publishing order matters.." :) > > > > I believe Johannes meant that we should only publish the entry to the > > LRU once it is fully initialized, to prevent writeback from using a > > partially initialized entry. > > > > What I am saying is that, even if we add a partially initialized entry > > to the zswap LRU, writeback will skip it anyway because the folio is > > locked in the swapcache. > > > > So basically I think the comment should say: > > > > /* > > * We finish initializing the entry while it's already in the > > * xarray. This is safe because the folio is locked in the swap > > * cache, which should protect against concurrent stores, > > * invalidations, and writeback. > > */ > > > > Johannes, what do you think? > > I don't think that's quite right. > > Writeback will bail on swapcache insert, yes, but it will access the > entry before attempting it. If LRU publishing happened before setting > entry->swpentry e.g., we'd have a problem, while your comment suggets > it would be safe to rearrange the code like this. > > So LRU publishing order does matter. Yes, I agree with Johannes on this one. The publish order does matter, it is not always safe recording the publish order. I will keep the V7 comments here. Chris