Re: [NFS] [PATCH] NFS: supress warnings about NFS4ERR_OLD_STATEID in nfs4_handle_exception

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Thu, 2007-04-26 at 15:13 -0400, Jeff Layton wrote:
> I recently posted this patch to the nfs mailing list, but got no comment.
> Reposting with an actual "[PATCH]" subject...
> 
> In certain situations, a NFSv4 client can end up with a set of racing calls
> that cause the server to return a NFS4ERR_OLD_STATEID error. When this occurs,
> the only real recourse that the client has is to retry the call. In the
> current code, when this occurs, it triggers a useless printk (in
> nfs4_map_errors):
> 
>     nfs4_map_errors could not handle NFSv4 error 10024
> 
> Trond recently mentioned that we really don't need to worry users with that
> info. The following patch is based on a suggestion by Frank Filz and supresses
> this error. Changing the return code here to 0 should be safe since all callers
> of nfs4_handle_exception are expected to retry the call in this situation
> anyway.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> Signed-off-by: Frank Filz <ffilz@us.ibm.com>
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index f52cf5c..b456783 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2774,6 +2774,7 @@ int nfs4_handle_exception(const struct nfs_server *server, int errorcode, struct
>  				break;
>  		case -NFS4ERR_OLD_STATEID:
>  			exception->retry = 1;
> +			ret = 0;
>  	}
>  	/* We failed to handle the error */
>  	return nfs4_map_errors(ret);

Surely "return 0;" is more efficient.

...and a minor nit about the comment: it doesn't trigger a printk(). It
rather triggers a dprintk(), (which should be turned off by default
unless you happen to be debugging the code).

The other nit is that I'm not sure the dprintk() is totally useless. It
is always nice to know _why_ the thing looped when you are debugging.
That said, we should perhaps rather put the dprintk() for that in
nfs4_handle_exception instead...

Trond