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 1B31DC0015E for ; Thu, 27 Jul 2023 17:17:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 943FA6B0071; Thu, 27 Jul 2023 13:17:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8F4656B0074; Thu, 27 Jul 2023 13:17:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 795A16B0075; Thu, 27 Jul 2023 13:17:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 688906B0071 for ; Thu, 27 Jul 2023 13:17:06 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D92621CA0DB for ; Thu, 27 Jul 2023 17:17:05 +0000 (UTC) X-FDA: 81058047210.01.5AD6F7E Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by imf30.hostedemail.com (Postfix) with ESMTP id 88E8380024 for ; Thu, 27 Jul 2023 17:17:03 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=google header.b=hWz9Rq7F; spf=pass (imf30.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.167.49 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=pass (policy=none) header.from=linuxfoundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690478223; 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=GvxANQuFKNvKufAyX6mUm0ShC2KKTHC+l1CTdx+XIO8=; b=0GPtQi8KQ4Wsj8EPAqj1PxWYsIZDaPIQfThUh7GVq33/VP2KHIbTPHUYM1g4Cbr3IogzCc akat5Gj6k3B7Q5XkWDOYOGIWyP5ZrxpallpKN6Tc5rg634Gu56qOwJI0tWZQbh3CoFEUU0 z7mQRk1UBoBWDxF/bmi4TILP/2lVHLI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690478223; a=rsa-sha256; cv=none; b=QHWZAgzC7wfvQFqT+b0BfoBEN/JNYfWdvp5VUcX++ooTn6gWZFMe1fLjap9bIVpo0mPjhg uPUeiAFWDL8zJBbohzQaJERt0IDSjebzF4vdzMrwW/xVrhKOCWvd/GmRRFZqT6FY07rOfA VqkG7t/ZsCqHJFAllPTlvqori4q5h5c= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=google header.b=hWz9Rq7F; spf=pass (imf30.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.167.49 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=pass (policy=none) header.from=linuxfoundation.org Received: by mail-lf1-f49.google.com with SMTP id 2adb3069b0e04-4fba86f069bso2091232e87.3 for ; Thu, 27 Jul 2023 10:17:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxfoundation.org; s=google; t=1690478221; x=1691083021; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=GvxANQuFKNvKufAyX6mUm0ShC2KKTHC+l1CTdx+XIO8=; b=hWz9Rq7F5NJgaKqyoHuxhtDW1EXvz/CW7aZ4foQHh7ytCQvvfXASDs6/INEK/u7av/ GXRNJbgB9P+pOHD0ByPmzB70WjE0Tin+2eW/p8E1lG/YD5RMbtYmgqdlWAAzMBur72hi FJaY+zZjP0rrQBypNpOD8eUrgJ+vrgFgzkjg0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478221; x=1691083021; 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=GvxANQuFKNvKufAyX6mUm0ShC2KKTHC+l1CTdx+XIO8=; b=ayDkoZgXEnYog9NsavbeFc2L0D2SG+acB5T3/w0m6d2yZNdyxOd7QRwGV5/AR6sg4d UN9SvQ7VXzO2YLzmF9FTZDb7qQRV2csW/jmCbg1yyVXrVOCyJkAkWywAUqYcPLExxmPd pWYHP/OZUO4jmRs30yhQhGN/06Y2g0aRLCzvlONiVSYUpikg7oc6ZdFxEbt6hBXZSF2X K87oU+jGZpaydJzD0dDyvqkjRuK0eTbvVlSs34nh16BGAZ8NJBRPqXBhiD+1d9q4ofzK b3bdbfDI6tuyqW31CidWW0tYGI4UMg5GPwPv4SDbbSR/GKZPiZeon78mm3rOmW/MXKFb KJtw== X-Gm-Message-State: ABy/qLZ6ftIW6JAKI1Wht/qmWLVUqdFe6shouIuZ+63zRE+jJE/4ZGX+ fYV3Rav8qQJEP/kEg/kLlqOb5/t4SgqRWZr7B0kRvE4o X-Google-Smtp-Source: APBJJlGRDult4azT/0ZkwWSAGDP/iaLWaC4FCoAO4zcy7PpAYPx3Hg5hVIQBJX+FGARMBJdXId80cw== X-Received: by 2002:a05:6512:b9f:b0:4fb:9168:1fce with SMTP id b31-20020a0565120b9f00b004fb91681fcemr2826645lfv.59.1690478221175; Thu, 27 Jul 2023 10:17:01 -0700 (PDT) Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com. [209.85.167.41]) by smtp.gmail.com with ESMTPSA id eq9-20020a056512488900b004fb7be56ff0sm396232lfb.292.2023.07.27.10.17.00 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jul 2023 10:17:00 -0700 (PDT) Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-4fe0d5f719dso2088659e87.2 for ; Thu, 27 Jul 2023 10:17:00 -0700 (PDT) X-Received: by 2002:a17:907:2c6a:b0:982:870f:9e8f with SMTP id ib10-20020a1709072c6a00b00982870f9e8fmr2474183ejc.62.1690477906607; Thu, 27 Jul 2023 10:11:46 -0700 (PDT) MIME-Version: 1.0 References: <20230726214103.3261108-1-jannh@google.com> <31df93bd-4862-432c-8135-5595ffd2bd43@paulmck-laptop> <20230727145747.GB19940@willie-the-truck> <13dc448b-712e-41ce-b74b-b95a55f3e740@rowland.harvard.edu> In-Reply-To: <13dc448b-712e-41ce-b74b-b95a55f3e740@rowland.harvard.edu> From: Linus Torvalds Date: Thu, 27 Jul 2023 10:11:29 -0700 X-Gmail-Original-Message-ID: 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 , 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 Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 88E8380024 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: 76yrbb9ixgkq1zc6g9nkrs67bouk9eur X-HE-Tag: 1690478223-265150 X-HE-Meta: U2FsdGVkX18Qm2y16ec3lstCWYR4uUkVhhtpwD1DhtLqSBRG64/o5b7h2Pudsq0Oha4kFP12nexqpFTaIEvGmz10d5m8eTKY1TfzYaanSUUQv9k7quJq5P1rnICdaqfwSit6PlR4DY9uMgDtISP8Avmk0xC5M8Qiv1Gspb+LMfjd3Q1xUywCHkuQTjFum1g0tIDxxYtMvq7QknNX+TS0wxLmbYAYMskaRinoov2u8AtZO9ZTp/Lpezm+h6ZsGXBXF7t8hXg7jbadBodOliRnMfQzbpGRWAIf6Ya+eN6aHFEIIm0XFNIBDpYCfAPfB5XP4Hc5+Puxm3VWPGq4rhdZ7CJGmiN099hENS8RRr8DyBzW/6F7OxvyXjjA6pHjVywPcphVAUILI9t81MBQU9iapQyG7YyW8foPkIQI9mnfX3XsNadD9HAN/VXaZm3CRD/gUELLishAQ0qBn9hVxtctsqzTYgB+OzhJ+VlLra7aencUwjE8p5ZxAeN9oPe2+q3m7KJrzUnDtGMkdV9kOuWxo0yVWyyBTqfCTOBTIKKtk5PgKFfXeUjRvjeobQf3D2CJRi3vXK9MpdaCdjRBecCJry9NXAyuy32sc6+0m9UL8uYiivbp6p3/83pgGDzPGQD9fV9XS7ayZb/vZFTmYxgANpaAjox50nCpAfLZca65TJ5MqRlJFrlQdkQK+z8lQX8oLhRicmxfB6gLpz2uC9F8rgfKmp1jaXgZ3SnxdF5VAC/RhwBNrfyiZ/x5sRSmkGCuARJfwXA2g9h9dFAha5kqSZ/TeDWLD/n4ffBiB5C/u7yJr2DHVzvKDJUXtXvwKZ+Yj+R6IYzD3Zx3kq85ocyrubCGgtSe/di8nmMDTd1bYiAvB/eaoJaf/A8uyyZmzFdDHwo3LnNMMyIchyxtQSURGPrvKbUAbN4raUzs3n3/Yg7nxJQX3g+HdX/Vy6FdAxj3qrSB0LirLoVxDSnb3St 0scHbxL5 h6mpFwvkRk74oz37z0nclFl4pK0CnlzgIqiN2v1C2ebaqiutwLgy+Cegw2y3a6VGLKLCTaPZ+AvbhhjZFy8wb0asIY5OumCQw2ek/Gd/tdl7Pvqar6mjFOGHrITMDp42hfgj0pL62udFWSE/Y0Gu37IuPg/iWTUCx156M1ngFEQQY4iVKHyi8chfVllnKdNbprcH7kxGEWJ3zYatxxYz7UnxHQp1mAFY0JVF8wxlTzcriUXIYsArIjN1BLUqkqmb69vJTyIc/RxTX14yGNhVmLDTIU/HvSFVmn33IoeXKg73uTdvjn0cqDhu9coihUE93cr6Nnbuf8Plev7GkHeiSvWqKHuVaqjoQ/Je+O2zqvqQe2BgbMoGj9eVswfH5v4Opm11V2doX7Gm8oq3i1qkW0vKD9pIQy9guI4JibmePljgckN1GS214RwLDfIl26vTeUN5z 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, 27 Jul 2023 at 08:44, Alan Stern wrote: > > This reads a little oddly, perhaps because it's a fragment from a larger > piece of code. Yes. As Jann already said, this is basically a preparatory step in a much longer sequence, and the code simply wants to make sure that any later code (possibly quite a bit later) will not see a NULL value. I do believe it happens to be safe to use READ_ONCE() for a number of reasons, but I will argue that we should *never* use a bare READ_ONCE if there is any actual real question about what any memory ordering might be. And yes, the RCU code obviously does use READ_ONCE(), so that's why I say "bare" - the RCU code wraps it in helper accessors with strict semantics. The reason I think it would be techncially *safe* to do here is that (a) code like this: if (READ_ONCE(..)) may end up having the conditional speculated by the CPU, and have the actual read done without any ordering by the CPU, but we do have *one* guarantee: the memory load instruction will be before the conditional branch (or whatever) in the instruction stream. So the code may be *executed* out of order, but we know the memory load can not be moved after the conditional (whatever form that conditional then takes) by a compiler. (We do have various barriers like "rcu_read_unlock()" that guarantees that the READ_ONCE() couldn't be moved lmuch later even in the absence of the conditional, but we can ignore that). (b) the later use of the anon_vma (that depends on the value being stable) is *so* much later, and behind things that the compiler sees as barriers (again, that rcu_read_unlock() acts at a minimum as a compiler barrier) that any subsequent use would not have its load moved down to before the READ_ONCE() in the instruction stream. Again, this is purely a "location in the instruction stream" ordering argument, not a "execution order" ordering argument. And finally (c) I do think that all our architectures have the rules that when you read from the *same* memory location from the same CPU, the accesses are ordered. Now, I didn't actually find (c) spelled out anywhere, but even alpha - the worst memory ordering ever devised - had that particular rule (the alpha architecture manual calls it "Location Access Order"). Now, with that said, I did argue to Jann that we should use smp_store_release and smp_load_acquire pairs here, because honestly, the above (a)-(c) argument is too damn subtle for me, and I don't think this particular use requires it. With smp_load_acquire(), you don't need any of the (a)-(c) rules. The rule is "the load is done before any subsequent memory operations". End of story. So while I do think READ_ONCE() is sufficient here, I actually think that once you start going down that path you can argue that READ_ONCE() is actually entirely unnecessary, because we also have other ordering rules that mean that the compiler can't really do anythinig else even *without* the READ_ONCE(). End result: I can trivially extend the (a)-(c) to say "even READ_ONCE() isn't strictly necessary here, because even any access tearing - which won't happen anyway - wouldn't actually change the result. So if we want to make it *obvious* that it's safe, we should use smp_load_acquire(). And if we do find that there are situations where we care so much about the (generally fairly cheap) possible additional synchronization, and we really want to use READ_ONCE() rather than smp_load_acquire(), I'd rather try to wrap a READ_ONCE in some kind of better abstraction (maybe make the conditional part of the operation, and make it clear that you are doing a "one-time test which depends on the same-location rule". Do we even have the same-location rule in the LKMM? Linus