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 49726C00528 for ; Thu, 27 Jul 2023 18:26:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A08066B0075; Thu, 27 Jul 2023 14:26:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9B86B6B0078; Thu, 27 Jul 2023 14:26:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8801A6B007B; Thu, 27 Jul 2023 14:26:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 784A96B0075 for ; Thu, 27 Jul 2023 14:26:04 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3A21A14105F for ; Thu, 27 Jul 2023 18:26:04 +0000 (UTC) X-FDA: 81058221048.27.A48E46C Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf10.hostedemail.com (Postfix) with ESMTP id B10AAC0010 for ; Thu, 27 Jul 2023 18:26:00 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=google header.b=Fd+R3Y7b; spf=pass (imf10.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.53 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=1690482360; 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=bzem6p2nNFcrPREy8LTws99E2B4HTj3oQRbG1EigUS4=; b=xyMj2gslBOA4s0t4F2q4BSQgRCh9EBtkSah0tOaSM54gMccPmIfsuNE+W9qDY5+YKbdu4A KbTsqL+Jx9NfnF5D32NGOYaOS+LR4VSm+LF/09cKn0zWCHACd5QUPv23SS7VvUU/wl2e59 sCXuWYW7KmOOAqC/7SbJ05h9zP+CPnE= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=linuxfoundation.org header.s=google header.b=Fd+R3Y7b; spf=pass (imf10.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.53 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=pass (policy=none) header.from=linuxfoundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690482360; a=rsa-sha256; cv=none; b=EtxtdqkyEL2Nlj3TwOlQHWiMEnRr092Ru5Kx5wpMoP62bfmyOWa8/MPWbUGCvhgE+T2UOE 6ZQb+cdxe0SDo4k3pQ/yLne9KItAgN65IJ0filtDRxiMUbdUEJ1+doy9BvOqt+OAhNOP7n O8e+9dI+KegmZtXKdqb6KwMyu9AVAwA= Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-99b9161b94aso168372666b.1 for ; Thu, 27 Jul 2023 11:26:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxfoundation.org; s=google; t=1690482359; x=1691087159; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=bzem6p2nNFcrPREy8LTws99E2B4HTj3oQRbG1EigUS4=; b=Fd+R3Y7b6VfbQWShS8Ce+6GAP4sb6H2I5ahSH3bJoHdta2bThQjXqAc/cVIWVorx3F WFE1TY6zO2s3vIBMiIHXGJ4A/RVlSEpegqCmdRJ5z/QCOPrhrDLF+SikgQoQ/5+DUegl FTA2bm3eSEvxRfqkpJWbum9EO5AagLB/gZBWg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690482359; x=1691087159; 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=bzem6p2nNFcrPREy8LTws99E2B4HTj3oQRbG1EigUS4=; b=ilm5Dzrn47esEwqW78XJ1kGEJW/2SVo2DAHhxVkDt8EVfk1b9U0rHotka1pPnn0aKN sUGR4kP0kYi4qvyhMLEHZmXADT6qSowBRZBCFlr4bBS3ok59oeMSQDsaznI3jSnGS7eH T/i0W+D6gbMHW9c2tYvtO1BsycvJ6jh59ZgfXBXbBBwpohwJyZOzKndy0WdxaCfvyqNN nQ48L4sJAZDVl/vk1JgQYJbqkvxaFgWvodgKsJb5nhXB69ylyZGr0Rov1azutL9wKsaG ZqSSAO6ApYPZHxaaa0SdTcJfnr+8s1Q60vPkpu6ar+d4pSyGJ13OU/GcDfGY5Fzcxwku C9Ug== X-Gm-Message-State: ABy/qLa2UrRQF6HudADMqfQYxs1p2uh6qjUFaxfajvkPzXj/CQkkQDfb W09nPnwtGhUxCpXkx7ekpds05jqBHvKtbdc9OpyRFEyC X-Google-Smtp-Source: APBJJlEhvSlSZyCedyazrjXFnQ7+/AO+MLlVRxhAcXUKdGb1fsof7+ulP1iYIGpxKrbpmk7l/VuK4A== X-Received: by 2002:a17:906:31d1:b0:993:db29:d27d with SMTP id f17-20020a17090631d100b00993db29d27dmr10760ejf.34.1690482359017; Thu, 27 Jul 2023 11:25:59 -0700 (PDT) Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com. [209.85.218.49]) by smtp.gmail.com with ESMTPSA id b14-20020a1709062b4e00b0099290e2c163sm1056361ejg.204.2023.07.27.11.25.57 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jul 2023 11:25:57 -0700 (PDT) Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-99357737980so167100566b.2 for ; Thu, 27 Jul 2023 11:25:57 -0700 (PDT) X-Received: by 2002:aa7:cfc5:0:b0:521:7a5e:ab1e with SMTP id r5-20020aa7cfc5000000b005217a5eab1emr2415059edy.21.1690482357203; Thu, 27 Jul 2023 11:25:57 -0700 (PDT) MIME-Version: 1.0 References: <20230726214103.3261108-1-jannh@google.com> <20230726214103.3261108-4-jannh@google.com> In-Reply-To: From: Linus Torvalds Date: Thu, 27 Jul 2023 11:25:40 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 2/2] mm: Fix anon_vma memory ordering To: Jann Horn Cc: Andrew Morton , Peter Zijlstra , Suren Baghdasaryan , Matthew Wilcox , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Alan Stern , Andrea Parri , Will Deacon , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , Joel Fernandes Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: B10AAC0010 X-Rspam-User: X-Stat-Signature: 1bdxkqhrrernx9m4xq1hnw4mcyn76jco X-Rspamd-Server: rspam01 X-HE-Tag: 1690482360-557334 X-HE-Meta: U2FsdGVkX1+93V7gHNviaONSG9VoScGBtyyLj67yu+3PQzJ8DV/yUwfy5HdF+DUJq4MhWnKUEBqxpXp0qdAnhgrSj/B51NPeuwgjzAT9URLV/FNqsDYQM7abiZyN2ekFNVDlG5303kQ2zX/qfPOR2nk5q1bJCklffSPuVsXkb5AbNjy9MbPJcXWM/q63LRnU5+UWwayFOA/xlF6S945WRQaZfbFtzzBinjUhJKJtH86xvTrX1K/cQ7pjMHte3WGqeDqznJjd1iVq8cqLeCXMdDwSCRi6IIwjks7W09OcS93jUKQwVojZOjT3XYQH6oKthZAJe5xPbJ9DA/ytYclNmfN4sKC0lswpDs5NS6y2B/8GRGOpLoH7K7gcHo4LwN1cE33oEjWFUpTWjqlCxTpu5XjgvNNbm5sVT0e4QbMZUiF1plnZXk8ApB8DCPzwmZzEbJ9746UfSI6uqtGAjb9iQSv87s/B0tGq4TcjorLcl8gvjn8xDExfN2A4jPgow4nvR/BiNwj0GVjnpRXF5EmbFVUAlaNFxzjbgWOUHwOfw460GglT6b3U3K8LKE22QnbEuZNO5mVewBt3aJgOLz8kWWuKp2UsUk2Hyu8r3QIHCwccW3GLNy4LevM0vWvhXvPR1Ak+7HHiz/K/RV1/tFXXrdA4zeLgL13M7YBXuKfk4FTKT+/r+Bpi6UFr50Mto3ElSMQ5LqqnuiHsMVnIAhI3uTTtBnnWrz6L3ckLWRFOtzt4u5G0cZkj0MOltUehzuqMt9shrK2BzRn3lcECKynZboSOc8ZELNBrOJHgn19j//6MXs6/XLsS8EoiqQN7MdEIFHQMAgdOReaTWARbhnapx0NCIJRGzzai7sSLOih1LQDPLAyBI96aj7cSm3+sncomrVIPoh3v25UkunVVVUPNBbSsMTJb+Gc4zDItPd1O7gk13dxB9Vhmjq7gIZAJM6kpGPqx7N6DA8PFEmbJgQf x6NCJW7B kgicN/A59z367wRr7Io+VQA422+X/iqWcGOVhW9YEcVaz5lC5sjg2rnl6YIOgvThezzN0GgJhdfBT3w6GCYcO9Xh+hLM+D2TEJOl8+Ssip8IwJ7atZijuZjzJV/MmieL+RbB8ilJXx18+kCpuppgr9PYz9iP561NFflIemTLg1PR799tfrGf5Wf1mQBR3xp1EMdhlekq68vahet+ze/YM+ewKmm7COAer15ivIOGYKcS+ZTQF4qx3YgsYIWF7u7szrvZueOVj9NZTziWBb2RuGmTL4U0Z8/4uNGImk/E1UlbFyXLm1jnqzVXkxOmxtBr+JzWCujYhH8v5SSo4i5AgSUGwji6uEM3UUwBUPIBnnHzHr4WvMfquOtPMQshJgVUV31BxpScPu6tFZ6tqCAuSary2ywOda8C9OABy5R6TqpWIxqw/85bj0kwuZXr2cHK/9hbrfHjXhHVeXuijt4plLLT4zA== 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 Wed, 26 Jul 2023 at 14:51, Jann Horn wrote: > > Of course I only realize directly after sending this patch that this > comment only holds... [..] > ... if we move the smp_store_release() down by one line here. So I've applied PATCH 1/2 as obvious, but am holding off on this one. Partly because of the other discussion about memory ordering, but also due to how the now sometimes much more complex-looking conditionals could be made a bit visually simpler. For example the patch does - if (!vma || !(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma) + /* see anon_vma_prepare() */ + if (!vma || !(vma->vm_flags & VM_MERGEABLE) || + !smp_load_acquire(&vma->anon_vma)) return NULL; which is a understandably mindless "just add the smp_load_acquire()", but it is also just unnecessarily hard to read. And the comment placement is a bit misleading too, since it really only refers to one part of the expression. And that's very obvious if you know what it's about, but the whole point of that comment would be that you don't necessarily know why the code is doing what it is doing. IOW, that would be much more legible just split up as if (!vma || !(vma->vm_flags & VM_MERGEABLE)) return NULL; /* see anon_vma_prepare() */ if (!smp_load_acquire(&vma->anon_vma)) return NULL; I feel. Also, I'm now wondering about the ordering of that > smp_store_release(&vma->anon_vma, anon_vma); > anon_vma_chain_link(vma, avc, anon_vma); sequence. Maybe we *do* want the anon_vma pointer to be set first, so that once it's on that anon_vma chain (and is visible in the anon_vma interval tree) it always has a proper anon_vma pointer. *OR* maybe the rule needs to be that any other concurrent user that sees 'anon_vma' as being valid also can rely on the links being set up? I *suspect* the ordering doesn't actually matter, but it just makes me wonder more about this area. Linus