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@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Sun, Apr 2, 2017 at 10:01 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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...
