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 71C33C54E58 for ; Wed, 20 Mar 2024 10:08:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 036CA6B0095; Wed, 20 Mar 2024 06:08:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F29006B0096; Wed, 20 Mar 2024 06:08:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E17A76B0098; Wed, 20 Mar 2024 06:08:11 -0400 (EDT) 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 D09356B0095 for ; Wed, 20 Mar 2024 06:08:11 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 7C8BB1A12CC for ; Wed, 20 Mar 2024 10:08:11 +0000 (UTC) X-FDA: 81916991982.24.BE3D9C3 Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by imf16.hostedemail.com (Postfix) with ESMTP id 6CA2918002B for ; Wed, 20 Mar 2024 10:08:09 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=L0JqXuls; spf=pass (imf16.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.42 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710929289; 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=YCvQMo3ZWIjWXG5M214OiwkknCIOzKyMIbVwF3D6h1Q=; b=uk+vkyWBJFRrEt0zb8ek+y6wxVGe23Y736qJdiNYAUijns8A0kDnWzlKGrkmdOGSbhpxW/ cq4KiHvyJLDbna6phxOgLI/dZlWPCp0Xa7ai6+7QZp2L1MZrJ+ktetUe0JAyXHl4Dg93Zs Cwy6EGARv2vzwpr3kNwqL9LPzB62Jlw= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=L0JqXuls; spf=pass (imf16.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.42 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710929289; a=rsa-sha256; cv=none; b=wcNQTwwKFXsDeQpXS/ACAzLL0jpIP7Gc+9D9/zI/CSyrjHvr46yuVeJU2/iKnJjfOYQYU/ wEaa+RmKb2hKlDVFkaV4vMX7ZdVFaT6TU4iHLl9MvvxHGWq0FN7/xoy8eylxCqWWbngxUP iD0NDFXc7EDWMiu9B9Vx+mmM04boxCU= Received: by mail-qv1-f42.google.com with SMTP id 6a1803df08f44-69107859bedso45434416d6.2 for ; Wed, 20 Mar 2024 03:08:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1710929288; x=1711534088; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=YCvQMo3ZWIjWXG5M214OiwkknCIOzKyMIbVwF3D6h1Q=; b=L0JqXulslWJ9Md6Rty71MwnDkK9Fl54CAOzLK3Ze8xqkgiztbpWcXGBawYXXWvHt6E y3YWpU4RFVPqQl/l5Z1eFIfdH+0SgFCcIkS/HZnexUg1wHwxk7akejrAxbiO9GAXXHma Jc0ZZyev4rYdF3FRo99rhjMSqpoVGFZ3qAB2BW99nL/MaC1TdXXYIi+P3GNKe5SwqIof /xTl5pesPnXnZ1nvcZ0xhC1FJb6U4JO1aVbHzuIF/Rcr8rB2jw+Ho4Y4L6cvxudaHx72 sfef7quQr1PsB53t3GKPS382lMUMIct3EbJCapNXg6W03jLD7tLF2DMnP9l2yJp/5PQP s2nA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710929288; x=1711534088; h=in-reply-to: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=YCvQMo3ZWIjWXG5M214OiwkknCIOzKyMIbVwF3D6h1Q=; b=sZmp1NuYyb67z18XVlIWjdrgrqHoTDufv6Q2a0NINtA0Y+wVJDr6DSk7y2OMfHtQ8d bcyIQ69rq8VIwI9MEI74nMD2Qi5i3WdxK7LTFXG2kGDSZt9ccec6rAmR6GJqlASBP1AE itVp0DRdHh9zArAMkHB3mqUgOTxew8brwG6sJ3+8XemsG/CY++Bi2i6ooDw5POQ2m+L/ 3nE+MZbpXVa6Q9PSXp/+0KSheF/Tsiydrdkmk21ToQdigIm5nmJ8+vOdc607r+oRv4g7 ZiKQBX7XuG7C1WJWztgVB9BveIRDN7I0Kf+OMZ8Lttnl9l6NJdBZWzEcwWEPawWbFtSL wm3A== X-Forwarded-Encrypted: i=1; AJvYcCXjyo39IcDkfvPEKDmxUfnPQ0KvMxrvfG/xLMqRWemhkxL5iu5WgJ3JJEBfP7XgddqwMeOQYtCasFEcpeBLf/LnAjU= X-Gm-Message-State: AOJu0Yx6jbLBW+CESASepDyAxivxgF3/6icuh9XA+eIpaF4ZFSKLIjwa MneCGNwywYelKf0aq3vYvpq7lFwcitn93Z6im2zb4hmEMK4fasFwItlo086kzXM= X-Google-Smtp-Source: AGHT+IEWV18aYAjvZ7bxiaK/qFfoU+/v7NHBEw/3Dd1cpE57bgZLsXRcM25hVl8CrDOXyT6aQPlgyw== X-Received: by 2002:a0c:c682:0:b0:696:3620:cfdb with SMTP id d2-20020a0cc682000000b006963620cfdbmr3985416qvj.8.1710929288376; Wed, 20 Mar 2024 03:08:08 -0700 (PDT) Received: from localhost (2603-7000-0c01-2716-da5e-d3ff-fee7-26e7.res6.spectrum.com. [2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id 11-20020ad45bab000000b0069046d929a3sm7550743qvq.145.2024.03.20.03.08.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Mar 2024 03:08:08 -0700 (PDT) Date: Wed, 20 Mar 2024 06:08:03 -0400 From: Johannes Weiner To: Yosry Ahmed Cc: Chris Li , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Nhat Pham , "Matthew Wilcox (Oracle)" , Chengming Zhou , Barry Song Subject: Re: [PATCH v7] zswap: replace RB tree with xarray Message-ID: <20240320100803.GB294822@cmpxchg.org> References: <20240319-zswap-xarray-v7-1-e9a03a049e86@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 6CA2918002B X-Rspam-User: X-Stat-Signature: b1n3o93wkceqfrxzzqb3aiterdd6fuyp X-Rspamd-Server: rspam01 X-HE-Tag: 1710929289-479858 X-HE-Meta: U2FsdGVkX1/AlFe4KU2bawFFq9sljcgPp6yGlKHNdAl1zPSbv6Bxx+lc/3w9lA4c/YFV4zkMpB9no25DrMPfvm1QaWFN3+12sWxfzX72y+PC6wwMCW+SyIDzMwcHdm1fSyQscN1VG0iCRrnP1jR3Ely9zCO/M5es6hI8bKyzebfDu5aeM9DL/AIFNAzk+4awAcbfFIQEZkw6ZUpuliqtDuP4gWCviQgnz6KAr4d5s/AYyLZk7dkWPHYRPsSqCgvmD1yUMMh+7AutfHpXfa/wUKrmKIbFMl4ontzOrJi5qvOM5asQJFlkMa3k/VGpQTK9sgDH2U/NnRNRdnLRbe/v7pFOUZJn7F2dH1MrfyImn0YyEQsluxVO5p1EO862mFmkKihBmwp4WEjcMza/zu8jvdk/FL+tPLF1xPQOfzpCbucgj7BAmuXamv01L4EMq862mYp/Ir42QFRpVd2aFRjZjuPMOvFdAoPgkhYssI79wQvUVDjo+KKZpg2Ggd3iP6keDtl/lE64ncIjJzVrtas0Zf2lGNutI/77UD16sKNrQAeadc/crXFLN45BMscEH2dSMhUEBY8lsmktR/1Cn/gHxSIiVFoW4PJyIOhDlCdhPPhPRYiPXUlZ0ezNYRrSoJgwOK756XH2pks0OShKdJ/kYiaWI7Ng27zxiqWkCThDqLIyvVHouLmJ5Pb3GfuCYpU1s/OIjzkYxonWAvKnfQ8BI4s88fPleC0q4Q2rAWmp/H6kSNb9LtT8DpHiXqzNjeMS+B3rjtv25+Pi9nYYZMfDMvsTfkvEFLZkXNz+9YL/cuNGxfHKwPBdJ9W3YAdlydWsL4lqbYnmctTnX+u+3QQJWLbueaUAwcsi/oOxSwhm2K5M8MnVVha3eXAYwZEqBVnJw2Y5mfIEc3ioDOIknmRShmF6mqmLjj5oyYkpS8S22qC6F6aWBih1TCxvR459AT1/Aj7EQfYE+pXA3IULKSj 9VkYeeNm kxE+OEMc+NTPKMqb2RMtSZQulsb518ldbfi0jaK44CeJh1j9y4OoFuN5Pkn3N7N0JUsfxMkWmb8vlkCNdydr/fGQn0Pi3vwTJnRtitiP5/5I+gNxTL+mcWImpvYS8hYB3awDVP1mf+ZOuhBfkXKkvut+r4McMjQUOCDXlOLucMnno2M9p9ZsbzrLow2seTXSUFtwNaP3nvVouqcpuHFvCNkap8VLcietTUttI1DV5iSKiHT4yujcVQMBmW1QBNjxcqEYXFp29zrgBe/w+tnNiJHjbFP/nspgLV8mswCz6Q+xD3iBAJyBsOoxxm66fwQlgwoODX3ASq1ILmSu0vrI/8akAGx3P+9RAWH2U+ZAoJashzzhCz048kpDe0XP1SRs+KIFl3mt7gGFmrzI= 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 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 xarray. > > > > + * This is safe because: > > > > + * > > > > + * 1. Concurrent stores and invalidations are excluded by folio lock. > > > > + * > > > > + * 2. Writeback is excluded by the entry not being on the LRU yet. > > > > + * The publishing order matters to prevent writeback from seeing > > > > + * an incoherent entry. > > > > > > As I mentioned before, writeback is also protected by the folio lock. > > > 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 protection, > > > 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.