Re: NFSv4.1: Handle NFS4ERR_DELAY on SEQUENCE correctly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jul. 13, 2010, 0:52 +0300, Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> 
> In RFC5661, an NFS4ERR_DELAY error on a SEQUENCE operation has the special
> meaning that the server is not finished processing the request. In this
> case we want to just retry the request without touching the slot.
> 
> Also fix a bug whereby we would fail to update the sequence id if the
> server returned any error other than NFS_OK/NFS4ERR_DELAY.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>

Thanks Trond!
I also merged it into the linux-pnfs tree so it gets some more testing
there too.
I'm not sure how exactly the hack I previously had actually worked,
but we did see the warning and nothing seemed to be blocked after that
or gone bad in any other way.

Benny

> ---
> 
>  fs/nfs/nfs4proc.c |   92 ++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 62 insertions(+), 30 deletions(-)
> 
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index d6413b4..ba79c0e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -389,11 +389,12 @@ static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
>  	res->sr_slotid = NFS4_MAX_SLOT_TABLE;
>  }
>  
> -static void nfs41_sequence_done(struct nfs4_sequence_res *res)
> +static int nfs41_sequence_done(struct rpc_task *task, struct nfs4_sequence_res *res)
>  {
>  	unsigned long timestamp;
>  	struct nfs4_slot_table *tbl;
>  	struct nfs4_slot *slot;
> +	struct nfs_client *clp;
>  
>  	/*
>  	 * sr_status remains 1 if an RPC level error occurred. The server
> @@ -408,14 +409,16 @@ static void nfs41_sequence_done(struct nfs4_sequence_res *res)
>  	if (res->sr_slotid == NFS4_MAX_SLOT_TABLE)
>  		goto out;
>  
> +	tbl = &res->sr_session->fc_slot_table;
> +	slot = tbl->slots + res->sr_slotid;
> +
>  	/* Check the SEQUENCE operation status */
> -	if (res->sr_status == 0) {
> -		struct nfs_client *clp = res->sr_session->clp;
> -		tbl = &res->sr_session->fc_slot_table;
> -		slot = tbl->slots + res->sr_slotid;
> +	switch (res->sr_status) {
> +	case 0:
>  		/* Update the slot's sequence and clientid lease timer */
>  		++slot->seq_nr;
>  		timestamp = res->sr_renewal_time;
> +		clp = res->sr_session->clp;
>  		spin_lock(&clp->cl_lock);
>  		if (time_before(clp->cl_last_renewal, timestamp))
>  			clp->cl_last_renewal = timestamp;
> @@ -423,18 +426,39 @@ static void nfs41_sequence_done(struct nfs4_sequence_res *res)
>  		/* Check sequence flags */
>  		if (atomic_read(&clp->cl_count) > 1)
>  			nfs41_handle_sequence_flag_errors(clp, res->sr_status_flags);
> +		break;
> +	case -NFS4ERR_DELAY:
> +		/* The server detected a resend of the RPC call and
> +		 * returned NFS4ERR_DELAY as per Section 2.10.6.2
> +		 * of RFC5661.
> +		 */
> +		dprintk("%s: slot=%d seq=%d: Operation in progress\n",
> +				__func__, res->sr_slotid, slot->seq_nr);
> +		goto out_retry;
> +	default:
> +		/* Just update the slot sequence no. */
> +		++slot->seq_nr;
>  	}
>  out:
>  	/* The session may be reset by one of the error handlers. */
>  	dprintk("%s: Error %d free the slot \n", __func__, res->sr_status);
>  	nfs41_sequence_free_slot(res);
> +	return 1;
> +out_retry:
> +	rpc_delay(task, NFS4_POLL_RETRY_MAX);
> +	rpc_restart_call(task);
> +	/* FIXME: rpc_restart_call() should be made to return success/fail */
> +	if (RPC_ASSASSINATED(task))
> +		goto out;
> +	return 0;
>  }
>  
> -static void nfs4_sequence_done(const struct nfs_server *server,
> -			       struct nfs4_sequence_res *res, int rpc_status)
> +static int nfs4_sequence_done(struct rpc_task *task,
> +			       struct nfs4_sequence_res *res)
>  {
> -	if (res->sr_session != NULL)
> -		nfs41_sequence_done(res);
> +	if (res->sr_session == NULL)
> +		return 1;
> +	return nfs41_sequence_done(task, res);
>  }
>  
>  /*
> @@ -592,7 +616,7 @@ static void nfs41_call_sync_done(struct rpc_task *task, void *calldata)
>  {
>  	struct nfs41_call_sync_data *data = calldata;
>  
> -	nfs41_sequence_done(data->seq_res);
> +	nfs41_sequence_done(task, data->seq_res);
>  }
>  
>  struct rpc_call_ops nfs41_call_sync_ops = {
> @@ -650,9 +674,10 @@ int _nfs4_call_sync_session(struct nfs_server *server,
>  }
>  
>  #else
> -static void nfs4_sequence_done(const struct nfs_server *server,
> -			       struct nfs4_sequence_res *res, int rpc_status)
> +static int nfs4_sequence_done(struct rpc_task *task,
> +			       struct nfs4_sequence_res *res)
>  {
> +	return 1;
>  }
>  #endif /* CONFIG_NFS_V4_1 */
>  
> @@ -1379,8 +1404,8 @@ static void nfs4_open_done(struct rpc_task *task, void *calldata)
>  
>  	data->rpc_status = task->tk_status;
>  
> -	nfs4_sequence_done(data->o_arg.server, &data->o_res.seq_res,
> -			task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->o_res.seq_res))
> +		return;
>  
>  	if (RPC_ASSASSINATED(task))
>  		return;
> @@ -1832,7 +1857,8 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
>  	struct nfs4_state *state = calldata->state;
>  	struct nfs_server *server = NFS_SERVER(calldata->inode);
>  
> -	nfs4_sequence_done(server, &calldata->res.seq_res, task->tk_status);
> +	if (!nfs4_sequence_done(task, &calldata->res.seq_res))
> +		return;
>  	if (RPC_ASSASSINATED(task))
>  		return;
>          /* hmm. we are done with the inode, and in the process of freeing
> @@ -2642,7 +2668,8 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir)
>  {
>  	struct nfs_removeres *res = task->tk_msg.rpc_resp;
>  
> -	nfs4_sequence_done(res->server, &res->seq_res, task->tk_status);
> +	if (!nfs4_sequence_done(task, &res->seq_res))
> +		return 0;
>  	if (nfs4_async_handle_error(task, res->server, NULL) == -EAGAIN)
>  		return 0;
>  	update_changeattr(dir, &res->cinfo);
> @@ -3087,7 +3114,8 @@ static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
>  
>  	dprintk("--> %s\n", __func__);
>  
> -	nfs4_sequence_done(server, &data->res.seq_res, task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->res.seq_res))
> +		return -EAGAIN;
>  
>  	if (nfs4_async_handle_error(task, server, data->args.context->state) == -EAGAIN) {
>  		nfs_restart_rpc(task, server->nfs_client);
> @@ -3110,8 +3138,8 @@ static int nfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
>  {
>  	struct inode *inode = data->inode;
>  	
> -	nfs4_sequence_done(NFS_SERVER(inode), &data->res.seq_res,
> -			   task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->res.seq_res))
> +		return -EAGAIN;
>  
>  	if (nfs4_async_handle_error(task, NFS_SERVER(inode), data->args.context->state) == -EAGAIN) {
>  		nfs_restart_rpc(task, NFS_SERVER(inode)->nfs_client);
> @@ -3139,8 +3167,9 @@ static int nfs4_commit_done(struct rpc_task *task, struct nfs_write_data *data)
>  {
>  	struct inode *inode = data->inode;
>  	
> -	nfs4_sequence_done(NFS_SERVER(inode), &data->res.seq_res,
> -			   task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->res.seq_res))
> +		return -EAGAIN;
> +
>  	if (nfs4_async_handle_error(task, NFS_SERVER(inode), NULL) == -EAGAIN) {
>  		nfs_restart_rpc(task, NFS_SERVER(inode)->nfs_client);
>  		return -EAGAIN;
> @@ -3630,8 +3659,8 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
>  {
>  	struct nfs4_delegreturndata *data = calldata;
>  
> -	nfs4_sequence_done(data->res.server, &data->res.seq_res,
> -			task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->res.seq_res))
> +		return;
>  
>  	switch (task->tk_status) {
>  	case -NFS4ERR_STALE_STATEID:
> @@ -3881,8 +3910,8 @@ static void nfs4_locku_done(struct rpc_task *task, void *data)
>  {
>  	struct nfs4_unlockdata *calldata = data;
>  
> -	nfs4_sequence_done(calldata->server, &calldata->res.seq_res,
> -			   task->tk_status);
> +	if (!nfs4_sequence_done(task, &calldata->res.seq_res))
> +		return;
>  	if (RPC_ASSASSINATED(task))
>  		return;
>  	switch (task->tk_status) {
> @@ -4091,8 +4120,8 @@ static void nfs4_lock_done(struct rpc_task *task, void *calldata)
>  
>  	dprintk("%s: begin!\n", __func__);
>  
> -	nfs4_sequence_done(data->server, &data->res.seq_res,
> -			task->tk_status);
> +	if (!nfs4_sequence_done(task, &data->res.seq_res))
> +		return;
>  
>  	data->rpc_status = task->tk_status;
>  	if (RPC_ASSASSINATED(task))
> @@ -4629,7 +4658,8 @@ static void nfs4_get_lease_time_done(struct rpc_task *task, void *calldata)
>  			(struct nfs4_get_lease_time_data *)calldata;
>  
>  	dprintk("--> %s\n", __func__);
> -	nfs41_sequence_done(&data->res->lr_seq_res);
> +	if (!nfs41_sequence_done(task, &data->res->lr_seq_res))
> +		return;
>  	switch (task->tk_status) {
>  	case -NFS4ERR_DELAY:
>  	case -NFS4ERR_GRACE:
> @@ -5111,7 +5141,8 @@ static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
>  	struct nfs4_sequence_data *calldata = data;
>  	struct nfs_client *clp = calldata->clp;
>  
> -	nfs41_sequence_done(task->tk_msg.rpc_resp);
> +	if (!nfs41_sequence_done(task, task->tk_msg.rpc_resp))
> +		return;
>  
>  	if (task->tk_status < 0) {
>  		dprintk("%s ERROR %d\n", __func__, task->tk_status);
> @@ -5255,7 +5286,8 @@ static void nfs4_reclaim_complete_done(struct rpc_task *task, void *data)
>  	struct nfs4_sequence_res *res = &calldata->res.seq_res;
>  
>  	dprintk("--> %s\n", __func__);
> -	nfs41_sequence_done(res);
> +	if (!nfs41_sequence_done(task, res))
> +		return;
>  
>  	if (nfs41_reclaim_complete_handle_errors(task, clp) == -EAGAIN) {
>  		rpc_restart_call_prepare(task);
> 
--
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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux