The proper response to receiving NFS4ERR_BAD_SEQID from a server is to
try the failing operation again with a different (new) open owner that
has a reset sequence ID.
Our client drops a state owner when it receives NFS4ERR_BAD_SEQID,
as it should. But chances are good we will pick up exactly the same
open owner ID from the nfs_server's ida struct and use it for the
retry, which is not really useful.
To fix this, add a uniquifier to the open owners generated by our NFS
client. The per-nfs_server ida struct will continue to guarantee we
don't hand out the same open owner more than once concurrently, while
the new uniquifier guarantees that the open owner string is always
different after a NFS4ERR_BAD_SEQID error.
The current value of the uniqifier is planted in a state owner upon
its creation. If our client receives NFS4ERR_BAD_SEQID, it bumps the
uniquifier and drops the state owner. The next state owner to be
created will have a fresh uniquifier, and thus should appear different
to the server. State owners that continue to be cached on the client
remain unchanged.
A certain amount of code clean up was also done.
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 21 +++++++++++----------
fs/nfs/nfs4state.c | 16 +++++++++++-----
fs/nfs/nfs4xdr.c | 15 +++++++--------
include/linux/nfs_fs_sb.h | 1 +
include/linux/nfs_xdr.h | 6 ++++--
6 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index c87e846..d8c2d39 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -55,6 +55,7 @@ struct nfs4_minor_version_ops {
#define NFS_SEQID_CONFIRMED 1
struct nfs_seqid_counter {
int owner_id;
+ int instance;
int flags;
u32 counter;
spinlock_t lock; /* Protects the list */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6ae0c14..84a26d9 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -838,7 +838,8 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.open_flags = flags;
p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE);
p->o_arg.clientid = server->nfs_client->cl_clientid;
- p->o_arg.id = sp->so_seqid.owner_id;
+ p->o_arg.owner_id = sp->so_seqid.owner_id;
+ p->o_arg.instance = sp->so_seqid.instance;
p->o_arg.name = &dentry->d_name;
p->o_arg.server = server;
p->o_arg.bitmask = server->attr_bitmask;
@@ -1467,7 +1468,8 @@ static void nfs4_open_prepare(struct rpc_task *task, void *calldata)
rcu_read_unlock();
}
/* Update sequence id. */
- data->o_arg.id = sp->so_seqid.owner_id;
+ data->o_arg.owner_id = sp->so_seqid.owner_id;
+ data->o_arg.instance = sp->so_seqid.instance;
data->o_arg.clientid = sp->so_server->nfs_client->cl_clientid;
if (data->o_arg.claim == NFS4_OPEN_CLAIM_PREVIOUS) {
task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_NOATTR];
@@ -1871,13 +1873,9 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir, struct dentry *dentry,
* If we receive a BAD_SEQID error in the particular case of
* doing an OPEN, we assume that nfs_increment_open_seqid() will
* have unhashed the old state_owner for us, and that we can
- * therefore safely retry using a new one. We should still warn
- * the user though...
+ * therefore safely retry using a new one.
*/
if (status == -NFS4ERR_BAD_SEQID) {
- pr_warn_ratelimited("NFS: v4 server %s "
- " returned a bad sequence-id error!\n",
- NFS_SERVER(dir)->nfs_client->cl_hostname);
exception.retry = 1;
continue;
}
@@ -4137,7 +4135,8 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
if (status != 0)
goto out;
lsp = request->fl_u.nfs4_fl.owner;
- arg.lock_owner.id = lsp->ls_seqid.owner_id;
+ arg.lock_owner.owner_id = lsp->ls_seqid.owner_id;
+ arg.lock_owner.instance = lsp->ls_seqid.instance;
arg.lock_owner.s_dev = server->s_dev;
status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1);
switch (status) {
@@ -4382,7 +4381,8 @@ static struct nfs4_lockdata *nfs4_alloc_lockdata(struct file_lock *fl,
goto out_free_seqid;
p->arg.lock_stateid = &lsp->ls_stateid;
p->arg.lock_owner.clientid = server->nfs_client->cl_clientid;
- p->arg.lock_owner.id = lsp->ls_seqid.owner_id;
+ p->arg.lock_owner.owner_id = lsp->ls_seqid.owner_id;
+ p->arg.lock_owner.instance = lsp->ls_seqid.instance;
p->arg.lock_owner.s_dev = server->s_dev;
p->res.lock_seqid = p->arg.lock_seqid;
p->lsp = lsp;
@@ -4832,7 +4832,8 @@ int nfs4_release_lockowner(struct nfs4_lock_state *lsp)
data->lsp = lsp;
data->server = server;
data->args.lock_owner.clientid = server->nfs_client->cl_clientid;
- data->args.lock_owner.id = lsp->ls_seqid.owner_id;
+ data->args.lock_owner.owner_id = lsp->ls_seqid.owner_id;
+ data->args.lock_owner.instance = lsp->ls_seqid.instance;
data->args.lock_owner.s_dev = server->s_dev;
msg.rpc_argp = &data->args;
rpc_call_async(server->client, &msg, 0, &nfs4_release_lockowner_ops, data);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 0f43414..cbef366 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -503,6 +503,7 @@ struct nfs4_state_owner *nfs4_get_state_owner(struct nfs_server *server,
if (ida_pre_get(&server->openowner_id, gfp_flags) == 0)
break;
spin_lock(&clp->cl_lock);
+ new->so_seqid.instance = server->bad_seqid_count;
sp = nfs4_insert_state_owner_locked(new);
spin_unlock(&clp->cl_lock);
} while (sp == ERR_PTR(-EAGAIN));
@@ -763,6 +764,7 @@ static struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, f
{
struct nfs4_lock_state *lsp;
struct nfs_server *server = state->owner->so_server;
+ struct nfs_client *clp = server->nfs_client;
lsp = kzalloc(sizeof(*lsp), GFP_NOFS);
if (lsp == NULL)
@@ -784,6 +786,9 @@ static struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, f
lsp->ls_seqid.owner_id = ida_simple_get(&server->lockowner_id, 0, 0, GFP_NOFS);
if (lsp->ls_seqid.owner_id < 0)
goto out_free;
+ spin_lock(&clp->cl_lock);
+ lsp->ls_seqid.instance = server->bad_seqid_count;
+ spin_unlock(&clp->cl_lock);
INIT_LIST_HEAD(&lsp->ls_locks);
return lsp;
out_free:
@@ -989,10 +994,6 @@ static void nfs_increment_seqid(int status, struct nfs_seqid *seqid)
case -NFS4ERR_BAD_SEQID:
if (seqid->sequence->flags & NFS_SEQID_CONFIRMED)
return;
- pr_warn_ratelimited("NFS: v4 server returned a bad"
- " sequence-id error on an"
- " unconfirmed sequence %p!\n",
- seqid->sequence);
case -NFS4ERR_STALE_CLIENTID:
case -NFS4ERR_STALE_STATEID:
case -NFS4ERR_BAD_STATEID:
@@ -1015,8 +1016,13 @@ void nfs_increment_open_seqid(int status, struct nfs_seqid *seqid)
struct nfs4_state_owner, so_seqid);
struct nfs_server *server = sp->so_server;
- if (status == -NFS4ERR_BAD_SEQID)
+ if (status == -NFS4ERR_BAD_SEQID) {
+ struct nfs_client *clp = server->nfs_client;
+ spin_lock(&clp->cl_lock);
+ ++server->bad_seqid_count;
+ spin_unlock(&clp->cl_lock);
nfs4_drop_state_owner(sp);
+ }
if (!nfs4_has_session(server->nfs_client))
nfs_increment_seqid(status, seqid);
}
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c74fdb1..9fce30f 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1242,8 +1242,9 @@ static void encode_lockowner(struct xdr_stream *xdr, const struct nfs_lowner *lo
p = xdr_encode_hyper(p, lowner->clientid);
*p++ = cpu_to_be32(20);
p = xdr_encode_opaque_fixed(p, "lock id:", 8);
- *p++ = cpu_to_be32(lowner->s_dev);
- xdr_encode_hyper(p, lowner->id);
+ *p++ = (__be32)lowner->s_dev;
+ *p++ = (__be32)lowner->instance;
+ *p++ = (__be32)lowner->owner_id;
}
/*
@@ -1334,18 +1335,16 @@ static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode)
static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg)
{
__be32 *p;
- /*
- * opcode 4, seqid 4, share_access 4, share_deny 4, clientid 8, ownerlen 4,
- * owner 4 = 32
- */
+
encode_nfs4_seqid(xdr, arg->seqid);
encode_share_access(xdr, arg->fmode);
p = reserve_space(xdr, 32);
p = xdr_encode_hyper(p, arg->clientid);
*p++ = cpu_to_be32(20);
p = xdr_encode_opaque_fixed(p, "open id:", 8);
- *p++ = cpu_to_be32(arg->server->s_dev);
- xdr_encode_hyper(p, arg->id);
+ *p++ = (__be32)arg->server->s_dev;
+ *p++ = (__be32)arg->instance;
+ *p++ = (__be32)arg->owner_id;
}
static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_openargs *arg)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 59410b3..3c38291 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -157,6 +157,7 @@ struct nfs_server {
/* the following fields are protected by nfs_client->cl_lock */
struct rb_root state_owners;
+ u32 bad_seqid_count;
#endif
struct ida openowner_id;
struct ida lockowner_id;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index eb1ce73..ece705d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -321,7 +321,8 @@ struct nfs_openargs {
int open_flags;
fmode_t fmode;
__u64 clientid;
- __u64 id;
+ int owner_id;
+ int instance;
union {
struct {
struct iattr * attrs; /* UNCHECKED, GUARDED */
@@ -395,7 +396,8 @@ struct nfs_closeres {
* */
struct nfs_lowner {
__u64 clientid;
- __u64 id;
+ int owner_id;
+ int instance;
dev_t s_dev;
};
--
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
