Skip to content

Commit

Permalink
ceph: don't set req->r_locked_dir in ceph_d_revalidate
Browse files Browse the repository at this point in the history
This function sets req->r_locked_dir which is supposed to indicate to
ceph_fill_trace that the parent's i_rwsem is locked for write.
Unfortunately, there is no guarantee that the dir will be locked when
d_revalidate is called, so we really don't want ceph_fill_trace to do
any dcache manipulation from this context. Clear req->r_locked_dir since
it's clearly not safe to do that.

What we really want to know with d_revalidate is whether the dentry
still points to the same inode. ceph_fill_trace installs a pointer to
the inode in req->r_target_inode, so we can just compare that to
d_inode(dentry) to see if it's the same one after the lookup.

Also, since we aren't generally interested in the parent here, we can
switch to using a GETATTR to hint that to the MDS, which also means that
we only need to reserve one cap.

Finally, just remove the d_unhashed check. That's really outside the
purview of a filesystem's d_revalidate. If the thing became unhashed
while we're checking it, then that's up to the VFS to handle anyway.

Fixes: 200fd27 ("ceph: use lookup request to revalidate dentry")
Link: http://tracker.ceph.com/issues/18041
Reported-by: Donatas Abraitis <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
Reviewed-by: "Yan, Zheng" <[email protected]>
Signed-off-by: Ilya Dryomov <[email protected]>
  • Loading branch information
jtlayton authored and idryomov committed Dec 8, 2016
1 parent 3e5de27 commit c3f4688
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions fs/ceph/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1261,26 +1261,30 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
return -ECHILD;

op = ceph_snap(dir) == CEPH_SNAPDIR ?
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_LOOKUP;
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_GETATTR;
req = ceph_mdsc_create_request(mdsc, op, USE_ANY_MDS);
if (!IS_ERR(req)) {
req->r_dentry = dget(dentry);
req->r_num_caps = 2;
req->r_num_caps = op == CEPH_MDS_OP_GETATTR ? 1 : 2;

mask = CEPH_STAT_CAP_INODE | CEPH_CAP_AUTH_SHARED;
if (ceph_security_xattr_wanted(dir))
mask |= CEPH_CAP_XATTR_SHARED;
req->r_args.getattr.mask = mask;

req->r_locked_dir = dir;
err = ceph_mdsc_do_request(mdsc, NULL, req);
if (err == 0 || err == -ENOENT) {
if (dentry == req->r_dentry) {
valid = !d_unhashed(dentry);
} else {
d_invalidate(req->r_dentry);
err = -EAGAIN;
}
switch (err) {
case 0:
if (d_really_is_positive(dentry) &&
d_inode(dentry) == req->r_target_inode)
valid = 1;
break;
case -ENOENT:
if (d_really_is_negative(dentry))
valid = 1;
/* Fallthrough */
default:
break;
}
ceph_mdsc_put_request(req);
dout("d_revalidate %p lookup result=%d\n",
Expand Down

0 comments on commit c3f4688

Please sign in to comment.