linux-kernel - Re: [NFS] [PATCH] NFS: supress warnings about NFS4ERR_OLD_STATEID in nfs4_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1178030549.5444.11.camel@heimdal.trondhjem.org>
Date:	Tue, 01 May 2007 07:42:29 -0700
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Jeff Layton <jlayton@...hat.com>
Cc:	nfs@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	ffilz@...ibm.com
Subject: Re: [NFS] [PATCH] NFS: supress warnings about NFS4ERR_OLD_STATEID
	in	nfs4_handle_exception

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@...hat.com>
> Signed-off-by: Frank Filz <ffilz@...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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/