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 7DA63C43334 for ; Tue, 5 Jul 2022 00:06:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 00BAE6B0071; Mon, 4 Jul 2022 20:06:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EFD346B0073; Mon, 4 Jul 2022 20:06:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9C406B0074; Mon, 4 Jul 2022 20:06:46 -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 C47256B0071 for ; Mon, 4 Jul 2022 20:06:46 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 97DC760573 for ; Tue, 5 Jul 2022 00:06:46 +0000 (UTC) X-FDA: 79651105212.11.EF3BC58 Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf14.hostedemail.com (Postfix) with ESMTP id 4ADAE10003B for ; Tue, 5 Jul 2022 00:06:46 +0000 (UTC) Received: by mail-lj1-f180.google.com with SMTP id s14so12647094ljs.3 for ; Mon, 04 Jul 2022 17:06:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Qmr+F6QyjrWZ2ZNhmVLeAUEKN7hzojbkjvHu88aEdVo=; b=DIX8InJ6+wbF8trPlzF4+I2SecOagwTWWWojE8MrUcdpyHa/pRG6jbX6znc3kN0wwY meZsX/cYHAia9UGV3E6pOMc+RLJkbdCCC5GHWJIC5ovXbkt+u1S6rePiMZINTfJyYWAf La2KI8ZnqJltrRgKiua/JxCOBBzGPIGfHCgC4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Qmr+F6QyjrWZ2ZNhmVLeAUEKN7hzojbkjvHu88aEdVo=; b=7V0800lxraQQsV/PLHhq3ifjEG8BhNbsrby9nfDSbkOlV8FGFFIbqfN6vPROgnN18i dgFecoUGjrrUnyx4tChurXmgrgnQQ0PczhFUyJj3VJwGN72fntePL/kxD40qMo8f25ZY yIJv97lBw1hFXHuGylUiHZwPObBFSOhtUEYBeb71L32daQfGmXIOtz73hnni2BjMwp9J 7UF3CwSVMZBb/0hS+LP+iAQgsQMb1ACgImLmCL4eE7aOWcdfB6wL1DLZj7maGt0Ex1MV VJygS2omRv94l1d/V2htQ5muyVPSnnqPBh4jOR3q69oeTGqiqVKyF6gKBb2A8EPpmotA y7LQ== X-Gm-Message-State: AJIora991sC0TXba/e/dPABSL+vScG96FEDjvAZyOd4i43hVer8C6tk1 QUCvQW/S5ZaKopxyOR2BAk2R47Zy8vPgWQU7xzk= X-Google-Smtp-Source: AGRyM1tQULE5Um8ixB48fCPVdHBR6//KDn0hYCnwLGjwI15+G7vkHHGB4zlaRveMAu66JDnyaNbhGQ== X-Received: by 2002:a2e:bf1c:0:b0:25d:2bcd:4e44 with SMTP id c28-20020a2ebf1c000000b0025d2bcd4e44mr1800787ljr.420.1656979604305; Mon, 04 Jul 2022 17:06:44 -0700 (PDT) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com. [209.85.167.52]) by smtp.gmail.com with ESMTPSA id bp23-20020a056512159700b00478f1e04655sm5392154lfb.14.2022.07.04.17.06.43 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Jul 2022 17:06:43 -0700 (PDT) Received: by mail-lf1-f52.google.com with SMTP id t19so17289326lfl.5 for ; Mon, 04 Jul 2022 17:06:43 -0700 (PDT) X-Received: by 2002:a5d:64e7:0:b0:21b:ad72:5401 with SMTP id g7-20020a5d64e7000000b0021bad725401mr27990729wri.442.1656979593298; Mon, 04 Jul 2022 17:06:33 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Linus Torvalds Date: Mon, 4 Jul 2022 17:06:17 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/7] __follow_mount_rcu(): verify that mount_lock remains unchanged To: Al Viro Cc: Alexander Potapenko , Alexei Starovoitov , Andrew Morton , Andrey Konovalov , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Christoph Hellwig , Christoph Lameter , David Rientjes , Dmitry Vyukov , Eric Dumazet , Greg Kroah-Hartman , Herbert Xu , Ilya Leoshkevich , Ingo Molnar , Jens Axboe , Joonsoo Kim , Kees Cook , Marco Elver , Mark Rutland , Matthew Wilcox , "Michael S. Tsirkin" , Pekka Enberg , Peter Zijlstra , Petr Mladek , Steven Rostedt , Thomas Gleixner , Vasily Gorbik , Vegard Nossum , Vlastimil Babka , kasan-dev , Linux-MM , linux-arch , Linux Kernel Mailing List , Evgenii Stepanov , Nathan Chancellor , Nick Desaulniers , Segher Boessenkool , Vitaly Buka , linux-toolchains Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=DIX8InJ6; dmarc=none; spf=pass (imf14.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.180 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1656979606; 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=Qmr+F6QyjrWZ2ZNhmVLeAUEKN7hzojbkjvHu88aEdVo=; b=p/QFqMo53DDwjpo18mbhCbqMh0t4uTRBTJk8fHyquTjmM1HMGa43cPNevGAUnmP4oLXNe+ EQzjqiPIjWgYQrOqcKyoQxCz3oOMjvnq71cRALsaQ1xFy5RergFCtDl2cD4JCn/i7BU9LP Soj6O4+tVnMbY7ZE2UshFYEeq8VQL20= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1656979606; a=rsa-sha256; cv=none; b=ooPzCyHJk2NCPm2Vq0u603y0aX/yMquMBn5O3JoTexF4xlf364uhJQkKbv0xQc3HWjBwG9 f0NMJX9sDpVxb2/JSYDbbqetlFe9qmSn6zdkHU7M2UPDPScYyXR3lHWmz+veGAVXyjk27q cHhxbHodB7SBQ0s9syQT0qTWT7tlRlQ= X-Stat-Signature: kr1ccta4p7i5i1i6u6gcnmir3bpx8iuc X-Rspamd-Queue-Id: 4ADAE10003B Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=DIX8InJ6; dmarc=none; spf=pass (imf14.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.180 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org X-Rspamd-Server: rspam03 X-Rspam-User: X-HE-Tag: 1656979606-907155 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 Mon, Jul 4, 2022 at 4:19 PM Al Viro wrote: > > - unsigned seq, m_seq, r_seq; > + unsigned seq, next_seq, m_seq, r_seq; So the main thing I react to here is how "next_seq" is in the "struct nameidata", but then it always goes together with a "struct dentry" that you end up having to pass separately (and that is *not* in that "struct nameidata"). Now, saving the associated dentry (as "next_dentry") in the nd would solve that, but ends up benign ugly since everything then wants to look at the dentry anyway, so while it would solve the inconsistency, it would be ugly. I wonder if the solution might not be to create a new structure like struct rcu_dentry { struct dentry *dentry; unsigned seq; }; and in fact then we could make __d_lookup_rcu() return one of these things (we already rely on that "returning a two-word structure is efficient" elsewhere). That would then make that "this dentry goes with this sequence number" be a very clear thing, and I actually thjink that it would make __d_lookup_rcu() have a cleaner calling convention too, ie we'd go from dentry = __d_lookup_rcu(parent, &nd->last, &nd->next_seq); rto dseq = __d_lookup_rcu(parent, &nd->last); and it would even improve code generation because it now returns the dentry and the sequence number in registers, instead of returning one in a register and one in memory. I did *not* look at how it would change some of the other places, but I do like the notion of "keep the dentry and the sequence number that goes with it together". That "keep dentry as a local, keep the sequence number that goes with it as a field in the 'nd'" really does seem an odd thing. So I'm throwing the above out as a "maybe we could do this instead..". Not a huge deal. That oddity or not, I think the patch series is an improvement. I do have a minor gripe with this too: > + nd->seq = nd->next_seq = 0; I'm not convinced "0" is a good value. It's not supposed to match anything, but it *could* match a valid sequence number. Wouldn't it be better to pick something that is explicitly invalid and has the low bit set (ie 1 or -1). We don't seem to have a SEQ_INVAL or anything like that, but it does seem that if the intent is to make it clear it's not a real sequence number any more at that point, then 0 isn't great. But again, this is more of a stylistic detail thing than a real complaint. Linus