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 97B39C47258 for ; Wed, 24 Jan 2024 03:20:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDEE56B0078; Tue, 23 Jan 2024 22:20:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C67F06B007D; Tue, 23 Jan 2024 22:20:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B08336B007E; Tue, 23 Jan 2024 22:20:58 -0500 (EST) 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 9B0D46B0078 for ; Tue, 23 Jan 2024 22:20:58 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2F87B40207 for ; Wed, 24 Jan 2024 03:20:58 +0000 (UTC) X-FDA: 81712752996.17.FC17851 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by imf11.hostedemail.com (Postfix) with ESMTP id 6BBFB40007 for ; Wed, 24 Jan 2024 03:20:56 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="RW1/o9q+"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.48 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=1706066456; 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=KAePt3EbboVv3TxbwUCofAP8uUAMMkWJoXv+3X6ccPI=; b=yGDTo2U8eKV6U4Dw/2g2DSGf7vUlhztEnr7GHwI5hTNbqZevTCH5Tt/XN+Cnx1nb83Zq9c T7PjIHw/cm79O+fp3gplW7OdKMENlzK1PQDtyU+aGKsM6T5z/x+2hlj3w9NWV1NGrI0y58 g4DoUbmRrfMTIviCM3OB2DFJ3YApieg= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="RW1/o9q+"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.48 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706066456; a=rsa-sha256; cv=none; b=uYrh0SUs5Qkq5plquAtpi4aJKOkd+z9LpYwnHOZwFmgQl0sAgG2vrRO1BVhBLS/nZ+MPR5 aOg9w2463sufdFSjpdEjdb3+BxaXWFluvD+W4HKfUIJ9cfUb3dxzZ+Hh31LukB3BPGlVVF HHptk+XO1G48flTP90keme/WngWmsbc= Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-336c8ab0b20so5048270f8f.1 for ; Tue, 23 Jan 2024 19:20:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706066455; x=1706671255; 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=KAePt3EbboVv3TxbwUCofAP8uUAMMkWJoXv+3X6ccPI=; b=RW1/o9q+8X+iOTpp5M0SFD3Y6KsTdS+Ue4/kbDGu+5I6INIoGFd5b18I45PuDB0OS9 7FO0GIMcXPqaDnAkn43vSvavAulkuxuQyy55ApI4OxvxZG3UnH3zpZdPGsGGeqpJ0DjJ fp/Zzmr3oyvE5Aek3lHS2z+Pj0JZu0L/KI83megbYsqbyCduseJQh7nuugvfP2aq/zBS z1GfCnyrgAMYhiV3VfB96+/2sO8uJiOwmmsdWLkcYAU77dFXgz4qISxNJgGfzbxAg4Jo j1oGyxG6W1O2W4UP7CiI9dGaFEt6VMPrI+W606JuLXmR34BMmvFDhWDJNquL2oRmhBCl CPnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706066455; x=1706671255; 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=KAePt3EbboVv3TxbwUCofAP8uUAMMkWJoXv+3X6ccPI=; b=JVDtVO5M6jLICk6Gh/NqSGezrEbT5OXOhwnWgoFHTWw/TpPPPAbyPKIV8t2DNxUKiW 2wRN8DoHRzqudDvLbps7YaEH8uVfk7X3Bk1e2Cu6Oj4MdgfIye818q3TApl6GnttoU44 3oviEcIl/6NJ7++VO/pLoAe/zBZPPuY2aEc4Y8Utr+k0aO8BWc62NycdioxBihukI6wC bFRddufIKQUYhqrIDztDLerl9yyWigakd4fGsjWCQR4EnGb21DtXWO0pD6B5EhpNwyj2 A1WlT4+gQCnn9HZChMPDtxIyE1bzigJwA6jgghQv5yi5MyE3o3YJ4Eyat/hjqIsw/lB0 HSig== X-Gm-Message-State: AOJu0YxNZqpdRsDZ36/rPAtI/qGw/miC1b+r6LgWWk/B61axVvylCLsU /sNEbR63KIyPpShlzWfddeaNzLWBfG5kIsDyOuY/KHUlTm6c0+f/scLF/gOVZAqZ9F4CcDWvbTo B/CjCu6iRNIbIUC3iKc9kWk7vO8goK4x3ptrD X-Google-Smtp-Source: AGHT+IG52SOXahv8KuKJM5amdkEJrHh+itGyuLplJrEk2YxxxRBnMyB3thEL7drxZ7rWTHHm76P8VMONmx+BrYEP4UE= X-Received: by 2002:a5d:448a:0:b0:337:c6b6:7e30 with SMTP id j10-20020a5d448a000000b00337c6b67e30mr124733wrq.36.1706066454776; Tue, 23 Jan 2024 19:20:54 -0800 (PST) MIME-Version: 1.0 References: <20240120024007.2850671-1-yosryahmed@google.com> <20240120024007.2850671-2-yosryahmed@google.com> <87wms0toh4.fsf@yhuang6-desk2.ccr.corp.intel.com> <878r4ftodl.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <878r4ftodl.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Yosry Ahmed Date: Tue, 23 Jan 2024 19:20:17 -0800 Message-ID: Subject: Re: [PATCH 1/2] mm: swap: update inuse_pages after all cleanups are done To: "Huang, Ying" Cc: Andrew Morton , Johannes Weiner , Nhat Pham , Chris Li , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 6BBFB40007 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: tpsw3uktbkkfg6er7mtbjc4fxo6fba5b X-HE-Tag: 1706066456-16134 X-HE-Meta: U2FsdGVkX18eCfO4Ftg4RODXNT+etwDDBxgIz3x/VvI5KKTAmPhnWo1pMAqaIIvl+VaXVEn3c9r3mjZUkUej3tb5jLSAxzJIXzMeNS1ZpZwB1AX0ZnAOrRAgma8BPopSPiAfazdLzNB9hylYT+LqdYPQgP4+flRa2A9spHFKxuwzeL9izQxu4CAVDqlei9zUCE8FLmia6O/47Z3aeSk/VJcez+PKLW2kd/DTTi58nz0EPSIZ1oxpd0VzD6hE4ASBAQYVqE3LkfjX9EMTKMVR5zwyqi7cX/e+7yE9D+/RQx7YTkF/7S0f1GuXNrwusRrtmb2aP34tM6zq/Rzgivdec6NCQP7LClRyxWJ7ab0ls5EFG228/JvEfs5vQ0KLxLhnwTrZfD+TcHQ4L1THFVZHx/48OPjkKB4jF7cpc9vSNKG0bLfRDvppmu7UIRzOIWUnBstV+FIdL9r+YJqi7MjvZS11diNuozWKo4/n1YnamrAG1LUl6TOCgydgl9hHqmIHC86n4Ki+fLDtPstBrm0fekYj/TkVzuZt7MdAq7zDgfCmWGaK3belAMjsfWGwjEYTwMA9/5iG5nAVaKiZukY7ISDjlBWUFiDr9XmdZKe96ua18rm7+rGWDIYtm1nKTbb+cnL1s41h7By8mkwjF5tGV5HyID4vOiC9urpGw10Ck61Mf3J0vImDJo8pZsq5Ed2yn/aa6ZF4KPib13OhRUTII7w3a+iDDKuAw08vH2aXyrpXV0pAf6B+xJmB+OuGkFA8ObS1Jfm6ExhK/tTX7g0s2r30aIqyNC5h26ufIdG9tFnoLFFNhhWZTH04BS4TaO3nk+3/rmVmE9MHKXFvBjIzj9mSldf/5MWpwfMcM19WTpA2ewzn3WgO8QQewW8yfBJc5j+UXL2ard5Itl0cVDjbzQtioGogFQCH87rUN3eBED9gW4UgXwuE5Fj/5a/qzv69XNWjLDR3oAb+FZhZRSg CB8h338l /4GIMS0ljN9xU7w+ZeNwT1LKgfDIHPOwxRN+SsI4QDz0A1zWwJko67d2GBTexwaLO7LPCAsoPogrjRsQOy15tuvkAXRXG3sy/xWvqQ5ACXG3F+xT95CvJzNGT/kWtsDDni4sPPPc1u2xUKBZ4xVvyJZyWM1w0wUnVQTz8Xske5rHE7MSATXTsuUwRwn5N5ghj1VOfu1xUx9UsTN9xkwJXJgoAI74IkJWRNe6tJBrcelHQJbI/Q040Scy4X9ETVJMxqlrSseS2QmgQx9M= 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: > > In swap_range_free, we want to make sure that the write to > > si->inuse_pages in swap_range_free() happens *after* the cleanups > > (specifically zswap_invalidate() in this case). > > In swap_off, we want to make sure that the cleanups following > > try_to_unuse() (e.g. zswap_swapoff) happen *after* reading > > si->inuse_pages == 0 in try_to_unuse(). > > > > So I think we want smp_wmb() in swap_range_free() and smp_mb() in > > try_to_unuse(). Does the below look correct to you? > > > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 2fedb148b9404..a2fa2f65a8ddd 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -750,6 +750,12 @@ static void swap_range_free(struct > > swap_info_struct *si, unsigned long offset, > > offset++; > > } > > clear_shadow_from_swap_cache(si->type, begin, end); > > + > > + /* > > + * Make sure that try_to_unuse() observes si->inuse_pages reaching 0 > > + * only after the above cleanups are done. > > + */ > > + smp_wmb(); > > atomic_long_add(nr_entries, &nr_swap_pages); > > WRITE_ONCE(si->inuse_pages, si->inuse_pages - nr_entries); > > } > > @@ -2130,6 +2136,11 @@ static int try_to_unuse(unsigned int type) > > return -EINTR; > > } > > > > + /* > > + * Make sure that further cleanups after try_to_unuse() returns happen > > + * after swap_range_free() reduces si->inuse_pages to 0. > > + */ > > + smp_mb(); > > return 0; > > } > > We need to take care of "si->inuse_pages" checking at the beginning of > try_to_unuse() too. Otherwise, it looks good to me. Hmm, why isn't one barrier at the end of the function enough? I think all we need is that before we return from try_to_unuse(), all the cleanups in swap_range_free() are taken care of, which the barrier at the end should be doing. We just want instructions after try_to_unuse() to not get re-ordered before si->inuse_pages is read as 0, right? > > > Alternatively, we may just hold the spinlock in try_to_unuse() when we > > check si->inuse_pages at the end. This will also ensure that any calls > > to swap_range_free() have completed. Let me know what you prefer. > > Personally, I prefer memory barriers here. Ack.