Skip to content

Commit c3f4688

Browse files
jtlaytonidryomov
authored andcommitted
ceph: don't set req->r_locked_dir in ceph_d_revalidate
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]>
1 parent 3e5de27 commit c3f4688

File tree

1 file changed

+14
-10
lines changed

1 file changed

+14
-10
lines changed

fs/ceph/dir.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,26 +1261,30 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
12611261
return -ECHILD;
12621262

12631263
op = ceph_snap(dir) == CEPH_SNAPDIR ?
1264-
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_LOOKUP;
1264+
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_GETATTR;
12651265
req = ceph_mdsc_create_request(mdsc, op, USE_ANY_MDS);
12661266
if (!IS_ERR(req)) {
12671267
req->r_dentry = dget(dentry);
1268-
req->r_num_caps = 2;
1268+
req->r_num_caps = op == CEPH_MDS_OP_GETATTR ? 1 : 2;
12691269

12701270
mask = CEPH_STAT_CAP_INODE | CEPH_CAP_AUTH_SHARED;
12711271
if (ceph_security_xattr_wanted(dir))
12721272
mask |= CEPH_CAP_XATTR_SHARED;
12731273
req->r_args.getattr.mask = mask;
12741274

1275-
req->r_locked_dir = dir;
12761275
err = ceph_mdsc_do_request(mdsc, NULL, req);
1277-
if (err == 0 || err == -ENOENT) {
1278-
if (dentry == req->r_dentry) {
1279-
valid = !d_unhashed(dentry);
1280-
} else {
1281-
d_invalidate(req->r_dentry);
1282-
err = -EAGAIN;
1283-
}
1276+
switch (err) {
1277+
case 0:
1278+
if (d_really_is_positive(dentry) &&
1279+
d_inode(dentry) == req->r_target_inode)
1280+
valid = 1;
1281+
break;
1282+
case -ENOENT:
1283+
if (d_really_is_negative(dentry))
1284+
valid = 1;
1285+
/* Fallthrough */
1286+
default:
1287+
break;
12841288
}
12851289
ceph_mdsc_put_request(req);
12861290
dout("d_revalidate %p lookup result=%d\n",

0 commit comments

Comments
 (0)