From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751537AbdDCAat (ORCPT <rfc822;w@1wt.eu>);
        Sun, 2 Apr 2017 20:30:49 -0400
Received: from zeniv.linux.org.uk ([195.92.253.2]:48008 "EHLO
        ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750992AbdDCAar (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sun, 2 Apr 2017 20:30:47 -0400
Date: Mon, 3 Apr 2017 01:30:45 +0100
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [git pull] vfs fixes
Message-ID: <20170403003045.GK29622@ZenIV.linux.org.uk>
References: <20170402170148.GI29622@ZenIV.linux.org.uk>
 <CA+55aFymH_GwFfh=PhHOmiT5qzr6dwdt3GfmUFJrccLgdhPjTg@mail.gmail.com>
 <CA+55aFwxjEjV++j-WRXomZrjBR6Dhx9nM-SOwM7pD=zCnMspKQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+55aFwxjEjV++j-WRXomZrjBR6Dhx9nM-SOwM7pD=zCnMspKQ@mail.gmail.com>
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sun, Apr 02, 2017 at 05:10:08PM -0700, Linus Torvalds wrote:
> On Sun, Apr 2, 2017 at 4:59 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Sun, Apr 2, 2017 at 10:01 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >>         statx followup fixes, fix for a nasty corner case in path_init()
> >> leaving path.dentry in RCU mode pointing to a dentry without DCACHE_RCUACCESS
> >> and a fix for stack-smashing on alpha.
> >
> > These were apparently committed minutes before sending me the pull request.
> >
> > Why? What kind of testing did this all get?
> 
> Also, that RCU fix really stinks. It makes no sense. Any valid base
> for actual pathname lookup will already have the bit set, so the only
> issue is when people play games and use a non-path file descriptor
> without a pathname. Right?
> 
> And that case shouldn't actually use RCU lookup AT ALL!
> 
> By definition such a case will  just be immediately unlazied anyway in
> complete_walk(), so doing an RCU lookup on an empty path only adds
> overhead, and there is no actual point in doing an RCU walk on an
> empty pathname.
> 
> So I get the feeling that the proper fix would be just something like
> 
>       /* Don't RCU-lookup empty pathnames */
>       if ((flags & LOOKUP_RCU) && !*s)
>             flags &= ~LOOKUP_RCU;
> 
> at the very top of path_init(), which
> 
>  (a) makes the code more efficiant, since we don't do those
> unnecessary games with sequence numbers and RCU locking only to un-RCU
> it immedately
> 
> and
> 
>  (b) obviates the need for those DCACHE_RCUACCESS games entirely,
> since anything that can actually be used as a base for pathname lookup
> will already have that bit set, afaik.

Currently true and almost certainly will remain so.  Point taken, what you
are suggesting is better.  Actually, the invariant to watch for is
"no d_can_lookup() withtout DCACHE_RCUACCESS" and that we can trivially
enforce by one-liner change in d_flags_for_inode() -
s/DCACHE_DIRECTORY_TYPE/& | DCACHE_RCUACCESS/

OK...