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 E0E0CC04A6A for ; Thu, 27 Jul 2023 15:44:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 38C496B0071; Thu, 27 Jul 2023 11:44:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 33CB66B0074; Thu, 27 Jul 2023 11:44:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 203ED6B0075; Thu, 27 Jul 2023 11:44:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 0D8016B0071 for ; Thu, 27 Jul 2023 11:44:08 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8D6A31210F9 for ; Thu, 27 Jul 2023 15:44:05 +0000 (UTC) X-FDA: 81057812850.28.5D14A02 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by imf30.hostedemail.com (Postfix) with SMTP id A998480012 for ; Thu, 27 Jul 2023 15:44:03 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=harvard.edu (policy=none); spf=pass (imf30.hostedemail.com: domain of stern+64c0f95a@netrider.rowland.org designates 192.131.102.5 as permitted sender) smtp.mailfrom=stern+64c0f95a@netrider.rowland.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690472643; 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; bh=UFRj272CuwjkZGW2q7rl0YIToVJlgjtglg3bGRF1LKA=; b=XY6NPc37/ntaNJTDV2IlyMhAI0NXOt7TPWEAoTa8Lk9EejPsBwvgPATnsZb3+iAHDdxXUl m+i0pR2nhmR3pl3U9E5WOCaTpBRIO2tIjTi4f6y3YHsxYCUSObsAhn0uTwmPQeBw+b3sL5 j+mUYWX79x5EXoGtfajwKtUhIQ2B/xY= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=harvard.edu (policy=none); spf=pass (imf30.hostedemail.com: domain of stern+64c0f95a@netrider.rowland.org designates 192.131.102.5 as permitted sender) smtp.mailfrom=stern+64c0f95a@netrider.rowland.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690472643; a=rsa-sha256; cv=none; b=qNv4sYc6gNyywZtjZ04D22AVcz9/wIu+0VMAmBX8LBH8msjxT+0Bth8v426NuN/2e49pV3 0fPQK5jTej6pSnNSqUl5eIIQaE3bcgO7M99Vvsn3PG1CDPBo23WlK3ZVuf2yg9Jqwb21HW pVWfCePPuhmUhNvuMsqePGsTsyPTKi0= Received: (qmail 1994132 invoked by uid 1000); 27 Jul 2023 11:44:02 -0400 Date: Thu, 27 Jul 2023 11:44:02 -0400 From: Alan Stern To: Will Deacon Cc: Jann Horn , paulmck@kernel.org, Andrew Morton , Linus Torvalds , Peter Zijlstra , Suren Baghdasaryan , Matthew Wilcox , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrea Parri , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , Akira Yokosawa , Daniel Lustig , Joel Fernandes Subject: Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix anon_vma memory ordering Message-ID: <13dc448b-712e-41ce-b74b-b95a55f3e740@rowland.harvard.edu> References: <20230726214103.3261108-1-jannh@google.com> <31df93bd-4862-432c-8135-5595ffd2bd43@paulmck-laptop> <20230727145747.GB19940@willie-the-truck> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230727145747.GB19940@willie-the-truck> X-Rspamd-Queue-Id: A998480012 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 5yesbff6jex9hsukwzq181irkw8u8f3k X-HE-Tag: 1690472643-135969 X-HE-Meta: U2FsdGVkX1+83ODkwAscZXaGRbs28SGuV6YOq29gVjlNcxBAhRecFDwNpWYk/IffjhzysslPnPVqwKSZwWhdB71CsqU1SeJ+CUv0lA6dqumv14g8aAGdmTMBrPXpryVeB/M8GghriBJH9sVtv0ZfwjPeuFCju6hW2CThBN4pxvzHDfzJBq4/BbAelgnlNx3T1ecJlvmmvHpZC5Z3Ea0vm7f1flnnvpGxU1gGAS6I6txlzqXm4w2xK9dSyU+UpSrEz8QpGn7jrKU8FApJSDFDPkFCoVAHDd8GbXpj5T4lD+OKeA/9xaxFSuZWF6kx0xV1re5iRI4gnYb5O2Nbfb2J4DWI5cgVuF0mLE5uKu8IiraLX4XVAOTvxMeNlzdXJ/fJGdBGYC9nZvRSA3m4P4nLDr11mtgmRsC+DCwF1s5RS5bONOlsZlRdAKMUt4Iw11hN4ifj/dW3ANENU3xsmgjpgnzd5Sim2+4Xpt1u22pGQvIk+OW6VKqXmHI6BNpXqNcm5nNbxO2jVCGtRBaHUG0ofgDT0WqWftMpqVUJn7vHRO2KKKDziew7lXd9i4AogA5pXIFk+OnFoaoCKeNUyCQ7d1JRepYqYGPyuDepdFVoFrD+ziAxMdMmRig7QbtJlc+3Jfe3eB+M8lSwhj4m++/euUCc+yWq1RV5mAL0QkJotqSD7W8CVR+meZMXi82rFs1kjpDWHBzzVVjKi3BENdBm/DlMVecr9Stj83XS7QxJT0L+e0IL4UYuf4WAuIKYten6iEd+HxLCMr4a/xe4OSMy5ZAjYfdkAN9ZjZPW1wBpJXq+4NtoerqVNGAfZOU4Vh0vzB7vE6+lAQE06ZlTqc3xpIKwIQhFDX15Z2I13dL1P+9W558o9dKTDw+Z+naEDOvkxo+UKSQJkFv/KuU3zL5RJ/BKw3YrNLZiwfqU2RPdLTAwabdcDS/6Xumr4Dy7J3IvOnPozyX8xRNmqTPSbAM z23qXw0G 0TzvoUTKaSaiW0xBHGy0dFpAqJq6g17DVh1T1l2ldSoHh6s4bvt2DsYEheqDypKhq2fnKcj06zrEAbxQIAo/HLNpIehDzNuQIkPZdyV8McYuUDTVNzmjU/I7W8H7sC3Wu6t7fMtDo3HhQufgzUgJ5IqEa9xKgufvvQERln+7HUzJO4bv0YFCtz/E3UZ1bLyCR7W/QH+9RSVcNSA8TG+dMKNcVRlSE5K8/C2tM9+zDdTQlh4PT7idWK1cP9m/aRTVYrI3KENI2w7dtnFTo9IT5MyOqiu9F+8O/7YzmsBZLvLdx0FCreecoKFSO15vJvYugAZhshG3XNycNZSEc2nSNl1y4a7NtpEYNu0HsUoQWHDrpWz0FBfBUk5+DlsyEFsjT7goNMRQgC5/SW1xUQq95z9sxWL1oX9XdClyhN9Y9z4lkkcE= 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: On Thu, Jul 27, 2023 at 03:57:47PM +0100, Will Deacon wrote: > On Thu, Jul 27, 2023 at 04:39:34PM +0200, Jann Horn wrote: > > Assume that we are holding some kind of lock that ensures that the > > only possible concurrent update to "vma->anon_vma" is that it changes > > from a NULL pointer to a non-NULL pointer (using smp_store_release()). > > > > > > if (READ_ONCE(vma->anon_vma) != NULL) { > > // we now know that vma->anon_vma cannot change anymore > > > > // access the same memory location again with a plain load > > struct anon_vma *a = vma->anon_vma; > > > > // this needs to be address-dependency-ordered against one of > > // the loads from vma->anon_vma > > struct anon_vma *root = a->root; > > } This reads a little oddly, perhaps because it's a fragment from a larger piece of code. Still, if I were doing something like this, I'd write it as: struct anon_vma *a; a = READ_ONCE(vma->anon_vma); if (a != NULL) { struct anon_vma *root = a->root; ... thus eliminating the possibility of confusion from multiple reads of the same address. In this situation, the ordering of the two reads is guaranteed by the address dependency. And people shouldn't worry too much about using that sort of ordering; RCU relies on it critically, all the time. > > Is this fine? If it is not fine just because the compiler might > > reorder the plain load of vma->anon_vma before the READ_ONCE() load, > > would it be fine after adding a barrier() directly after the > > READ_ONCE()? > > I'm _very_ wary of mixing READ_ONCE() and plain loads to the same variable, > as I've run into cases where you have sequences such as: > > // Assume *ptr is initially 0 and somebody else writes it to 1 > // concurrently > > foo = *ptr; > bar = READ_ONCE(*ptr); > baz = *ptr; > > and you can get foo == baz == 0 but bar == 1 because the compiler only > ends up reading from memory twice. > > That was the root cause behind f069faba6887 ("arm64: mm: Use READ_ONCE > when dereferencing pointer to pte table"), which was very unpleasant to > debug. Indeed, that's the sort of thing that can happen when plain accesses are involved in a race. Alan Stern