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 2B0E6D3E18D for ; Fri, 18 Oct 2024 22:39:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A29096B009C; Fri, 18 Oct 2024 18:39:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9D9B16B009E; Fri, 18 Oct 2024 18:39:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8A11B6B00A0; Fri, 18 Oct 2024 18:39:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 6C7DB6B009C for ; Fri, 18 Oct 2024 18:39:06 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B55411A030D for ; Fri, 18 Oct 2024 22:38:43 +0000 (UTC) X-FDA: 82688189136.20.7ACDE48 Received: from mail-qv1-f47.google.com (mail-qv1-f47.google.com [209.85.219.47]) by imf24.hostedemail.com (Postfix) with ESMTP id E0696180003 for ; Fri, 18 Oct 2024 22:39:01 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=WHvQzbLQ; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf24.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.47 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729291095; a=rsa-sha256; cv=none; b=EarNX4up4rAFrBxvPk235eqbtSf1wS1qaqsy0NTSbZCSxPf/Ow4PCfkHAMkvFkBRVt+xhS 7n4SIYvjIjcXQ9sE7W2f4cbqxT7RRPeeDRNBgdEn9P27TlSSs4zA0eBXlR3k8qtX+CQ1Vx jRavSLniscWV+syas/nb5fP6gN16yWA= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=WHvQzbLQ; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf24.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.47 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729291095; 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=27O84oor1VFOi2QdCyPslhDi+MuWwwNB8nDvjSXIg5I=; b=RmWAmvhGfKG6I1oOARrJUXwCG0mDsNL29MrQltDU0nSL+W7+FyozPAe9N6dh59mPPNIkQh iMmT1zk3WKO27frQP5pouClRLEsioE1qP1NraWotgYTxWx9Otoy3m5zo0POB1IvDaETgjj TdWf6UO26/yqN1h8NEROFP3sOxmW4uk= Received: by mail-qv1-f47.google.com with SMTP id 6a1803df08f44-6cbe53a68b5so14015836d6.1 for ; Fri, 18 Oct 2024 15:39:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1729291143; x=1729895943; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=27O84oor1VFOi2QdCyPslhDi+MuWwwNB8nDvjSXIg5I=; b=WHvQzbLQtDgIOR6+hs8DrY78F57gX3BiItEUW5/r/R5UXjCVYTP02eulAFIAEttXov te3VUF1kxsss1nYA+4V85SCK7/5YRG8+2d+J7MAoz3+l3r66bqyOgvPh0ZIY1sAybwi1 BC8qCn8JJGAVac7dxQwbRw0iVp+pD7cBQcARhVV0stIyNaP3nKcxKy9C1e2Zz0vN9fCd d3mW44dyMDhHMoHFhTMU5fY/PzoLuJ9416zZL0xKU7xWg4LebqZrFWOnkJAIz0sisviw 2ohdEQSvDL+YW+PtUN9j0nlXvdJx/SXBAPGS8nbNJDaHHdkbFQ28N3PUP87QmQO0GshT pQrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729291143; x=1729895943; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=27O84oor1VFOi2QdCyPslhDi+MuWwwNB8nDvjSXIg5I=; b=JZ1csmz/LybdPa4eZoJBMLiIjZDTDOzE2WrNFS525HYXkr7kt1KOU3o5E22XJOACwu v6paqkC5B4Tuu/EDtI7hWazTpf5IKJKNBfF/V2R7TerZnPhswXeipGC9VSgu9BdKjEjR D22wLPItvx9T2Z1b9TD0yy9nes3TtnH0T/w+AD2m+BgNSFUyy3MfmXgJz1Hbh8yyLfPe MrTiYpAaDX25piOnfrv/O7iOaT0QbmpjiVG+xrN5NdM9e9lCqr1AH7C0U9USRTHGrNSH sQAE7iiQyL2q5FRSrMBaI3l2leUfgERnv0ml2Zv/mG3aecIljMkUkadRg9iCDlfYmkRs Q5DA== X-Forwarded-Encrypted: i=1; AJvYcCUYDQAxE36jFKDhDA+NQ0te9xugQVd8piSWP+8rI4r0mzz9VmGfssha77Y+4kBYW8Mdytbo60w5vA==@kvack.org X-Gm-Message-State: AOJu0YwcicdziFxXiEs784GZ6r6iWWk4FUKnzox/5wWkJK8ht/qTCPxy X4wIdD73w6QBR+gtsAq2Rhs4ldMooNvxTf5qqJ8AqSTt1HO3Hszn3mYC5ZDdZDU= X-Google-Smtp-Source: AGHT+IFJ7PMB0YWbSXcu0BJe/T0BQhm3CnzjPN0ToSLmEoHdAVs7NdsRrZlFVzkEuSF5l5ZKvIKWNg== X-Received: by 2002:a05:6214:469c:b0:6c7:50bf:a443 with SMTP id 6a1803df08f44-6cde1583893mr60176556d6.30.1729291143089; Fri, 18 Oct 2024 15:39:03 -0700 (PDT) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6cde111ccd4sm11346246d6.26.2024.10.18.15.38.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Oct 2024 15:38:59 -0700 (PDT) Date: Fri, 18 Oct 2024 18:38:55 -0400 From: Johannes Weiner To: Kairui Song Cc: Matthew Wilcox , linux-mm@kvack.org, Andrew Morton , Yosry Ahmed , Nhat Pham , Chengming Zhou , Chris Li , Barry Song , "Huang, Ying" , linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm, zswap: don't touch the XArray lock if there is no entry to free Message-ID: <20241018223855.GC81612@cmpxchg.org> References: <20241018192525.95862-1-ryncsn@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: E0696180003 X-Rspamd-Server: rspam01 X-Stat-Signature: o9fona9ogz1ojwsafzufobhrf8wga5mo X-HE-Tag: 1729291141-202274 X-HE-Meta: U2FsdGVkX18EcClgnrOFhaHUBKTGgnf3vfbDFtIw/IbrN/pbh6YRBkSptmpI/9+23ydVdZHBQ9/p9f6SGqepLSScY+tpGyKrhJjY9SrDtmJHQzuQuW0mPNAn11qV+IVGFE2HO0SKEir+osHqzMyi03tUENkBYniN6o+LxJcFz1eIhM0RTVbm1j1C7YpDhnHpKZVSMZmdYdyqr9KtxXta4Z3kDxas4Z1ND0/+nC7wA825klZ2xCm0XuhS1JUSCR8RTmzWcIUxXurHXHP6kOuSfXkCoqpDTfD3RumahQrmrhaVpsSaVIYtWRlZjkm202mAiBcW09hun1A4dx1ELPpZsW3cSu0i3ffLPtIsu4ybjomazixqsxySGZ6Kw/YRi4KPEr+woJIrLiHt0WJFI0Oxe0jFHzQoYMmCHDUCW2OXo4C7yNPX7UstUy/PpAvbQfcRUHJ2lj5TFHKUj35EZHkI8TQT+JSU+M99K46l5woY9JKLnIaFWsWXf45rwFbPM3EfFazDIaKj/rOBH8qmKNNiDWuibYQ9qxlhi4s/Lv4n6l86ffR72nJ1iu4HkeSaHUeEkVfpenRob8rL/pCVv73yhBSIz+HtocgHwAqteHtkcF0EfDilIyfVPfkJ75aMv7JGwDfShkaF12CmbhW+reE9FgCfHwklug+OvQGORHhl/GTgcZTQC57oNTdnV/w40dEjkU6hMOhWCuYWpU8Z2htvHdK3CvGmoQhtRP09liXGOasUMCzUwwNWlK+zyQjrFquQqRXmarO1DYqzIZaoDpzoA2+prTcaCiJyY8a7yzUtyefItEtTdb+40ujBSTXoH1hroSD2/QQQ+hclU2J17T0FtwutDSEJJG0LJBECi3+bAfYEW0ToR5fEB+RXmOr50QaRULQV/ibJLLc0DZ049+jgtJpz49DZEBsqYDP9nEJfAC6I3eGjok/Ffe/X6L7kiqffGL88+KsF9yO2If0ISJf 5HoC9HvD ZJgMTdJl8u1mqwYR15X1f/+65WnwaEfClMZYPHzI1HFsk639q1OkiFLtdDTK8dU3aEgkA6xOt5FdMfPmczMZVr1i6Hxv5byD+56V6EwbFwvP1E6P0KybFgZXIA5kyICUSeAypFH4cZWaV6/bLphhoQD1uUVKz3LInBW+zGvIJIPy/wxVKlB5HM4+LDniV6RTSSpEiYkRKRhII89xfPILR1EwbOsAJHEd9/q8JWUtJ8R8pD/IZupVksTJLIZdV1r64QSHZRe3YPTQBSDPGlDkGvaOcQG9dH56df94rx7EsrCrDplZZzbDvXkZe3emjNTJbLdt1oKycc0fic7o6R9YdANhsMAtBffUhzhJTZWVmKKz6SPSuvF0jvoLGSchHLIDFsSms9xY57y7tQDQ9m9biGWDhkhTVJVD+L5K5M7CjcAWPvPcbYVIaJLdt0UJ3pLMZzfRyq3Bxh0KNeM5KO2+GGu1Wv4mLmJQYrCYMBZg9H0qyr4K1hDpyjr4VUrV2VGlvwJpXYHjJE6MOMuZXqi+J7c1f/EuJhBVmG+R68sKk5Ual4/T1htYlpttnBD6uQd5Oa3r0 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 Sat, Oct 19, 2024 at 04:01:18AM +0800, Kairui Song wrote: > On Sat, Oct 19, 2024 at 3:46 AM Matthew Wilcox wrote: > > > > On Sat, Oct 19, 2024 at 03:25:25AM +0800, Kairui Song wrote: > > > if (xa_empty(tree)) > > > return; > > > > > > - entry = xa_erase(tree, offset); > > > - if (entry) > > > + rcu_read_lock(); > > > + entry = xas_load(&xas); > > > + if (entry) { > > > > You should call xas_reset() here. 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. Spinlocks already implicitly acquire an RCU read-side lock before beginning to spin, so we shouldn't be worse for wear by doing this. > Thanks for the review. I thought about it, that could cancel this optimization. > > 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. This seems fine too, exlusivity during invalidation is a fundamental property of swap. If a load were possible, we'd be freeing an entry with ptes pointing to it (or readahead a slot whose backing space has been discarded). If a store were possible, we could write new data into a dead slot and lose it. Even the swapcache bypass path in do_swap_page() must at least acquire HAS_CACHE due to this. So from a swap POV, if we find an entry here it's guaranteed to remain in the tree by the calling context. The xa lock is for protection the tree structure against concurrent changes (e.g. from adjacent entries). With that said, is there still a way for the tree to change internally before we acquire the lock? Such that tree + index might end up pointing to the same contents in a different memory location? AFAIK there are two possible ways: - xas_split() - this shouldn't be possible because we don't do large entries inside the zswap trees. - xas_shrink() - this could move the entry from a node to xa->head, iff it's the last entry in the tree and its index is 0. Swap offset 0 is never a valid swap entry (swap header), but unfortunately we have split trees so it could happen to any offset that is a multiple of SWAP_ADDRESS_SPACE_PAGES. AFAICS xas_store() doesn't detect such a transition. And making it do that honestly sounds a bit hairy... So this doesn't look safe to me without a reload :(