On Mon, Jan 28, 2013 at 09:20:34PM -0800, Sage Weil wrote:
> Yep, that is indeed a problem. I think we just need to do the r_aborted
> and/or r_locked_dir check in the else if condition...
>
> > I'm not sure if we are guaranteed that ceph_readdir_prepopulate() won't
> > get to its splice_dentry() and d_delete() calls in similar situations -
> > I hadn't checked that one yet. If it isn't guaranteed, we have a problem
> > there as well.
>
> ...and the condition guarding readdir_prepopulate(). :)
> I think you're reading it correctly. The main thing to keep in mind here
> is that we *do* need to call fill_inode() for the inode metadata on these
> requests to keep the mds and client state in sync. The dentry state is
> safe to ignore.
You mean the parts under
if (rinfo->head->is_dentry) {
and
if (rinfo->head->is_target) {
in there? Because there's fill_inode() called from readdir_prepopulate()
and it's a lot more problematic than those two...
> It would be great to have the dir i_mutex rules summarized somewhere, even
> if it is just a copy of the below. It took a fair bit of trial and error
> to infer what was going on when writing this code. :)
Directory ->i_mutex rules are in part documented - "what VFS guarantees
to hold" side is in Documentation/filesystems/directory-locking. It's
the other side ("what locks are expected to be held by callers of dcache.c
functions") that is badly missing...
> Ping me when you've pushed that branch and I'll take a look...
To gitolite@xxxxxxxxxxxxx:/pub/scm/linux/kernel/git/viro/vfs.git
01a88fa..4056362 master -> master
with tentative ceph patch in the very end. Should be on git.kernel.org
shortly...
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
