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 C6E56C0015E for ; Fri, 28 Jul 2023 18:03:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E490C6B0075; Fri, 28 Jul 2023 14:03:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DF9A68D0002; Fri, 28 Jul 2023 14:03:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CC2228D0001; Fri, 28 Jul 2023 14:03:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id BEA896B0075 for ; Fri, 28 Jul 2023 14:03:26 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 676E6B2BEA for ; Fri, 28 Jul 2023 18:03:26 +0000 (UTC) X-FDA: 81061792812.03.6F31A43 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by imf01.hostedemail.com (Postfix) with ESMTP id D7EB54000D for ; Fri, 28 Jul 2023 18:03:22 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=lwmNpruG; dmarc=none; spf=pass (imf01.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.208.172 as permitted sender) smtp.mailfrom=joel@joelfernandes.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690567403; 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=fPYow9WFU1jaMCih0ymMDtROAfjzkXiwxcD5gbF7K20=; b=BsCTV+KEVzcnMXCn1Tw2cDH5P6JK6SiGO5dgoXp9IleigL+dQQ9IRKDDTgdZmotQwX2RAz t+n7fNhLOdUcGQXw6XHMbYchq2hfcJB5LXNJpLRKBwbiHvluqJ5XJwToc9aSg6RudPlTdD ayuqx5QDFzPhPxM2xaKAgCZuXmgLJvc= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=lwmNpruG; dmarc=none; spf=pass (imf01.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.208.172 as permitted sender) smtp.mailfrom=joel@joelfernandes.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690567403; a=rsa-sha256; cv=none; b=BlUxGrvxH01uOxOv7r7avwI0kQkyBuX3KthCQ+Nb9ApgxPytnI6TTYNitQ684Q7phDrZwm ruCYx2sFrC0W7gvGSPTfHS7fuEVH8Vx3UZM9VBiHckqayDAt4uUcisjS2H1iD94KtB2qaJ GcrhNiXn6j/scX6gbR+7rhWehBKcbFs= Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2b9bee2d320so36512371fa.1 for ; Fri, 28 Jul 2023 11:03:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; t=1690567401; x=1691172201; 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=fPYow9WFU1jaMCih0ymMDtROAfjzkXiwxcD5gbF7K20=; b=lwmNpruGPdLgD4RfM/3SayVz6x7Uqn3ph5XE2Kzy/W4DtPEueu5lsgz/yLqva8d8Fx TFbFihJRFLqjE3m0mhSmOp5kv38Irv5iMFMYcpx9jCvT0spD/UTxWXk5dk2UcfUcBcHp jYn8BgPbHli/PhLM1m2FAgoE5D6jhtBkFBplE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690567401; x=1691172201; 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=fPYow9WFU1jaMCih0ymMDtROAfjzkXiwxcD5gbF7K20=; b=Q+DYD7mlqdaambN3awNe5V9fhR+tuDAkWFe6PxCKFYlo3rl5UDD0UbAHP/t64C0PYt GBHg00jkI/Gb0GfUrSKB/6Sny70yz+ytaQavr+9lDR1FlkUx5X9lJCTf6tB7VcsHJQ7J RWY/gdJBk0aTSI7OjxEbxgY6ltEpieG1397ThW983vzMIJDvrvnTKp1JuglMyZOv3exj BZ0wYEdiysl1s0srb8CZqjoP8TYTEJtKMX4I6lBuZQCKfRUYtXMLceihhSHYrnYNcW8M OUoHWGDFU1chcOIKFwfu5bfauj93vXT3QaIwY6MdI2SLHAPjnwxDsyTAFN4nyS0I4/Wa CuVw== X-Gm-Message-State: ABy/qLY0vcMS7WARraO+rNo6vlaQppdqivy7dXTpXwJJZGTl8yJsQkm4 e63qmUnNUgW+VsKdF/RtDUcjuS5UJ3OtC0emvtpdvQ== X-Google-Smtp-Source: APBJJlEhn+9zJD4Q4qVaoy//OUafQkPLi9nT38hQl8hoaQ0XlIG3FGGMz5osLwru18XRPkqVBrWwl9UZSWnt/1bNxKo= X-Received: by 2002:a05:651c:cd:b0:2b6:eb5a:d377 with SMTP id 13-20020a05651c00cd00b002b6eb5ad377mr2486386ljr.5.1690567400660; Fri, 28 Jul 2023 11:03:20 -0700 (PDT) MIME-Version: 1.0 References: <20230728124412.GA21303@willie-the-truck> <9fd99405-a3ff-4ab7-b6b7-e74849f1d334@rowland.harvard.edu> In-Reply-To: <9fd99405-a3ff-4ab7-b6b7-e74849f1d334@rowland.harvard.edu> From: Joel Fernandes Date: Fri, 28 Jul 2023 14:03:09 -0400 Message-ID: Subject: Re: [PATCH 0/2] fix vma->anon_vma check for per-VMA locking; fix anon_vma memory ordering To: Alan Stern Cc: Will Deacon , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: D7EB54000D X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: u8ttdzbin78guciyoa9a4dojbr765kuw X-HE-Tag: 1690567402-733427 X-HE-Meta: U2FsdGVkX1+z8/cM8A64UgMQReJJ0FrJ7OZKRN+OkZ8eDYbwxNVR/aO2FhDLEn0mSntJP6wkvnQ5sMwiVnNJJWRrPWqn2YYmzmYSNKCkIxqxFwiFTrUXsG25qOl++Op/jIFqi1s+41DBLQFujxaPgXxbnU69Yx6sXrgsiyDmj1WYNf/e4FdBRSipTMWalI9e0Yp46jl7T4IIfIMMoo5OqXAvFZ8R4MxIOuRbtB5iQx7vZusXQfGHEcFyndRUJReUclTiUO6EkNoMjOAqLEqbTyhxq11CTBOENJGAybSJzwB+YYiS8KHk8Orh4wN4yFQhh/bRcJK4WBG04Wghw/57YrEKYEJp7BGugvCwDsrJFcqZXBkSLHeT5mqvWy5X0kS2seaP2NPOq8NKmKNat0HwkEKg9t15WR2t88tafqGdfPdDC/3awjHNGJivnQwzcnGk7h/LfarELwM11Ah+axL2tSskG3oo7g7nNNYni7g+gTmx5+q2jIk8YqCoLyKd+Ym91MiaAVF5YNLMVNGnYtEC6VpfjAseu05i3Kx2QNlzo1HONLvO3PPXo73NBFRPoE5W5iCfllvnxP18yjeq4caHTihZtD6VpzMjx8bJz4wu19FKO9yP4n1naTpghkAtjl8/l8M0OIodQalaMHoFVfQ3+A0QuTSLfOB7f1WEa8lKRmV++N3b5KEFeCEdK9zQh+lZEi9isej2kRpEnfnHOXI27i+hMqUtr2Y7yQ2IQYJ93utPMAmoZd1mP2IdxNKRGmEH7z9kz6V3aRRd7f+tmMspYF6LPFP+louGakdQ3v99fYs+pYD7jI5LpRgn9v18jV9PnvGj/6EE6JT9r2xj1mNhZfsFGwPtgSlNs7SvFrnT8elYYDx/aByU4S8zWm85WNUFHV/atZqSJJQMXS2IbpXQ4JV4NzZaNh3V/MifsjdQAjUBKiP5WubjVLOOlFf1Yuvahd1RxsGVUkK+M5nZFAS EJdMC/Yw s5JTCztPTCADroWCaSd/tm8Tk5Cpq2EUB5JYrQoxIZ8xK3u7cKmAV+XTNmnBBxcx5Gv1YteygsJj1v4xez//sNsfQiOJLRxNTt2ysYkduzabzdwF6oA2PPdT+ZrN+K2EbU93rY+fLbX7auflPI2kQY+LDvU2fYRhZV/2qj7c8Sfr633PNcZMGiyT64UiPN1rv5rAZq2CfaqJjiOAtSAJBpDNYugJcZ2+UaaSRAf4att1PpR2tMKS4OYgoHSrdemqgef42J1Qh064m3r8mbBcPpyC+vcf3hGJkfk6o4G7a7Q+4J9+NBccmxqceBJc5GkfsCzAZ1wQj6QzbHsFzQh6uwKzuXScEl2iCUd8BByiZoKKreZBNmw7t9CAEkRyzMjRxg6CPd1LYUWPm3aXKkVTfQsiGR0fDGjUGn/xV 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 Fri, Jul 28, 2023 at 1:51=E2=80=AFPM Alan Stern wrote: > > On Fri, Jul 28, 2023 at 01:35:43PM -0400, Joel Fernandes wrote: > > On Fri, Jul 28, 2023 at 8:44=E2=80=AFAM Will Deacon w= rote: > > > > > > On Thu, Jul 27, 2023 at 12:34:44PM -0400, Joel Fernandes wrote: > > > > > On Jul 27, 2023, at 10:57 AM, Will Deacon wrote= : > > > > > =EF=BB=BFOn Thu, Jul 27, 2023 at 04:39:34PM +0200, Jann Horn wrot= e: > > > > >> if (READ_ONCE(vma->anon_vma) !=3D 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 =3D vma->anon_vma; > > > > >> > > > > >> // this needs to be address-dependency-ordered against one of > > > > >> // the loads from vma->anon_vma > > > > >> struct anon_vma *root =3D a->root; > > > > >> } > > > > >> > > > > >> > > > > >> 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() l= oad, > > > > >> 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 =3D *ptr; > > > > > bar =3D READ_ONCE(*ptr); > > > > > baz =3D *ptr; > > > > > > > > > > and you can get foo =3D=3D baz =3D=3D 0 but bar =3D=3D 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 unpleas= ant to > > > > > debug. > > > > > > > > Will, Unless I am missing something fundamental, this case is diffe= rent though. > > > > This case does not care about fewer reads. As long as the first rea= d is volatile, the subsequent loads (even plain) > > > > should work fine, no? > > > > I am not seeing how the compiler can screw that up, so please do en= lighten :). > > > > > > I guess the thing I'm worried about is if there is some previous read= of > > > 'vma->anon_vma' which didn't use READ_ONCE() and the compiler kept th= e > > > result around in a register. In that case, 'a' could be NULL, even if > > > the READ_ONCE(vma->anon_vma) returned non-NULL. > > > > If I can be a bit brave enough to say -- that appears to be a compiler > > bug to me. It seems that the compiler in such an instance violates the > > "Sequential Consistency Per Variable" rule? I mean if it can't even > > keep SCPV true for a same memory-location load (plain or not) for a > > sequence of code, how can it expect the hardware to. > > It's not a compiler bug. In this example, some other thread performs a > write that changes vma->anon_vma from NULL to non-NULL. This write > races with the plain reads, and compilers are not required to obey the > "Sequential Consistency Per Variable" rule (or indeed, any rule) when > there is a data race. So you're saying the following code behavior is OK? /* Say anon_vma can only ever transition from NULL to non-NULL values */ a =3D vma->anon_vma; // Reads NULL b =3D READ_ONCE(vma->anon_vma); // Reads non-NULL c =3D vma->anon_vma; // Reads NULL!!! if (b) { c->some_attribute++; // Oopsie } thanks, - Joel (On the road now, so my replies will be slightly delayed for few hours)