Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.700.6\))
Subject: Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix
 anon_vma memory ordering
From:   Nadav Amit <nadav.amit@gmail.com>
In-Reply-To: <2229115A-23F2-4B6B-800C-7182199DF79D@gmail.com>
Date:   Fri, 28 Jul 2023 02:18:20 -0700
Cc:     Will Deacon <will@kernel.org>, Jann Horn <jannh@google.com>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Suren Baghdasaryan <surenb@google.com>,
        Matthew Wilcox <willy@infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-mm <linux-mm@kvack.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Andrea Parri <parri.andrea@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        Nicholas Piggin <npiggin@gmail.com>,
        David Howells <dhowells@redhat.com>,
        Jade Alglave <j.alglave@ucl.ac.uk>,
        Luc Maranget <luc.maranget@inria.fr>,
        Akira Yokosawa <akiyks@gmail.com>,
        Daniel Lustig <dlustig@nvidia.com>,
        Joel Fernandes <joel@joelfernandes.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <802750CF-1C45-4F10-920A-CE7C55DBF3B2@gmail.com>
References: <20230726214103.3261108-1-jannh@google.com>
 <31df93bd-4862-432c-8135-5595ffd2bd43@paulmck-laptop>
 <CAG48ez1fDzHzdD8YHEK-9D=7YcsR7Bp-FHCr25x13aqXpz7UnQ@mail.gmail.com>
 <20230727145747.GB19940@willie-the-truck>
 <8EA729DD-F1CE-4C6F-A074-147A6A1BBCE0@gmail.com>
 <CAHk-=wgeMOvL=ko_eruf5St7h0e9dVn4+GMKjcdshTJoxyD8tw@mail.gmail.com>
 <2229115A-23F2-4B6B-800C-7182199DF79D@gmail.com>
To:     Linus Torvalds <torvalds@linuxfoundation.org>
Precedence: bulk


> On Jul 27, 2023, at 1:11 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
>=20
>=20
>=20
>> On Jul 27, 2023, at 12:39 PM, Linus Torvalds =
<torvalds@linuxfoundation.org> wrote:
>>=20
>> On Thu, 27 Jul 2023 at 12:10, Nadav Amit <nadav.amit@gmail.com> =
wrote:
>>>=20
>>> Interesting. I wonder if you considered adding to READ_ONCE() =
something
>>> like:
>>>=20
>>>       asm volatile("" : "+g" (x) );
>>>=20
>>> So later loads (such as baz =3D *ptr) would reload the updated =
value.
>>=20
>> Not necessarily a bad idea.  Although I suspect you'd want to add
>> *two* of them - on either side - to make sure any previous loads
>> wouldn't be moved around it either.
>=20
> You are right, two are needed.
>=20
> I=E2=80=99ll give it a shot and see if I see changes to the binary.

Just for the record (so nobody will make my mistakes):

I implemented it, but it works poorly. It appears to communicate the
constraints but the generated code is much worse.

The problem is that if the GCC inline asm decides to use a memory =
operand
(e.g. =E2=80=9C+m=E2=80=9D), it computes the address (uses LEA first =
before the MOV) and
allocates a register for the address. Using =E2=80=9C+g=E2=80=9D also =
causes the compiler
to allocate a register.

-- >8 --

---
 include/asm-generic/rwonce.h | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/rwonce.h b/include/asm-generic/rwonce.h
index 8d0a6280e982..c6a2f8e3013c 100644
--- a/include/asm-generic/rwonce.h
+++ b/include/asm-generic/rwonce.h
@@ -44,10 +44,34 @@
 #define __READ_ONCE(x)	(*(const volatile __unqual_scalar_typeof(x) =
*)&(x))
 #endif
=20
-#define READ_ONCE(x)							=
\
-({									=
\
-	compiletime_assert_rwonce_type(x);				=
\
-	__READ_ONCE(x);							=
\
+#define as_scalar(x)							=
\
+    __builtin_choose_expr(sizeof(x) =3D=3D sizeof(char),  			=
\
+				(__force char *)&(x), 			=
\
+    __builtin_choose_expr(sizeof(x) =3D=3D sizeof(short),			=
\
+				(__force short *)&(x),			=
\
+    __builtin_choose_expr(sizeof(x) =3D=3D sizeof(int),			=
\
+				(__force int *)&(x), 			=
\
+    __builtin_choose_expr(sizeof(x) =3D=3D sizeof(long),			=
\
+				(__force long *)&(x),			=
\
+    __builtin_choose_expr(sizeof(x) =3D=3D sizeof(long long),		=
\
+				(__force long long *)&(x),		=
\
+    (void*)0)))))
+
+#define READ_ONCE(x)                                                    =
\
+({                                                                      =
\
+    typeof(x) * px =3D &(x);						=
\
+    union {								=
\
+        typeof(x) __orig;						=
\
+        typeof(*as_scalar(x)) __alias;					=
\
+    } __u;								=
\
+									=
\
+    compiletime_assert_rwonce_type(x);					=
\
+    asm volatile ("" :							=
\
+               "+g" (*(__force typeof(*as_scalar(x)) *)(px)));		=
\
+    __u.__alias =3D __READ_ONCE(*as_scalar(*px));				=
\
+    asm volatile ("" :							=
\
+               "+g" (*(__force typeof(*as_scalar(x)) *)(px)));		=
\
+    __u.__orig;								=
\
 })
=20
 #define __WRITE_ONCE(x, val)						=
\
--=20
2.34.1