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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EFAD2C433EF for ; Tue, 5 Jul 2022 00:06:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230312AbiGEAGs (ORCPT ); Mon, 4 Jul 2022 20:06:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45794 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230240AbiGEAGr (ORCPT ); Mon, 4 Jul 2022 20:06:47 -0400 Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7439DB48D for ; Mon, 4 Jul 2022 17:06:46 -0700 (PDT) Received: by mail-lf1-x135.google.com with SMTP id z25so360622lfr.2 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=cWbmz2YmfxV4jPRqRtsdrNbWuKnF0Lf9WboK2wVN5uiHxnlZiqfbiQNbvWQgQB6uUq 1154/wBKFrL6EKiNPOZRBcdS+ipsPMI7xVGcz495ucflqnYd8J6XYPz9O2aV1Kgwy7jJ DDDGeCFrWDefOqTSCcpd1BW5tWOjZ93WP7cpUCO9yEMgdBjBx3w0lz70IlVcPAJTbV0t pd/6A4Wpy1a3hNEyPBQSDsI0eDASXUESCLP5p+OBL/w/sLBg5kNs/3ihJkHxGq3NVOYq GucvJ7AsK1fEqIDZpI0mo+t5D0KD3XimiyYtLrgZDW75bLGXabMzPE5Qbq8+K3A7BDoD Ljbw== X-Gm-Message-State: AJIora9G2zD4oTqUT/GoIWGC+963h+/HFQbmjgaT1x3zSlTyCi36bsjJ xCdJn8qLUGciMsfiTW2x8v4mT2UsSFKPDZy586I= X-Google-Smtp-Source: AGRyM1uPK5chBosritKTehJSLZuGhm0gLqyyAjNDROPGK/tVTQbwWdJX2WX2L6IVk+knDTFeaW01Bw== X-Received: by 2002:a05:6512:1151:b0:481:1675:f343 with SMTP id m17-20020a056512115100b004811675f343mr20032378lfg.280.1656979604870; Mon, 04 Jul 2022 17:06:44 -0700 (PDT) Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com. [209.85.167.54]) by smtp.gmail.com with ESMTPSA id a23-20020a056512201700b0048162b71f9fsm1925335lfb.301.2022.07.04.17.06.44 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Jul 2022 17:06:44 -0700 (PDT) Received: by mail-lf1-f54.google.com with SMTP id z21so17872898lfb.12 for ; Mon, 04 Jul 2022 17:06:44 -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" Precedence: bulk List-ID: X-Mailing-List: linux-toolchains@vger.kernel.org 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