On Fri, Jun 17, 2016 at 12:09:19AM -0400, Oleg Drokin wrote: > So they both do d_drop(), the dentry is now unhashed, and they both > dive into nfs_lookup(). > There eventually they both call > > res = d_splice_alias(inode, dentry); > > And so the first lucky one continues on it's merry way with a hashed dentry, > but the other less lucky one ends up calling into d_splice_alias() with > dentry that's already hashed and hits the very familiar assertion. > > I took a brief look into ceph and it looks like a very similar thing > might happen there with handle_reply() for two parallel replies calling into > ceph_fill_trace() and then splice_alias()->d_splice_alias(), since the > unhashed check it does is not under any locks, it's unsafe, so the problem > might be more generic than just NFS too. > > So I wonder how to best fix this? Holding some sort of dentry lock across a call > into atomic_open in VFS? We cannot just make d_splice_alias() callers call with > inode->i_lock held because dentry might be negative. Oh, lovely... So basically the problem is that we violate the "no lookups on the same name in parallel" rule on those fallbacks from foo_atomic_open() to foo_lookup(). The thing is, a lot of ->atomic_open() instances have such fallbacks and I wonder if that's a sign that we need to lift some of that to fs/namei.c... Hell knows; alternative is to have that d_drop() followed by d_alloc_parallel() and feeding that dentry to lookup. I'll play with that a bit and see what's better; hopefully I'll have something by tomorrow. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
