From patchwork Tue Aug 23 21:41:41 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "J. Bruce Fields" X-Patchwork-Id: 1090002 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p7NLfimT019619 for ; Tue, 23 Aug 2011 21:41:44 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753247Ab1HWVln (ORCPT ); Tue, 23 Aug 2011 17:41:43 -0400 Received: from fieldses.org ([174.143.236.118]:43153 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752730Ab1HWVlm (ORCPT ); Tue, 23 Aug 2011 17:41:42 -0400 Received: from bfields by fieldses.org with local (Exim 4.72) (envelope-from ) id 1Qvyj7-0006m8-Bq; Tue, 23 Aug 2011 17:41:41 -0400 Date: Tue, 23 Aug 2011 17:41:41 -0400 From: "J. Bruce Fields" To: Trond Myklebust Cc: linux-nfs@vger.kernel.org Subject: Re: [PATCH] nfsd4: fix seqid_mutating_error Message-ID: <20110823214141.GC25350@fieldses.org> References: <20110819172416.GA1112@fieldses.org> <1313788373.26701.1.camel@lade.trondhjem.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1313788373.26701.1.camel@lade.trondhjem.org> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 23 Aug 2011 21:41:44 +0000 (UTC) On Fri, Aug 19, 2011 at 05:12:53PM -0400, Trond Myklebust wrote: > On Fri, 2011-08-19 at 13:24 -0400, J. Bruce Fields wrote: > > +static bool seqid_mutating_err(__be32 err) > > +{ > > + /* rfc 3530 section 8.1.5: */ > > + return err != nfserr_stale_clientid && > > + err != nfserr_stale_stateid && > > + err != nfserr_bad_stateid && > > + err != nfserr_bad_seqid && > > + err != nfserr_bad_xdr && > > + err != nfserr_resource && > > + err != nfserr_nofilehandle; > > Any reason not to convert the above into a switch() statement while you > are at it? It would be more straightforward without the negatives. Also, we've got two copies of this list. How about something like this? --b. commit 9ee2cabf7a814d48798380bc2cb8516645d1d0dc Author: J. Bruce Fields Date: Tue Aug 23 15:43:04 2011 -0400 nfsd4: cleanup and consolidate seqid_mutating_err Signed-off-by: J. Bruce Fields --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index 1ec1a85..1a652a0 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -13,30 +13,6 @@ struct idmap; -/* - * In a seqid-mutating op, this macro controls which error return - * values trigger incrementation of the seqid. - * - * from rfc 3010: - * The client MUST monotonically increment the sequence number for the - * CLOSE, LOCK, LOCKU, OPEN, OPEN_CONFIRM, and OPEN_DOWNGRADE - * operations. This is true even in the event that the previous - * operation that used the sequence number received an error. The only - * exception to this rule is if the previous operation received one of - * the following errors: NFSERR_STALE_CLIENTID, NFSERR_STALE_STATEID, - * NFSERR_BAD_STATEID, NFSERR_BAD_SEQID, NFSERR_BADXDR, - * NFSERR_RESOURCE, NFSERR_NOFILEHANDLE. - * - */ -#define seqid_mutating_err(err) \ -(((err) != NFSERR_STALE_CLIENTID) && \ - ((err) != NFSERR_STALE_STATEID) && \ - ((err) != NFSERR_BAD_STATEID) && \ - ((err) != NFSERR_BAD_SEQID) && \ - ((err) != NFSERR_BAD_XDR) && \ - ((err) != NFSERR_RESOURCE) && \ - ((err) != NFSERR_NOFILEHANDLE)) - enum nfs4_client_state { NFS4CLNT_MANAGER_RUNNING = 0, NFS4CLNT_CHECK_LEASE, diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 78c792f..04ad9a2 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -1623,18 +1623,6 @@ static void write_cinfo(__be32 **p, struct nfsd4_change_info *c) \ save = resp->p; -static bool seqid_mutating_err(__be32 err) -{ - /* rfc 3530 section 8.1.5: */ - return err != nfserr_stale_clientid && - err != nfserr_stale_stateid && - err != nfserr_bad_stateid && - err != nfserr_bad_seqid && - err != nfserr_bad_xdr && - err != nfserr_resource && - err != nfserr_nofilehandle; -} - /* * Routine for encoding the result of a "seqid-mutating" NFSv4 operation. This * is where sequence id's are incremented, and the replay cache is filled. @@ -1643,7 +1631,7 @@ static bool seqid_mutating_err(__be32 err) */ #define ENCODE_SEQID_OP_TAIL(stateowner) do { \ - if (seqid_mutating_err(nfserr) && stateowner) { \ + if (seqid_mutating_err(ntohl(nfserr)) && stateowner) { \ stateowner->so_seqid++; \ stateowner->so_replay.rp_status = nfserr; \ stateowner->so_replay.rp_buflen = \ diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index 76f99e8..b875b03 100644 --- a/include/linux/nfs4.h +++ b/include/linux/nfs4.h @@ -373,6 +373,22 @@ enum nfsstat4 { NFS4ERR_DELEG_REVOKED = 10087, /* deleg./layout revoked */ }; +static inline bool seqid_mutating_err(u32 err) +{ + /* rfc 3530 section 8.1.5: */ + switch (err) { + case NFS4ERR_STALE_CLIENTID: + case NFS4ERR_STALE_STATEID: + case NFS4ERR_BAD_STATEID: + case NFS4ERR_BAD_SEQID: + case NFS4ERR_BADXDR: + case NFS4ERR_RESOURCE: + case NFS4ERR_NOFILEHANDLE: + return false; + }; + return true; +} + /* * Note: NF4BAD is not actually part of the protocol; it is just used * internally by nfsd.