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 99F29D3F29B for ; Fri, 18 Oct 2024 20:41:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0F6666B00A8; Fri, 18 Oct 2024 16:41:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0A6E66B00AA; Fri, 18 Oct 2024 16:41:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8A896B00AC; Fri, 18 Oct 2024 16:40:59 -0400 (EDT) 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 C4ED66B00A8 for ; Fri, 18 Oct 2024 16:40:59 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 7703816017A for ; Fri, 18 Oct 2024 20:40:45 +0000 (UTC) X-FDA: 82687891902.10.B792CC6 Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) by imf28.hostedemail.com (Postfix) with ESMTP id 44A30C0023 for ; Fri, 18 Oct 2024 20:40:46 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BnxvuuKW; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729283949; a=rsa-sha256; cv=none; b=k49jjFCTEKpFidhD4MjdIb4KIzTHO42sI0qJySr4vyw7KnLbPvJh3CiWmFbQg2G/c5xSCO HrW1NEgrWZqn1QT5h1Y/E0INkCpjwxRMQq7RlFh1IFobJFR2feI1on6/mOmlucY2TV1gQr qkmusQXsMKulOhqKaeu1PyrGz2QueIs= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=BnxvuuKW; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729283949; 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=htuQ6Fkdyy+fM986uZCUwobTiIqwyPS/bkEyY4ioUA4=; b=szOvgsbgsNGlaRYuSZLfAWVIn6aNB5gwwNqyea/XqGli2pWO+rOIeeup6mFXi5H10XGEYQ BH/eMSjl0hTjeF05x51NNbnGESa/FT9A4NeVNqEXrh3kKE+rkdz6hsPEwjqH+voOirkXI2 xzPbNCkdf31yKvisFbC11NCV2D/Cnf4= Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-539f72c8fc1so3014186e87.1 for ; Fri, 18 Oct 2024 13:40:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729284056; x=1729888856; 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=htuQ6Fkdyy+fM986uZCUwobTiIqwyPS/bkEyY4ioUA4=; b=BnxvuuKWfXVtr44PVyrj7FXFTi4flvF7ORpwdc/+8bIYZzAuzM1UrjVH5QzHK60tsY 1P+oavWEDiTrpSxC8y4IGYbs8R3Vz9I3TfimSYzlxvmCMXlmTNGkwhseX9XJ437lK0I8 OpSUIXC1GjSQjoPbMVmilZO5NU8FZwU9Mm8SdVWVwvSafBAF5ExREPaJflnYP1G81bMH eOR+hg4KjsU470Cpfv3IhdyXKc1yVJ80AWAhcOg7V2vB+/fNbXcvWgyPrnki1borFZ7m LBpmGoD5tkvKYu89qAZtq7L0CAONXR3wIEjtU5ADJa0x9eJbUILYh6DgkN9rCotsbZhM rxiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729284056; x=1729888856; 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=htuQ6Fkdyy+fM986uZCUwobTiIqwyPS/bkEyY4ioUA4=; b=qc43NU6YqfDPfwut/9ciq4UA9M+LlEgkP7Ir6KCZvmDOt3n18n6iKw/63usrjnTo7H 2se8qMCxz7WYqXtpHOzosjVUzJqfUzhn9vPaDPEaYU7gS0U9IBpGWH45aB4f09WcFoLB 1oLWy53mV9o/aoFnJBJgUhz8RfujDqgiYHCJSuT0/jElh71sYqZ7J3AUa3VU3vAfaOg0 XG7d/EQHv8qheEUTm3dTiObTjFbUvQFt1LNjCyBd623kYVUB3YkJ/bx/ZXolKCQ5BYFz e4IUuPCY4HN/WT1N1QU6onAEMZkxsfRx/1VAYWKiUXb16h5ab00E6N8zYfpYcaTXCdr5 +mPw== X-Forwarded-Encrypted: i=1; AJvYcCXeeWrvLAufgUqJHZSkXM90Xo3NzEARGZQxJS9rmKmdU7S7bfFZy3c+eLwGyXcdYEbHUQxUbMCC+A==@kvack.org X-Gm-Message-State: AOJu0YxGRRGkKvxRe2MG/MSve2A4Jmj474VkPRHRNL8AFFDDEBtLT0ln c0HHY0vCrIHEWNx0YpsGGWUXKqI820Qbx4aD1ZJ8M1ALkbZittuygnMXnn+XL0MOvkTRmChzian fozXyJRXWT4oQ4GJDI+2mJncwkZPx21/6eh8g X-Google-Smtp-Source: AGHT+IGG4ckJnR/P89yT/YqDqPaZ69UYHlnK5W2i22Qmdt+BY29V/dv3SueOaylo3n8+Y9lyTyKiq+N7ELER2ObfcZo= X-Received: by 2002:a05:6512:3b12:b0:539:fcf6:5d24 with SMTP id 2adb3069b0e04-53a15471efemr2566878e87.59.1729284055416; Fri, 18 Oct 2024 13:40:55 -0700 (PDT) MIME-Version: 1.0 References: <20241018192525.95862-1-ryncsn@gmail.com> In-Reply-To: From: Yosry Ahmed Date: Fri, 18 Oct 2024 13:40:18 -0700 Message-ID: Subject: Re: [PATCH] mm, zswap: don't touch the XArray lock if there is no entry to free To: Kairui Song Cc: Matthew Wilcox , linux-mm@kvack.org, Andrew Morton , Johannes Weiner , Nhat Pham , Chengming Zhou , Chris Li , Barry Song , "Huang, Ying" , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 44A30C0023 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: sx7di4ss3arxt1dzj4mkdnnspbbx51af X-HE-Tag: 1729284046-971323 X-HE-Meta: U2FsdGVkX19vRudbtRvyMGcB0TB/kOQ3TgUtSRKGvfG5bXK2HMxwbSQ1W+c4osi2nf8aoEA5VnWZUCm2My1Rb/rjZV9ZZPJudAlZySaXGWcF/vBd9/4HH8aqD5BLRwFFUZ7s9BOLC5735NnQiZHwo9Rm4MFstYErbXXLu7QvHcxiTWBnpdJDZIugN2lWdC1/942nMRnbRJTNnSYqxbRM9LkwOS5thcDxI0ZaVoNGEe6EVB+nmnGcHUNmZs6EN1Rb0lVBSS1Tr9+5pNdK334i5YazGG41ghn5UuDS32y2cKHsMF/c2GCZXFBd4mkuhIqB0qFmyIkIJxw0nkQZ162//G6YsnfV837ZT5YEJHgDcZDoTSuDjUfIr2vlLWJxyqKci5Osr36l8tdw7RtUdi2qO4NJ/91SQwf3u2o0onNsxj2jZ+xNzlTUQQyWjgB2O2D6OS4uCGQZXjpXwh4AeTlLb655vNS6XMUkb2T95viYC6pQrd2gsoB6voiuPtz3a9tb+2gsqr/VKb/wmwNH9eJSkakZP514ZmvDQ/ke70vu0pVGCUIi1yvKyli6o/tjAWdotjUkCyw9WcTw13UsGjqdDitU7sowgpuRL3YbV5RlymZPtWrhYd1gbxA42RG+RU+cmsvOlGKLAejKxrEv3xm58j6rYtzI4numhXh5U+rT4JbhTe20TwbdsnTlbyidb787ZNHngho04INAIX3+WFsOCiQI+g0Di++EXhNYZJZFo3eFhTDW/aIA1Vc3Nae+3VFjFxAMgzlssON5IlaRnKPvkUDpBkG9jcrgNwI/xZPsy/YhMTrswUYVhxVWOcKYm8hiJYkTqWKi4+srEFHMtdgFsYy1HdJu8nhpbomavwXZQNu3SSmO8dW3hevJrQ3o7vAJgerF8jrxXvmyxX+S9O5l2sHzMSMn9wC7V4GYpLZDp8An3uuH+UQl+KVETtdVgb/e+WjElOgVPwCy7BrbJHp 38ZrERJV aaUlAz+KgYNJCsK6Wlr3rrv/1baDJ191vK+lH8j2pOdHbqcVLCENHspIEtaDylano0dVbXon2uFTYCa/ssVcKP0UfSgdcL3fnDIa3sPdgc+rJ2kAItG6cNAX8/e6gopHQgG/5YHVLtCLJ/J3TnpOE10QYLJb1OiSdWjMZ0mYL0uhIl7kJLCTzKitiWCA/u3YC9hu9z+EXhPE+/Qp8DHghPP1jiskEwsCbBtvWpfgQCD4oJTo6sa9QiAvVnCgMV/FusCxiOGrUNSBy6+P6Yol7eXRJJ/WsYCX9io7YPV1qCN3gHYg5WCA4PSQbBw== 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 Fri, Oct 18, 2024 at 1:01=E2=80=AFPM Kairui Song wrot= e: > > On Sat, Oct 19, 2024 at 3:46=E2=80=AFAM Matthew Wilcox wrote: > > > > On Sat, Oct 19, 2024 at 03:25:25AM +0800, Kairui Song wrote: > > > if (xa_empty(tree)) > > > return; > > > > > > - entry =3D xa_erase(tree, offset); > > > - if (entry) > > > + rcu_read_lock(); > > > + entry =3D xas_load(&xas); > > > + if (entry) { > > > > You should call xas_reset() here. Oh I thought xas_reload() is enough here to check that the entry is still there after the lock is acquired. Do we have to start the walk over after holding the lock? If yes, it seems like that would be equivalent to the following: entry =3D xa_load(tree, offset); if (entry) xa_erase(tree, offset); >> And I'm not sure it's a great idea to > > spin waiting for the xa lock while holding the RCU read lock? Probably > > not awful but I could easily be wrong. If we end up using xa_load() and xa_erase() then we avoid that, but then we'd need to walk the xarray twice. I thought we could avoid the rewalk with xas_reload(). I am not sure if the xa_load() check would still be worth it at this point -- or maybe the second walk will be much faster as everything will be cache hot? Idk. Matthew, any prior experience with such patterns of lockless lookups followed by a conditional locked operation? > > Thanks for the review. I thought about it, that could cancel this optimiz= ation. > > Oh, and there is a thing I forgot to mention (maybe I should add some > comments about it?). If xas_load found an entry, that entry must be > pinned by HAS_CACHE or swap slot count right now, and one entry can > only be freed once. > So it should be safe here? > > This might be a little fragile though, maybe this optimization can > better be done after some zswap invalidation path cleanup. The only guarantee that we are requiring from the caller here is that the swap entry is stable, i.e. is not freed and reused while zswap_invalidate() is running. This seems to be a reasonable assumption, or did I miss something here?