From dacc7b6099f5c2e758e2811a7a6722bd0946eb39 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 20 Feb 2018 13:54:56 -0500 Subject: [PATCH] client: allow getattr, lookup, fstatx, and readdir to use implemented caps caps_issued_mask is a function that we call to see if we have all of the caps provided in a mask. Usually this ignores caps that have been revoked by the MDS but that have not yet been released by the client. These caps are still valid though -- nothing else can use them until they've been released. These codepaths call caps_issued_mask only in order to determine whether the client holds enough caps to trust cached attributes. They don't take new references to the caps, so it should be safe to use the "implemented" capset in these functions. This helps break a deadlock that can occur when the client holds a delegation that has been recalled, but needs to satisfy a getattr request before releasing it. Tracker: http://tracker.ceph.com/issues/23028 Signed-off-by: Jeff Layton --- src/client/Client.cc | 16 ++++++++-------- src/client/Inode.cc | 25 ++++++++++++++++++++++++- src/client/Inode.h | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 0ea23e82294..bb0163b11ef 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -6135,7 +6135,7 @@ int Client::_lookup(Inode *dir, const string& dname, int mask, InodeRef *target, << " seq " << dn->lease_seq << dendl; - if (!dn->inode || dn->inode->caps_issued_mask(mask)) { + if (!dn->inode || dn->inode->caps_issued_mask(mask, true)) { // is dn lease valid? utime_t now = ceph_clock_now(); if (dn->lease_mds >= 0 && @@ -6153,9 +6153,9 @@ int Client::_lookup(Inode *dir, const string& dname, int mask, InodeRef *target, << " vs lease_gen " << dn->lease_gen << dendl; } // dir lease? - if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED)) { + if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED, true)) { if (dn->cap_shared_gen == dir->shared_gen && - (!dn->inode || dn->inode->caps_issued_mask(mask))) + (!dn->inode || dn->inode->caps_issued_mask(mask, true))) goto hit_dn; if (!dn->inode && (dir->flags & I_COMPLETE)) { ldout(cct, 10) << __func__ << " concluded ENOENT locally for " @@ -6168,7 +6168,7 @@ int Client::_lookup(Inode *dir, const string& dname, int mask, InodeRef *target, } } else { // can we conclude ENOENT locally? - if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED) && + if (dir->caps_issued_mask(CEPH_CAP_FILE_SHARED, true) && (dir->flags & I_COMPLETE)) { ldout(cct, 10) << __func__ << " concluded ENOENT locally for " << *dir << " dn '" << dname << "'" << dendl; return -ENOENT; @@ -6632,7 +6632,7 @@ int Client::_readlink(Inode *in, char *buf, size_t size) int Client::_getattr(Inode *in, int mask, const UserPerm& perms, bool force) { - bool yes = in->caps_issued_mask(mask); + bool yes = in->caps_issued_mask(mask, true); ldout(cct, 10) << __func__ << " mask " << ccap_string(mask) << " issued=" << yes << dendl; if (yes && !force) @@ -7843,7 +7843,7 @@ int Client::readdir_r_cb(dir_result_t *d, add_dirent_cb_t cb, void *p, << dendl; if (dirp->inode->snapid != CEPH_SNAPDIR && dirp->inode->is_complete_and_ordered() && - dirp->inode->caps_issued_mask(CEPH_CAP_FILE_SHARED)) { + dirp->inode->caps_issued_mask(CEPH_CAP_FILE_SHARED, true)) { int err = _readdir_cache_cb(dirp, cb, p, caps, getref); if (err != -EAGAIN) return err; @@ -9524,7 +9524,7 @@ int Client::fstatx(int fd, struct ceph_statx *stx, const UserPerm& perms, unsigned mask = statx_to_mask(flags, want); int r = 0; - if (mask && !f->inode->caps_issued_mask(mask)) { + if (mask && !f->inode->caps_issued_mask(mask, true)) { r = _getattr(f->inode, mask, perms); if (r < 0) { ldout(cct, 3) << "fstatx exit on error!" << dendl; @@ -10610,7 +10610,7 @@ int Client::ll_getattrx(Inode *in, struct ceph_statx *stx, unsigned int want, int res = 0; unsigned mask = statx_to_mask(flags, want); - if (mask && !in->caps_issued_mask(mask)) + if (mask && !in->caps_issued_mask(mask, true)) res = _ll_getattr(in, mask, perms); if (res == 0) diff --git a/src/client/Inode.cc b/src/client/Inode.cc index 3f2502b4c9c..1c0e1f679a5 100644 --- a/src/client/Inode.cc +++ b/src/client/Inode.cc @@ -219,9 +219,27 @@ void Inode::try_touch_cap(mds_rank_t mds) } } -bool Inode::caps_issued_mask(unsigned mask) +/** + * caps_issued_mask - check whether we have all of the caps in the mask + * @mask: mask to check against + * @allow_impl: whether the caller can also use caps that are implemented but not issued + * + * This is the bog standard "check whether we have the required caps" operation. + * Typically, we only check against the capset that is currently "issued". + * In other words, we ignore caps that have been revoked but not yet released. + * + * Some callers (particularly those doing attribute retrieval) can also make + * use of the full set of "implemented" caps to satisfy requests from the + * cache. + * + * Those callers should refrain from taking new references to implemented + * caps! + */ +bool Inode::caps_issued_mask(unsigned mask, bool allow_impl) { int c = snap_caps; + int i = 0; + if ((c & mask) == mask) return true; // prefer auth cap @@ -240,8 +258,13 @@ bool Inode::caps_issued_mask(unsigned mask) return true; } c |= cap.issued; + i |= cap.implemented; } } + + if (allow_impl) + c |= i; + if ((c & mask) == mask) { // bah.. touch them all for (auto &pair : caps) { diff --git a/src/client/Inode.h b/src/client/Inode.h index 04d3ff312a6..7c25f8d5ffc 100644 --- a/src/client/Inode.h +++ b/src/client/Inode.h @@ -306,7 +306,7 @@ struct Inode { bool cap_is_valid(const Cap &cap) const; int caps_issued(int *implemented = 0) const; void try_touch_cap(mds_rank_t mds); - bool caps_issued_mask(unsigned mask); + bool caps_issued_mask(unsigned mask, bool allow_impl=false); int caps_used(); int caps_file_wanted(); int caps_wanted(); -- 2.39.5