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 7199EC46CD2 for ; Wed, 24 Jan 2024 04:16:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D2B428D0002; Tue, 23 Jan 2024 23:16:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CDB848D0001; Tue, 23 Jan 2024 23:16:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BA31F8D0002; Tue, 23 Jan 2024 23:16:16 -0500 (EST) 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 A9FEA8D0001 for ; Tue, 23 Jan 2024 23:16:16 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 6DFB540223 for ; Wed, 24 Jan 2024 04:16:16 +0000 (UTC) X-FDA: 81712892352.05.9515F08 Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by imf19.hostedemail.com (Postfix) with ESMTP id 940321A0007 for ; Wed, 24 Jan 2024 04:16:14 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="Wb2/e/Dn"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706069774; a=rsa-sha256; cv=none; b=0XLx6idziet2Lm6evAKGd8qjwRTH/1OiKlHn97im5mEJApk/tbGap6XzBcZIenE5IBlnFy wC7CsE+5vYMYmi2KlRAt2Xb9btQqIvPoanc6Lg5EvDAUE63IAmxLqv/UhJc1LiiHFc/roT hvKoXYdTSOg//6hNGurJDKUbnhQBwcU= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="Wb2/e/Dn"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.47 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=1706069774; 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=0Qab8SRWACvf6cv/ZfXhgOQYzxgAOfcaBZoqSpTBiLQ=; b=5A+9TZISFeu8tV9oia5EYuWmbj1A6kgHxv6nkSNSUL6THRzFkInL30rhIvAhR0MCI7WuKo gjc1kQwQIhQcMT48y9GemMGzPREBXcBDwj+Q0mAbAsi3FZ+dSaSvha6kWBHReIDaRttnUV an95192xM72AD9sZQdPO25N69eNQdAc= Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-50e7af5f618so5763975e87.1 for ; Tue, 23 Jan 2024 20:16:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706069773; x=1706674573; 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=0Qab8SRWACvf6cv/ZfXhgOQYzxgAOfcaBZoqSpTBiLQ=; b=Wb2/e/Dnvqg1GO397i1NE4EgxZ2mQGu90bdPr3WIeIftDxiFwpuMo5FpxM7MsCCPfv SMMEZjoE6TgbSkIzyKkUod4cY1NF+B1ftZxc+6M64u6vFLN9jyJPEUy0dCjtfo+Ettaj EotXR2EmCTUqRI677Pp11kWn8mO51Ak6FyzS6G5ws6BPHanf9oyX/mq2egYTloBhmmgR oWa8F7cASK9eDH1hq3/bIfjgu3iL4cYGH3gK0B7UiegSZlMti0J7r8a3MXYZeAxAofF6 HAPi3J7tYTsH04EH3FDMD9emDu2FTEbzSZlSXfHtKDh5dSLP4dqyds51kb7fDLayNY2i q8Aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706069773; x=1706674573; 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=0Qab8SRWACvf6cv/ZfXhgOQYzxgAOfcaBZoqSpTBiLQ=; b=cyBnSLWCqCbofVVXFNOrwXEyzg4f02+AVAuiLi+tjNipj0spp5ISbV+JdkL4oJ7xbE 5i89FUu5rPtrpV2HjgOO/mpP6dX5YNWBuLSX6xWebee2V9pLmm07wFJrVgP9I0PX56lB qRK1QvsLF8IMYnld+s2uqoF0k17IehirOqJHXTg+5v+ll+mdcrg1yKtYgONYWhDAlWlW 0CP7c22/RqHFfxZ4BzzM+vqXW9AXVwuew8air03OBVXpOiYp7ya1kJUsWficXpYJNWWC IaeP7eIqsRTWV1bAU7ORAS8ga7keF8JnQVUtOGwQETpluWHYXns0zaCPGt+oZ1rZ70wr /yMQ== X-Gm-Message-State: AOJu0YyQJxXY1w2geUKwtK1zF+UD+2BDTGa1SlAm1TX0IxQT1TwqAM0A wwIm+cBYdFG67ZY0xbiBKEiY+u6C3GcWP11hjOEvNOsQhGe5A9nQKaJ348c05cXMeGGqQ83OvuS gcby3LrIHOCaHfYshUuaMY2KyzpyqLUnX/aS0 X-Google-Smtp-Source: AGHT+IFVgSEvSMkCTmyC8Ljl0LoXmFmNHu+mQPfw91AtNRoOEzESlE289mx7tNZkHE//cn5CCuSPPV65dolc8E9vDA8= X-Received: by 2002:a05:6512:685:b0:50e:594d:3533 with SMTP id t5-20020a056512068500b0050e594d3533mr3810083lfe.106.1706069772716; Tue, 23 Jan 2024 20:16:12 -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> <874jf3tnpf.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <874jf3tnpf.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Yosry Ahmed Date: Tue, 23 Jan 2024 20:15:36 -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" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 940321A0007 X-Stat-Signature: ibdixx9z75t6tertrxd7skoofwaenp4p X-HE-Tag: 1706069774-727298 X-HE-Meta: U2FsdGVkX1//QEfomefm81mb52HQe4eUjsz3VMRjGhsbJhBR0rB3yQCtZKWbbpHnOdqyY146hX+qFSXQo/9UP+Au/p8QSim482Q6Oaf7EcuMQh04irpNMUzCp7vi1tOYma3aFlTBcMNrFb9qdCnqYzH0GAFvrXuHR0uXrgVf0gx+jw0Wvft74BsDbgVmxBoY156AvqX2PiLRCl1F2bA7d0lmQuVnkmKSeBI6tu9rZSwtV7FAXL+5l2AkpiyYXiHzhaKq2keYBBfeQQxxm4Xw6Vdl4NXnVYUwnAqWEiD7sGq8Ff0/Q5TY6L2VXrR0MrTDmvAo8zco6dbXQHUJtqo049sitalInGuQPwCqKDeDMILo4Cz31CKlddkGaOX8xq8noy0roiIytrHnA4iYKuHVg+dpvjyv7OnVvbGdxLQcVMavxR+A7b2C8pODxvwQUMyyTR/mq81nQhmPYuEO7anOQqFB3/O4KShNU2K1ch15E8ayjx8L+NM3Ysqw4Lik+FPkHZoAGDRp1I0zeZljh5/sYqqS0kHqVMnMaUZz3AJ/+BLU2Y6twtQa9qzqhrhYV8gx/EWDvsYK6ZnIlsZJwi3eNxD8ENqRo/URpoFBKvLgdnTRdNA0TgcCHG6b49VrbYG2HqhcRGhdLoO3FjLXC9K8NIuSZ5SsJ6B59tWnL+kHoQRdtvgPITdVVu47KsS8UhXJGXCmEhk2TYN94GfV57LBvGj1gOSELt60kwG43awDVYdCLhyJDAZnyRikfnHymbhghblej8XhA6FEjVnOr0P0t1DKYPSgcX//n0G0MYuFyGkoNzRoo0/KqNlnwFJdWiWaO7Xm95l9AUfe8G+IEEo5dK72ZETqoPk7+Iyg0LVtcnE6TaEXsoR3AGzQGVXvSsog7AGZMb+xv1e4BDPxhxEBiJDXDT7OL0YpXRbdo+5eXoGA6FFK0YlCYlzT7kQpifybM1OWZe7HGKKWq3ub3VG roL/P9Cs 6Ln1oFBS6gN5NciJZphUSwluygZlL6I/iLFgUij81IaMmqwku3Ff8yWZUec9XcZnmlSlryR11Lg6EmTkGwb6ILxREniSQZZ0cPbFVBY/5Zd/8lMofhc3N6EFP1hPVDTVa+xZyvZV+fqryx7+lKAgO6ERPwt+htayDUrarEqQw9XkD9HV6BeSmYr5w0deVgLT7nZxmC6wYkPw1xnBzGYqJhrHotMSZ72fGnNVuMYICd8PsYPQUHE9Luv41N31L9r0FYnSLfudXgVU7Kt+NsZE3Q/VO812Dwb8wGlsxF2csnnaUcFgDo4ubuzON2pWxjeVU8JGd 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 Tue, Jan 23, 2024 at 7:29=E2=80=AFPM Huang, Ying = wrote: > > Yosry Ahmed writes: > > >> > 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 =3D=3D 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 re= aching 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() retu= rns 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? > > Because at the begin of try_to_unuse() as below, after reading, function > returns directly without any memory barriers. > > if (!READ_ONCE(si->inuse_pages)) > return 0; Right, I missed this one. Let me fix this up and send a v2. Thanks!