On Thu, May 10, 2018 at 07:20:58PM +0100, Al Viro wrote: > [in the spirit of "don't put 'em in without posting for review; the > this is present in vfs.git#for-linus, if you prefer to look in git. > > Background: a bunch of nfsd races fixes from back in 2008 had > problems with lockdep enabled; in 2012 that got "fixed", unfortunately > reopening a narrow race window. The patch below does *NOT* fix > all filesystems, but it does fix most of the exported local ones > and it is easy to backport, so it makes for a sane starting point. Do you have a pointer to the commits/test case for this? XFS has a fairly significant separation between unlock_new_inode() and dentry cache instantiation for some paths.... > If anyone has objections, this is your chance to yell. > ] > > For anything NFS-exported we do _not_ want to unlock new inode > before it has grown an alias; original set of fixes got the > ordering right, but missed the nasty complication in case of > lockdep being enabled - unlock_new_inode() does > lockdep_annotate_inode_mutex_key(inode) > which can only be done before anyone gets a chance to touch > ->i_mutex. Unfortunately, flipping the order and doing > unlock_new_inode() before d_instantiate() opens a window when > mkdir can race with open-by-fhandle on a guessed fhandle, leading > to multiple aliases for a directory inode and all the breakage > that follows from that. > > Correct solution: a new primitive (d_instantiate_new()) > combining these two in the right order - lockdep annotate, then > d_instantiate(), then the rest of unlock_new_inode(). All > combinations of d_instantiate() with unlock_new_inode() should > be converted to that. Ok, so this seems to touch only the paths that create new inodes (mkdir, mknod, etc). Is the lookup path that does: unlock_new_inode() ..... d_splice_alias(inode, dentry); OK? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
