On Mon, Feb 02, 2015 at 11:23:58AM +0800, Yan, Zheng wrote: > > we should _never_ create multiple dentries pointing to directory inode, so > > d_instantiate() in there isn't mitigating anything - it's actively breaking > > things as far as the rest of the kernel is concerned... What are you > > trying to do there? > > ceph_handle_notrace_create() is for case: Ceph Metadata server restarted, > client re-sent a request. Ceph MDS found the request has already completed, > so it sent a traceless reply to client. (traceless reply contains no information > for the dentry and the newly created inode). It's hard to handle this case > elegantly, because MDS may have done other operation, which moved the > newly created file/directory to other place. > > For multiple dentries pointing to directory inode, maybe we can return an error > (-ENOENT or -ESTALE). IDGI... Suppose we got non-NULL from ceph_lookup() there. So we'd sent either CEPH_MDS_OP_LOOKUP or CEPH_MDS_OP_LOOKUPSNAP and got ->r_dentry changed (presumably in ceph_fill_trace(), after it got the sucker from splice_dentry(), i.e. ultimately d_splice_alias()). Right? Now, we have acquired a reference to it (in ceph_finish_lookup()). So far, so good; for one thing, we definitely do *NOT* want to forget about that reference. For another, we've got a good and valid dentry; so why are we playing with the originally passed one? Just unhash the original and be done with that... Looking at the code downstream from the calls of ceph_handle_notrace_create(), ceph_mkdir() looks very dubious - we do ceph_init_inode_acls(dentry->d_inode, &acls); there, after having set ->d_inode to that of a preexisting alias. Is that really correct in the case when such an alias used to exist? -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
