Re: [RFC] several ceph dentry leaks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 2, 2015 at 12:41 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> 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?

In theory, yes. But I think it never happens in reality. Because
parent directory of the newly created
file/directory is locked, client has no other way to lookup the newly
created file/directory.


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...
>

I think the reason is that VFS still uses the original dentry. (for
example, after calling
filesystem's mkdir callback, vfs_mkdir() calls fsnotify_mkdir() and
passing the original
dentry to it)


> 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?

you are right, it's incorrect.

how about make ceph_handle_notrace_create()  return an error when it
gets an non-NULL
from ceph_lookup() ? this method is not perfect but should work in most case.

Regards
Yan, Zheng
--
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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux