We've observed the nfsd server in a state where there are
multiple delegations on the same nfs4_file for the same client.
The nfs client does attempt to DELEGRETURN these when they are presented to
it - but apparently under some (unknown) circumstances the client does not
manage to return all of them. This leads to the eventual
attempt to CB_RECALL more than one delegation with the same nfs
filehandle to the same client. The first recall will succeed, but the
next recall will fail with NFS4ERR_BADHANDLE. This leads to the server
having delegations on cl_revoked that the client has no way to FREE
or DELEGRETURN, with resulting inability to recover. The state manager
on the server will continually assert SEQ4_STATUS_RECALLABLE_STATE_REVOKED,
and the state manager on the client will be looping unable to satisfy
the server.
So, let's not hand out duplicate delegations in the first place.
RFC 5561:
9.1.1:
"Delegations and layouts, on the other hand, are not associated with a
specific owner but are associated with the client as a whole
(identified by a client ID)."
10.2:
"...the stateid for a delegation is associated with a client ID and may be
used on behalf of all the open-owners for the given client. A
delegation is made to the client as a whole and not to any specific
process or thread of control within it."
Reported-by: Eric Meddaugh <etmsys@xxxxxxx>
Signed-off-by: Andrew Elble <aweits@xxxxxxx>
---
fs/nfsd/nfs4state.c | 147 +++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 127 insertions(+), 20 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0f1d5691b795..eea5c84101e5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -765,16 +765,84 @@ void nfs4_unhash_stid(struct nfs4_stid *s)
s->sc_type = 0;
}
-static void
+static int
+same_clid(clientid_t *cl1, clientid_t *cl2)
+{
+ return (cl1->cl_boot == cl2->cl_boot) && (cl1->cl_id == cl2->cl_id);
+}
+
+/**
+ * nfs4_get_existing_delegation - Discover if this delegation already exists
+ * @clp: a pointer to the nfs4_client we're granting a delegation to
+ * @fp: a pointer to the nfs4_file we're granting a delegation on
+ *
+ * Return:
+ * On success: NULL if the delegation was successfully hashed.
+ *
+ * On error: an ERR_PTR if there was an existing delegation, but
+ * it is being recalled - or, a pointer to the existing nfs4_delegation
+ * if one was previously granted to this nfs4_client for this nfs4_file.
+ *
+ */
+
+static struct nfs4_delegation *
+nfs4_get_existing_delegation(struct nfs4_client *clp, struct nfs4_file *fp)
+{
+ struct nfs4_delegation *searchdp = NULL;
+ struct nfs4_client *searchclp = NULL;
+
+ lockdep_assert_held(&state_lock);
+ lockdep_assert_held(&fp->fi_lock);
+
+ list_for_each_entry(searchdp, &fp->fi_delegations, dl_perfile) {
+ searchclp = searchdp->dl_stid.sc_client;
+ if (same_clid(&clp->cl_clientid, &searchclp->cl_clientid)) {
+ if (searchdp->dl_time == 0) {
+ atomic_inc(&searchdp->dl_stid.sc_count);
+ return searchdp;
+ } else {
+ return ERR_PTR(-EAGAIN);
+ }
+ }
+ }
+ return NULL;
+}
+
+/**
+ * hash_delegation_locked - Add a delegation to the appropriate lists
+ * @dp: a pointer to the nfs4_delegation we are adding.
+ * @fp: a pointer to the nfs4_file we're granting a delegation on
+ *
+ * Return:
+ * On success: NULL if the delegation was successfully hashed.
+ *
+ * On error: an ERR_PTR if there was an existing delegation, but
+ * it is being recalled - or, a pointer to the existing
+ * nfs4_delegation if one was previously granted to this
+ * nfs4_client for this nfs4_file.
+ *
+ */
+
+static struct nfs4_delegation *
hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
{
+ struct nfs4_delegation *searchdp = NULL;
+ struct nfs4_client *clp = dp->dl_stid.sc_client;
+
lockdep_assert_held(&state_lock);
lockdep_assert_held(&fp->fi_lock);
- atomic_inc(&dp->dl_stid.sc_count);
- dp->dl_stid.sc_type = NFS4_DELEG_STID;
- list_add(&dp->dl_perfile, &fp->fi_delegations);
- list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
+ searchdp = nfs4_get_existing_delegation(clp, fp);
+ if (!searchdp) {
+ ++fp->fi_delegees;
+ atomic_inc(&dp->dl_stid.sc_count);
+ dp->dl_stid.sc_type = NFS4_DELEG_STID;
+ list_add(&dp->dl_perfile, &fp->fi_delegations);
+ list_add(&dp->dl_perclnt, &clp->cl_delegations);
+ } else {
+ return searchdp;
+ }
+ return NULL;
}
static bool
@@ -1833,12 +1901,6 @@ same_verf(nfs4_verifier *v1, nfs4_verifier *v2)
return 0 == memcmp(v1->data, v2->data, sizeof(v1->data));
}
-static int
-same_clid(clientid_t *cl1, clientid_t *cl2)
-{
- return (cl1->cl_boot == cl2->cl_boot) && (cl1->cl_id == cl2->cl_id);
-}
-
static bool groups_equal(struct group_info *g1, struct group_info *g2)
{
int i;
@@ -3945,9 +4007,29 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
return fl;
}
-static int nfs4_setlease(struct nfs4_delegation *dp)
+/**
+ * nfs4_setlease - Obtain a delegation by requesting locks from vfs layer
+ * @indp: a pointer to a pointer to the nfs4_delegation we're adding.
+ *
+ * Return:
+ * On success: If there already was an existing delegation for
+ * this client on this file, the allocated delegation we were
+ * passed is dropped and the pointer is changed and passed back
+ * to the caller to reflect this. If the delegation is not
+ * already hashed, there are no changes to the caller's version
+ * of the delegation. Return code will be 0 on success.
+ *
+ * On error: an ERR_PTR will be passed back on indp if there was
+ * an existing delegation, but it is being recalled. Return code
+ * will be an nonzero if there is an error in other cases.
+ *
+ */
+
+static int nfs4_setlease(struct nfs4_delegation **indp)
{
+ struct nfs4_delegation *dp = *indp;
struct nfs4_file *fp = dp->dl_stid.sc_file;
+ struct nfs4_delegation *found = NULL;
struct file_lock *fl;
struct file *filp;
int status = 0;
@@ -3977,34 +4059,55 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
/* Race breaker */
if (fp->fi_deleg_file) {
status = 0;
- ++fp->fi_delegees;
- hash_delegation_locked(dp, fp);
+ found = hash_delegation_locked(dp, fp);
goto out_unlock;
}
fp->fi_deleg_file = filp;
- fp->fi_delegees = 1;
- hash_delegation_locked(dp, fp);
+ fp->fi_delegees = 0;
+ found = hash_delegation_locked(dp, fp);
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
+ if (found) {
+ /* Should never happen, this is a new fi_deleg_file */
+ WARN_ON_ONCE(1);
+ goto out;
+ }
return 0;
out_unlock:
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
+ if (found) {
+out:
+ put_clnt_odstate(dp->dl_clnt_odstate);
+ nfs4_put_stid(&dp->dl_stid);
+ *indp = found;
+ }
out_fput:
fput(filp);
return status;
}
+
static struct nfs4_delegation *
nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
struct nfs4_file *fp, struct nfs4_clnt_odstate *odstate)
{
int status;
- struct nfs4_delegation *dp;
+ struct nfs4_delegation *dp = NULL;
+ struct nfs4_delegation *found = NULL;
if (fp->fi_had_conflict)
return ERR_PTR(-EAGAIN);
+ spin_lock(&state_lock);
+ spin_lock(&fp->fi_lock);
+ dp = nfs4_get_existing_delegation(clp, fp);
+ spin_unlock(&fp->fi_lock);
+ spin_unlock(&state_lock);
+
+ if (dp)
+ return dp;
+
dp = alloc_init_deleg(clp, fh, odstate);
if (!dp)
return ERR_PTR(-ENOMEM);
@@ -4016,19 +4119,23 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
if (!fp->fi_deleg_file) {
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
- status = nfs4_setlease(dp);
+ status = nfs4_setlease(&dp);
goto out;
}
if (fp->fi_had_conflict) {
status = -EAGAIN;
goto out_unlock;
}
- ++fp->fi_delegees;
- hash_delegation_locked(dp, fp);
+ found = hash_delegation_locked(dp, fp);
status = 0;
out_unlock:
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
+ if (found) {
+ put_clnt_odstate(dp->dl_clnt_odstate);
+ nfs4_put_stid(&dp->dl_stid);
+ dp = found;
+ }
out:
if (status) {
put_clnt_odstate(dp->dl_clnt_odstate);
--
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
