miscellaneous nfsd (mainly v4 state) cleanup & bugfixes
Subsystems (2)
This is some cleanup (and smaller bugfixes) done mainly on the way to other stuff.
Mainly just getting rid of cruft:
12 files changed, 273 insertions(+), 385 deletions(-)
--b.
PATCH 1/15 nfsd: remove unused defines
At least one of these is actually wrong anyway.
--- include/linux/nfsd/const.h | 13 ------------- 1 files changed, 0 insertions(+), 13 deletions(-)
include/linux/nfsd/const.h +0 -13
@@ -15,11 +15,6 @@#include <linux/nfs4.h> /* -* Maximum protocol version supported by knfsd -*/ -#define NFSSVC_MAXVERS 3 - -/* * Maximum blocksizes supported by daemon under various circumstances. */ #define NFSSVC_MAXBLKSIZE RPCSVC_MAXPAYLOAD
@@ -42,14 +37,6 @@*/ #define NFSD_BUFSIZE ((RPC_MAX_HEADER_WITH_AUTH+26)*XDR_UNIT + NFSSVC_MAXBLKSIZE) -#ifdef CONFIG_NFSD_V4 -# define NFSSVC_XDRSIZE NFS4_SVC_XDRSIZE -#elif defined(CONFIG_NFSD_V3 ) -# define NFSSVC_XDRSIZE NFS3_SVC_XDRSIZE -#else -# define NFSSVC_XDRSIZE NFS2_SVC_XDRSIZE -#endif -#endif /* __KERNEL__ */ #endif /* _LINUX_NFSD_CONST_H */
Signature · 1 line
1.7.4.1
PATCH 2/15 Remove include/linux/nfsd/const.h
In reply to J. Bruce FieldsUserspace shouldn't have a use for these constants. Nothing here is used outside fs/nfsd.
--- fs/nfsd/nfsd.h | 26 ++++++++++++++++++++++++++ include/linux/nfsd/Kbuild | 1 - include/linux/nfsd/const.h | 42 ------------------------------------------ include/linux/nfsd/nfsfh.h | 7 +++++-- 4 files changed, 31 insertions(+), 45 deletions(-) delete mode 100644 include/linux/nfsd/const.h
fs/nfsd/nfsd.h +26 -0
@@ -11,13 +11,39 @@#include <linux/types.h> #include <linux/mount.h> +#include <linux/nfs.h> +#include <linux/nfs2.h> +#include <linux/nfs3.h> +#include <linux/nfs4.h> +#include <linux/sunrpc/msg_prot.h> +#include <linux/nfsd/debug.h> #include <linux/nfsd/export.h> #include <linux/nfsd/stats.h> +/* * nfsd version */ #define NFSD_SUPPORTED_MINOR_VERSION 1 +/* +* Maximum blocksizes supported by daemon under various circumstances. +*/ +#define NFSSVC_MAXBLKSIZE RPCSVC_MAXPAYLOAD +/* NFSv2 is limited by the protocol specification, see RFC 1094 */ +#define NFSSVC_MAXBLKSIZE_V2 (8*1024) + + +/* +* Largest number of bytes we need to allocate for an NFS +* call or reply. Used to control buffer sizes. We use +* the length of v3 WRITE, READDIR and READDIR replies +* which are an RPC header, up to 26 XDR units of reply +* data, and some page data. +* +* Note that accuracy here doesn't matter too much as the +* size is rounded up to a page size when allocating space. +*/ +#define NFSD_BUFSIZE ((RPC_MAX_HEADER_WITH_AUTH+26)*XDR_UNIT + NFSSVC_MAXBLKSIZE)struct readdir_cd {__be32 err;/* 0, nfserr, or nfserr_eof */
include/linux/nfsd/Kbuild +0 -1
@@ -1,4 +1,3 @@ -header-y += const.h header-y += debug.h header-y += export.h header-y += nfsfh.h
include/linux/nfsd/const.h deleted +0 -42
42 lines not on this page · raw message
include/linux/nfsd/nfsfh.h +5 -2
@@ -14,11 +14,14 @@#ifndef _LINUX_NFSD_FH_H#define _LINUX_NFSD_FH_H -# include <linux/types.h> +#include <linux/types.h> +#include <linux/nfs.h> +#include <linux/nfs2.h> +#include <linux/nfs3.h> +#include <linux/nfs4.h> #ifdef __KERNEL__# include <linux/sunrpc/svc.h> #endif -#include <linux/nfsd/const.h> /* * This is the old "dentry style" Linux NFSv2 file handle.
Signature · 1 line
1.7.4.1
PATCH 3/15 nfsd4: stop using nfserr_resource for transitory errors
In reply to J. Bruce FieldsThe server is returning nfserr_resource for both permanent errors and for errors (like allocation failures) that might be resolved by retrying later. Save nfserr_resource for the former and use delay/jukebox for the latter.
--- fs/nfsd/nfs4proc.c | 2 +- fs/nfsd/nfs4recover.c | 2 +- fs/nfsd/nfs4state.c | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-)
fs/nfsd/nfs4proc.c +1 -1
@@ -938,7 +938,7 @@ _nfsd4_verify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, count =4 + (verify->ve_attrlen >>2 ); buf =kmalloc (count <<2 ,GFP_KERNEL );if (!buf) -return nfserr_resource; +return nfserr_jukebox; status = nfsd4_encode_fattr(&cstate->current_fh, cstate->current_fh.fh_export,
fs/nfsd/nfs4recover.c +1 -1
@@ -88,7 +88,7 @@ nfs4_make_rec_clidname(char *dname, struct xdr_netobj *clname)struct xdr_netobj cksum;struct hash_desc desc;struct scatterlist sg; -__be32 status = nfserr_resource; +__be32 status = nfserr_jukebox;dprintk ("NFSD: nfs4_make_rec_clidname for %.*s\n" , clname->len, clname->data);
fs/nfsd/nfs4state.c +7 -7
@@ -1946,7 +1946,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,* of 5 bullet points, labeled as CASE0 - CASE4 below. */ unconf =find_unconfirmed_client_by_str (dname, strhashval); - status = nfserr_resource; + status = nfserr_jukebox;if (!conf) {/* * RFC 3530 14.2.33 CASE 4:
@@ -2483,7 +2483,7 @@ renew:if (open->op_stateowner ==NULL ) { sop =alloc_init_open_stateowner (strhashval, clp, open);if (sop ==NULL ) -return nfserr_resource; +return nfserr_jukebox; open->op_stateowner = sop; }list_del_init (&sop->so_close_lru);
@@ -2619,7 +2619,7 @@ nfs4_new_open(struct svc_rqst *rqstp, struct nfs4_stateid **stpp, stp =nfs4_alloc_stateid ();if (stp ==NULL ) -return nfserr_resource; +return nfserr_jukebox; status =nfs4_get_vfs_file (rqstp, fp, cur_fh, open);if (status) {
@@ -2850,7 +2850,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf status = nfserr_bad_stateid;if (open->op_claim_type ==NFS4_OPEN_CLAIM_DELEGATE_CUR ) goto out; - status = nfserr_resource; + status = nfserr_jukebox; fp =alloc_init_file (ino);if (fp ==NULL ) goto out;
@@ -4035,7 +4035,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,/* XXX: Do we need to check for duplicate stateowners on * the same file, or should they just be allowed (and * create new stateids)? */ - status = nfserr_resource; + status = nfserr_jukebox; lock_sop =alloc_init_lock_stateowner (strhashval, open_sop->so_client, open_stp, lock);if (lock_sop ==NULL )
@@ -4119,9 +4119,9 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,case (EDEADLK ): status = nfserr_deadlock;break ; -default : +default :dprintk ("NFSD: nfsd4_lock: vfs_lock_file() failed! status %d\n" ,err); - status = nfserr_resource; + status =nfserrno (err);break ; } out:
Signature · 1 line
1.7.4.1
PATCH 4/15 nfsd4: replace some macros by functions
In reply to J. Bruce FieldsFor all the usual reasons. (Type safety, readability.)
--- fs/nfsd/nfs4state.c | 53 ++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 38 insertions(+), 15 deletions(-)
fs/nfsd/nfs4state.c +38 -15
@@ -141,10 +141,19 @@ unsigned int max_delegations;#define OWNER_HASH_SIZE (1 << OWNER_HASH_BITS)#define OWNER_HASH_MASK (OWNER_HASH_SIZE - 1) -#define ownerid_hashval (id) \ - ((id) & OWNER_HASH_MASK) -#define ownerstr_hashval (clientid, ownername) \ - (((clientid) + opaque_hashval((ownername.data), (ownername.len))) & OWNER_HASH_MASK) +static unsigned int ownerid_hashval (const u32 id) +{ +return id &OWNER_HASH_MASK ; +} + +static unsigned int ownerstr_hashval (u32 clientid,struct xdr_netobj *ownername) +{ +unsigned int ret; + + ret =opaque_hashval (ownername->data, ownername->len); + ret += clientid; +return ret &OWNER_HASH_MASK ; +}static struct list_head ownerid_hashtbl[OWNER_HASH_SIZE ];static struct list_head ownerstr_hashtbl[OWNER_HASH_SIZE ];
@@ -158,10 +167,16 @@ static struct list_head ownerstr_hashtbl[OWNER_HASH_SIZE];#define STATEID_HASH_SIZE (1 << STATEID_HASH_BITS)#define STATEID_HASH_MASK (STATEID_HASH_SIZE - 1) -#define file_hashval (x) \ - hash_ptr(x, FILE_HASH_BITS) -#define stateid_hashval (owner_id, file_id) \ - (((owner_id) + (file_id)) & STATEID_HASH_MASK) +static unsigned int file_hashval (struct inode *ino) +{ +/* XXX: why are we hashing on inode pointer, anyway? */ +return hash_ptr (ino,FILE_HASH_BITS ); +} + +static unsigned int stateid_hashval (u32 owner_id,u32 file_id) +{ +return (owner_id + file_id) &STATEID_HASH_MASK ; +}static struct list_head file_hashtbl[FILE_HASH_SIZE ];static struct list_head stateid_hashtbl[STATEID_HASH_SIZE ];
@@ -292,10 +307,16 @@ static DEFINE_SPINLOCK(client_lock);#define CLIENT_HASH_SIZE (1 << CLIENT_HASH_BITS)#define CLIENT_HASH_MASK (CLIENT_HASH_SIZE - 1) -#define clientid_hashval (id) \ - ((id) & CLIENT_HASH_MASK) -#define clientstr_hashval (name) \ - (opaque_hashval((name), 8) & CLIENT_HASH_MASK) +static unsigned int clientid_hashval (u32 id) +{ +return id &CLIENT_HASH_MASK ; +} + +static unsigned int clientstr_hashval (const char *name) +{ +return opaque_hashval (name,8 ) &CLIENT_HASH_MASK ; +} +/* * reclaim_str_hashtbl[] holds known client info from previous reset/reboot * used in reboot/reset lease grace period processing
@@ -2445,7 +2466,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,if (STALE_CLIENTID (&open->op_clientid))return nfserr_stale_clientid; - strhashval =ownerstr_hashval (clientid->cl_id, open->op_owner); + strhashval =ownerstr_hashval (clientid->cl_id, &open->op_owner); sop =find_openstateowner_str (strhashval, open); open->op_stateowner = sop;if (!sop) {
@@ -3713,8 +3734,10 @@ last_byte_offset(u64 start, u64 len)return end > start ? end -1 :NFS4_MAX_UINT64 ; } -#define lockownerid_hashval (id) \ - ((id) & LOCK_HASH_MASK) +static unsigned int lockownerid_hashval (u32 id) +{ +return id &LOCK_HASH_MASK ; +} static inline unsigned int lock_ownerstr_hashval(struct inode *inode, u32 cl_id,
Signature · 1 line
1.7.4.1
PATCH 5/15 nfsd4: name openowner data structures more clearly
In reply to J. Bruce FieldsThese appear to be generic (for both open and lock owners), but they're actually just for open owners. This has confused me more than once.
--- fs/nfsd/nfs4state.c | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-)
fs/nfsd/nfs4state.c +18 -18
@@ -136,27 +136,27 @@ unsigned int max_delegations;* Open owner state (share locks) */ -/* hash tables for nfs4_stateowner */ -#define OWNER_HASH_BITS 8 -#define OWNER_HASH_SIZE (1 << OWNER_HASH_BITS) -#define OWNER_HASH_MASK (OWNER_HASH_SIZE - 1) +/* hash tables for open owners */ +#define OPEN_OWNER_HASH_BITS 8 +#define OPEN_OWNER_HASH_SIZE (1 << OPEN_OWNER_HASH_BITS) +#define OPEN_OWNER_HASH_MASK (OPEN_OWNER_HASH_SIZE - 1) -static unsigned int ownerid_hashval (const u32 id) +static unsigned int open_ownerid_hashval (const u32 id) { -return id &OWNER_HASH_MASK ; +return id &OPEN_OWNER_HASH_MASK ; } -static unsigned int ownerstr_hashval (u32 clientid,struct xdr_netobj *ownername) +static unsigned int open_ownerstr_hashval (u32 clientid,struct xdr_netobj *ownername) {unsigned int ret; ret =opaque_hashval (ownername->data, ownername->len); ret += clientid; -return ret &OWNER_HASH_MASK ; +return ret &OPEN_OWNER_HASH_MASK ; } -static struct list_head ownerid_hashtbl[OWNER_HASH_SIZE ]; -static struct list_head ownerstr_hashtbl[OWNER_HASH_SIZE ]; +static struct list_head open_ownerid_hashtbl[OPEN_OWNER_HASH_SIZE ]; +static struct list_head open_ownerstr_hashtbl[OPEN_OWNER_HASH_SIZE ];/* hash table for nfs4_file */ #define FILE_HASH_BITS 8
@@ -2242,7 +2242,7 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfs4_client *clp, strif (!(sop =alloc_stateowner (&open->op_owner)))return NULL ; - idhashval =ownerid_hashval (current_ownerid); + idhashval =open_ownerid_hashval (current_ownerid);INIT_LIST_HEAD (&sop->so_idhash);INIT_LIST_HEAD (&sop->so_strhash);INIT_LIST_HEAD (&sop->so_perclient);
@@ -2250,8 +2250,8 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfs4_client *clp, strINIT_LIST_HEAD (&sop->so_perstateid);/* not used */ INIT_LIST_HEAD (&sop->so_close_lru); sop->so_time =0 ; -list_add (&sop->so_idhash, &ownerid_hashtbl[idhashval]); -list_add (&sop->so_strhash, &ownerstr_hashtbl[strhashval]); +list_add (&sop->so_idhash, &open_ownerid_hashtbl[idhashval]); +list_add (&sop->so_strhash, &open_ownerstr_hashtbl[strhashval]);list_add (&sop->so_perclient, &clp->cl_openowners); sop->so_is_open_owner =1 ; sop->so_id = current_ownerid++;
@@ -2315,7 +2315,7 @@ find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open) {struct nfs4_stateowner *so =NULL ; -list_for_each_entry (so, &ownerstr_hashtbl[hashval], so_strhash) { +list_for_each_entry (so, &open_ownerstr_hashtbl[hashval], so_strhash) {if (same_owner_str (so, &open->op_owner, &open->op_clientid))return so; }
@@ -2466,7 +2466,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,if (STALE_CLIENTID (&open->op_clientid))return nfserr_stale_clientid; - strhashval =ownerstr_hashval (clientid->cl_id, &open->op_owner); + strhashval =open_ownerstr_hashval (clientid->cl_id, &open->op_owner); sop =find_openstateowner_str (strhashval, open); open->op_stateowner = sop;if (!sop) {
@@ -4520,9 +4520,9 @@ nfs4_state_init(void)for (i =0 ; i <FILE_HASH_SIZE ; i++) {INIT_LIST_HEAD (&file_hashtbl[i]); } -for (i =0 ; i <OWNER_HASH_SIZE ; i++) { -INIT_LIST_HEAD (&ownerstr_hashtbl[i]); -INIT_LIST_HEAD (&ownerid_hashtbl[i]); +for (i =0 ; i <OPEN_OWNER_HASH_SIZE ; i++) { +INIT_LIST_HEAD (&open_ownerstr_hashtbl[i]); +INIT_LIST_HEAD (&open_ownerid_hashtbl[i]); }for (i =0 ; i <STATEID_HASH_SIZE ; i++) {INIT_LIST_HEAD (&stateid_hashtbl[i]);
Signature · 1 line
1.7.4.1
PATCH 6/15 nfsd4: cleanup lock/stateowner initialization
In reply to J. Bruce FieldsShare some common code, stop doing silly things like initializing a list head immediately before adding it to a list, etc.
--- fs/nfsd/nfs4state.c | 100 ++++++++++++++++++++++++++------------------------ 1 files changed, 52 insertions(+), 48 deletions(-)
fs/nfsd/nfs4state.c +52 -48
@@ -2217,51 +2217,61 @@ nfs4_free_stateowner(struct kref *kref)kmem_cache_free (stateowner_slab, sop); } -static inline struct nfs4_stateowner * -alloc_stateowner (struct xdr_netobj *owner) +static void init_nfs4_replay (struct nfs4_replay *rp) { -struct nfs4_stateowner *sop; - -if ((sop =kmem_cache_alloc (stateowner_slab,GFP_KERNEL ))) { -if ((sop->so_owner.data =kmalloc (owner->len,GFP_KERNEL ))) { -memcpy (sop->so_owner.data, owner->data, owner->len); - sop->so_owner.len = owner->len; -kref_init (&sop->so_ref); -return sop; - } -kmem_cache_free (stateowner_slab, sop); - } -return NULL ; + rp->rp_status = nfserr_serverfault; + rp->rp_buflen =0 ; + rp->rp_buf = rp->rp_ibuf; } -static struct nfs4_stateowner * -alloc_init_open_stateowner (unsigned int strhashval,struct nfs4_client *clp,struct nfsd4_open *open) { +static inline struct nfs4_stateowner *alloc_stateowner (struct xdr_netobj *owner,struct nfs4_client *clp) +{struct nfs4_stateowner *sop; -struct nfs4_replay *rp; -unsigned int idhashval; -if (!(sop =alloc_stateowner (&open->op_owner))) + sop =kmem_cache_alloc (stateowner_slab,GFP_KERNEL ); +if (!sop) +return NULL ; + + sop->so_owner.data =kmemdup (owner->data, owner->len,GFP_KERNEL ); +if (!sop->so_owner.data) { +kmem_cache_free (stateowner_slab, sop);return NULL ; - idhashval =open_ownerid_hashval (current_ownerid); -INIT_LIST_HEAD (&sop->so_idhash); -INIT_LIST_HEAD (&sop->so_strhash); + } + sop->so_owner.len = owner->len; + +kref_init (&sop->so_ref);INIT_LIST_HEAD (&sop->so_perclient);INIT_LIST_HEAD (&sop->so_stateids); -INIT_LIST_HEAD (&sop->so_perstateid);/* not used */ +INIT_LIST_HEAD (&sop->so_perstateid);INIT_LIST_HEAD (&sop->so_close_lru); + sop->so_id = current_ownerid++; sop->so_time =0 ; + sop->so_client = clp; +init_nfs4_replay (&sop->so_replay); +return sop; +} + +static void hash_openowner (struct nfs4_stateowner *sop,struct nfs4_client *clp,unsigned int strhashval) +{ +unsigned int idhashval; + + idhashval =open_ownerid_hashval (sop->so_id);list_add (&sop->so_idhash, &open_ownerid_hashtbl[idhashval]);list_add (&sop->so_strhash, &open_ownerstr_hashtbl[strhashval]);list_add (&sop->so_perclient, &clp->cl_openowners); +} + +static struct nfs4_stateowner * +alloc_init_open_stateowner (unsigned int strhashval,struct nfs4_client *clp,struct nfsd4_open *open) { +struct nfs4_stateowner *sop; + + sop =alloc_stateowner (&open->op_owner, clp); +if (!sop) +return NULL ; sop->so_is_open_owner =1 ; - sop->so_id = current_ownerid++; - sop->so_client = clp; sop->so_seqid = open->op_seqid; sop->so_confirmed =0 ; - rp = &sop->so_replay; - rp->rp_status = nfserr_serverfault; - rp->rp_buflen =0 ; - rp->rp_buf = rp->rp_ibuf; +hash_openowner (sop, clp, strhashval);return sop; }
@@ -3904,6 +3914,16 @@ find_lockstateowner_str(struct inode *inode, clientid_t *clid,return NULL ; } +static void hash_lockowner (struct nfs4_stateowner *sop,unsigned int strhashval,struct nfs4_client *clp,struct nfs4_stateid *open_stp) +{ +unsigned int idhashval; + + idhashval =lockownerid_hashval (sop->so_id); +list_add (&sop->so_idhash, &lock_ownerid_hashtbl[idhashval]); +list_add (&sop->so_strhash, &lock_ownerstr_hashtbl[strhashval]); +list_add (&sop->so_perstateid, &open_stp->st_lockowners); +} +/* * Alloc a lock owner structure. * Called in nfsd4_lock - therefore, OPEN and OPEN_CONFIRM (if needed) has
@@ -3915,33 +3935,17 @@ find_lockstateowner_str(struct inode *inode, clientid_t *clid,static struct nfs4_stateowner *alloc_init_lock_stateowner (unsigned int strhashval,struct nfs4_client *clp,struct nfs4_stateid *open_stp,struct nfsd4_lock *lock) {struct nfs4_stateowner *sop; -struct nfs4_replay *rp; -unsigned int idhashval; -if (!(sop =alloc_stateowner (&lock->lk_new_owner))) + sop =alloc_stateowner (&lock->lk_new_owner, clp); +if (!sop)return NULL ; - idhashval =lockownerid_hashval (current_ownerid); -INIT_LIST_HEAD (&sop->so_idhash); -INIT_LIST_HEAD (&sop->so_strhash); -INIT_LIST_HEAD (&sop->so_perclient);INIT_LIST_HEAD (&sop->so_stateids); -INIT_LIST_HEAD (&sop->so_perstateid); -INIT_LIST_HEAD (&sop->so_close_lru);/* not used */ - sop->so_time =0 ; -list_add (&sop->so_idhash, &lock_ownerid_hashtbl[idhashval]); -list_add (&sop->so_strhash, &lock_ownerstr_hashtbl[strhashval]); -list_add (&sop->so_perstateid, &open_stp->st_lockowners); sop->so_is_open_owner =0 ; - sop->so_id = current_ownerid++; - sop->so_client = clp;/* It is the openowner seqid that will be incremented in encode in the * case of new lockowners; so increment the lock seqid manually: */ sop->so_seqid = lock->lk_new_lock_seqid +1 ; sop->so_confirmed =1 ; - rp = &sop->so_replay; - rp->rp_status = nfserr_serverfault; - rp->rp_buflen =0 ; - rp->rp_buf = rp->rp_ibuf; +hash_lockowner (sop, strhashval, clp, open_stp);return sop; }
Signature · 1 line
1.7.4.1
PATCH 7/15 nfsd4: remove HAS_SESSION
In reply to J. Bruce FieldsThis flag doesn't really buy us anything.
--- fs/nfsd/nfs4state.c | 30 ++++++++++-------------------- fs/nfsd/state.h | 3 +-- fs/nfsd/xdr4.h | 2 +- 3 files changed, 12 insertions(+), 23 deletions(-)
fs/nfsd/nfs4state.c +10 -20
@@ -3170,13 +3170,13 @@ grace_disallows_io(struct inode *inode)return locks_in_grace () &&mandatory_lock (inode); } -static int check_stateid_generation (stateid_t *in,stateid_t *ref,int flags) +static int check_stateid_generation (stateid_t *in,stateid_t *ref,bool has_session) {/* * When sessions are used the stateid generation number is ignored * when it is zero. */ -if ((flags &HAS_SESSION ) && in->si_generation ==0 ) +if (has_session && in->si_generation ==0 ) goto out;/* If the client sends us a stateid from the future, it's buggy: */
@@ -3208,7 +3208,7 @@ static int is_open_stateid(struct nfs4_stateid *stateid)return stateid->st_openstp ==NULL ; } -__be32 nfs4_validate_stateid (stateid_t *stateid,int flags) +__be32 nfs4_validate_stateid (stateid_t *stateid,bool has_session) {struct nfs4_stateid *stp =NULL ;__be32 status = nfserr_stale_stateid;
@@ -3225,7 +3225,7 @@ __be32 nfs4_validate_stateid(stateid_t *stateid, int flags)if (!stp->st_stateowner->so_confirmed) goto out; - status =check_stateid_generation (stateid, &stp->st_stateid, flags); + status =check_stateid_generation (stateid, &stp->st_stateid, has_session);if (status) goto out;
@@ -3253,9 +3253,6 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,if (grace_disallows_io (ino))return nfserr_grace; -if (nfsd4_has_session (cstate)) - flags |=HAS_SESSION ; -if (ZERO_STATEID (stateid) ||ONE_STATEID (stateid))return check_special_stateids (current_fh, stateid, flags);
@@ -3272,8 +3269,7 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate, dp =find_delegation_stateid (ino, stateid);if (!dp) goto out; - status =check_stateid_generation (stateid, &dp->dl_stateid, - flags); + status =check_stateid_generation (stateid, &dp->dl_stateid,nfsd4_has_session (cstate));if (status) goto out; status =nfs4_check_delegmode (dp, flags);
@@ -3294,7 +3290,7 @@ nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,if (!stp->st_stateowner->so_confirmed) goto out; status =check_stateid_generation (stateid, &stp->st_stateid, - flags); +nfsd4_has_session (cstate));if (status) goto out; status =nfs4_check_openmode (stp, flags);
@@ -3423,9 +3419,6 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,if (STALE_STATEID (stateid))return nfserr_stale_stateid; -if (nfsd4_has_session (cstate)) - flags |=HAS_SESSION ; -/* * We return BAD_STATEID if filehandle doesn't match stateid, * the confirmed flag is incorrecly set, or the generation
@@ -3459,7 +3452,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,if (lock->lk_is_new) {if (!sop->so_is_open_owner)return nfserr_bad_stateid; -if (!(flags &HAS_SESSION ) && +if (!nfsd4_has_session (cstate) && !same_clid (&clp->cl_clientid, lockclid))return nfserr_bad_stateid;/* stp is the open stateid */
@@ -3484,7 +3477,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,* For the moment, we ignore the possibility of * generation number wraparound. */ -if (!(flags &HAS_SESSION ) && seqid != sop->so_seqid) +if (!nfsd4_has_session (cstate) && seqid != sop->so_seqid) goto check_replay;if (sop->so_confirmed && flags &CONFIRM ) {
@@ -3497,7 +3490,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, " confirmed yet!\n");return nfserr_bad_stateid; } - status =check_stateid_generation (stateid, &stp->st_stateid, flags); + status =check_stateid_generation (stateid, &stp->st_stateid,nfsd4_has_session (cstate));if (status)return status;renew_client (sop->so_client);
@@ -3681,14 +3674,11 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,stateid_t *stateid = &dr->dr_stateid;struct inode *inode;__be32 status; -int flags =0 ;if ((status =fh_verify (rqstp, &cstate->current_fh,S_IFREG ,0 )))return status; inode = cstate->current_fh.fh_dentry->d_inode; -if (nfsd4_has_session (cstate)) - flags |=HAS_SESSION ;nfs4_lock_state (); status = nfserr_bad_stateid;if (ZERO_STATEID (stateid) ||ONE_STATEID (stateid))
@@ -3703,7 +3693,7 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, dp =find_delegation_stateid (inode, stateid);if (!dp) goto out; - status =check_stateid_generation (stateid, &dp->dl_stateid, flags); + status =check_stateid_generation (stateid, &dp->dl_stateid,nfsd4_has_session (cstate));if (status) goto out;renew_client (dp->dl_client);
fs/nfsd/state.h +1 -2
@@ -439,7 +439,6 @@ struct nfs4_stateid { };/* flags for preprocess_seqid_op() */ -#define HAS_SESSION 0x00000001#define CONFIRM 0x00000002#define OPEN_STATE 0x00000004#define LOCK_STATE 0x00000008
@@ -476,7 +475,7 @@ extern void nfsd4_recdir_purge_old(void);extern int nfsd4_create_clid_dir (struct nfs4_client *clp);extern void nfsd4_remove_clid_dir (struct nfs4_client *clp);extern void release_session_client (struct nfsd4_session *); -extern __be32 nfs4_validate_stateid (stateid_t *,int ); +extern __be32 nfs4_validate_stateid (stateid_t *,bool );static inline void nfs4_put_stateowner (struct nfs4_stateowner *so)
fs/nfsd/xdr4.h +1 -1
@@ -351,7 +351,7 @@ struct nfsd4_saved_compoundargs {struct nfsd4_test_stateid {__be32 ts_num_ids; -__be32 ts_has_session; +bool ts_has_session;struct nfsd4_compoundargs *ts_saved_args;struct nfsd4_saved_compoundargs ts_savedp; };
Signature · 1 line
1.7.4.1
PATCH 8/15 nfsd4: cleanup and consolidate seqid_mutating_err
In reply to J. Bruce Fields--- fs/nfs/nfs4_fs.h | 24 ------------------------ fs/nfsd/nfs4xdr.c | 14 +------------- include/linux/nfs4.h | 16 ++++++++++++++++ 3 files changed, 17 insertions(+), 37 deletions(-)
fs/nfs/nfs4_fs.h +0 -24
@@ -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 ,
fs/nfsd/nfs4xdr.c +1 -13
@@ -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 = \
include/linux/nfs4.h +16 -0
@@ -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.
Signature · 1 line
1.7.4.1
PATCH 9/15 nfsd4: simplify lock openmode check
In reply to J. Bruce FieldsNote that the special handling for the lock stateid case is already done by nfs4_check_openmode() (as of 02921914170e3b7fea1cd82dac9713685d2de5e2 "nfsd4: fix openmode checking on IO using lock stateid") so we no longer need these two cases in the caller.
--- fs/nfsd/nfs4state.c | 15 +++++---------- 1 files changed, 5 insertions(+), 10 deletions(-)
fs/nfsd/nfs4state.c +5 -10
@@ -3455,16 +3455,11 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,if (!nfsd4_has_session (cstate) && !same_clid (&clp->cl_clientid, lockclid))return nfserr_bad_stateid; -/* stp is the open stateid */ - status =nfs4_check_openmode (stp, lkflg); -if (status) -return status; - }else { -/* stp is the lock stateid */ - status =nfs4_check_openmode (stp->st_openstp, lkflg); -if (status) -return status; - } + } +/* stp is the open stateid */ + status =nfs4_check_openmode (stp, lkflg); +if (status) +return status; }if (nfs4_check_fh (current_fh, stp)) {
Signature · 1 line
1.7.4.1
PATCH 10/15 nfsd4: get lock checks out of preprocess_seqid_op
In reply to J. Bruce FieldsWe've got some lock-specific code here in nfs4_preprocess_seqid_op which is only used by nfsd4_lock(). Move it to the caller.
--- fs/nfsd/nfs4state.c | 49 ++++++++++++++++++++----------------------------- 1 files changed, 20 insertions(+), 29 deletions(-)
fs/nfsd/nfs4state.c +20 -29
@@ -3398,7 +3398,7 @@ static __be32 nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, stateid_t *stateid, int flags, struct nfs4_stateowner **sopp, - struct nfs4_stateid **stpp, struct nfsd4_lock *lock) + struct nfs4_stateid **stpp) { struct nfs4_stateid *stp;struct nfs4_stateowner *sop;
@@ -3441,27 +3441,6 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, *stpp = stp; *sopp = sop = stp->st_stateowner; -if (lock) { -clientid_t *lockclid = &lock->v.new.clientid; -struct nfs4_client *clp = sop->so_client; -int lkflg =0 ; -__be32 status; - - lkflg =setlkflg (lock->lk_type); - -if (lock->lk_is_new) { -if (!sop->so_is_open_owner) -return nfserr_bad_stateid; -if (!nfsd4_has_session (cstate) && - !same_clid (&clp->cl_clientid, lockclid)) -return nfserr_bad_stateid; - } -/* stp is the open stateid */ - status =nfs4_check_openmode (stp, lkflg); -if (status) -return status; - } -if (nfs4_check_fh (current_fh, stp)) {dprintk ("NFSD: preprocess_seqid_op: fh-stateid mismatch!\n" );return nfserr_bad_stateid;
@@ -3524,7 +3503,7 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,if ((status =nfs4_preprocess_seqid_op (cstate, oc->oc_seqid, &oc->oc_req_stateid,CONFIRM |OPEN_STATE , - &oc->oc_stateowner, &stp,NULL ))) + &oc->oc_stateowner, &stp))) goto out; sop = oc->oc_stateowner;
@@ -3587,7 +3566,7 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp, od->od_seqid, &od->od_stateid,OPEN_STATE , - &od->od_stateowner, &stp, NULL))) + &od->od_stateowner, &stp))) goto out; status = nfserr_inval;
@@ -3637,7 +3616,7 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, close->cl_seqid, &close->cl_stateid,OPEN_STATE |CLOSE_STATE , - &close->cl_stateowner, &stp, NULL))) + &close->cl_stateowner, &stp))) goto out; status = nfs_ok;update_stateid (&stp->st_stateid);
@@ -3999,6 +3978,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,struct file_lock conflock;__be32 status =0 ;unsigned int strhashval; +int lkflg;int err; dprintk("NFSD: nfsd4_lock: start=%Ld length=%Ld\n",
@@ -4034,11 +4014,17 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, lock->lk_new_open_seqid, &lock->lk_new_open_stateid,OPEN_STATE , - &lock->lk_replay_owner, &open_stp, - lock); + &lock->lk_replay_owner, &open_stp);if (status) goto out; + status = nfserr_bad_stateid; open_sop = lock->lk_replay_owner; +if (!open_sop->so_is_open_owner) + goto out; +if (!nfsd4_has_session (cstate) && + !same_clid (&open_sop->so_client->cl_clientid, + &lock->v.new.clientid)) + goto out;/* create lockowner and lock stateid */ fp = open_stp->st_file; strhashval = lock_ownerstr_hashval(fp->fi_inode,
@@ -4061,7 +4047,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, lock->lk_old_lock_seqid, &lock->lk_old_lock_stateid,LOCK_STATE , - &lock->lk_replay_owner, &lock_stp, lock); + &lock->lk_replay_owner, &lock_stp);if (status) goto out; lock_sop = lock->lk_replay_owner;
@@ -4069,6 +4055,11 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, }/* lock->lk_replay_owner and lock_stp have been created or found */ + lkflg =setlkflg (lock->lk_type); + status =nfs4_check_openmode (lock_stp, lkflg); +if (status) + goto out; + status = nfserr_grace;if (locks_in_grace () && !lock->lk_reclaim) goto out;
@@ -4261,7 +4252,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, locku->lu_seqid, &locku->lu_stateid,LOCK_STATE , - &locku->lu_stateowner, &stp, NULL))) + &locku->lu_stateowner, &stp))) goto out; filp =find_any_file (stp->st_file);
Signature · 1 line
1.7.4.1
PATCH 11/15 nfsd4: remove redundant is_open_owner check
In reply to J. Bruce FieldsWhen called with OPEN_STATE, preprocess_seqid_op only returns an open stateid, hence only an open owner.
--- fs/nfsd/nfs4state.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +0 -2
@@ -4019,8 +4019,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, goto out; status = nfserr_bad_stateid; open_sop = lock->lk_replay_owner; -if (!open_sop->so_is_open_owner) - goto out;if (!nfsd4_has_session (cstate) && !same_clid (&open_sop->so_client->cl_clientid, &lock->v.new.clientid))
Signature · 1 line
1.7.4.1
PATCH 12/15 nfsd4: consolidate lock & open stateid tables
In reply to J. Bruce FieldsThere's no reason to have two separate hash tables for open and lock stateid's.
--- fs/nfsd/nfs4state.c | 65 +++++++++++++------------------------------------- fs/nfsd/state.h | 3 ++ 2 files changed, 20 insertions(+), 48 deletions(-)
fs/nfsd/nfs4state.c +17 -48
@@ -61,7 +61,6 @@ static u64 current_sessionid = 1;/* forward declarations */ static struct nfs4_stateid *find_stateid (stateid_t *stid,int flags); -static struct nfs4_stateid *search_for_stateid (stateid_t *stid);static struct nfs4_delegation *search_for_delegation (stateid_t *stid);static struct nfs4_delegation *find_delegation_stateid (struct inode *ino,stateid_t *stid);static char user_recovery_dirname[PATH_MAX ] ="/var/lib/nfs/v4recovery" ;
@@ -2287,6 +2286,7 @@ init_stateid(struct nfs4_stateid *stp, struct nfs4_file *fp, struct nfsd4_open *list_add (&stp->st_hash, &stateid_hashtbl[hashval]);list_add (&stp->st_perstateowner, &sop->so_stateids);list_add (&stp->st_perfile, &fp->fi_stateids); + stp->st_type =NFS4_OPEN_STID ; stp->st_stateowner = sop;get_nfs4_file (fp); stp->st_file = fp;
@@ -3217,7 +3217,7 @@ __be32 nfs4_validate_stateid(stateid_t *stateid, bool has_session) goto out; status = nfserr_expired; - stp =search_for_stateid (stateid); + stp =find_stateid (stateid,0 );if (!stp) goto out; status = nfserr_bad_stateid;
@@ -3355,7 +3355,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, goto out; } - stp =search_for_stateid (stateid); + stp =find_stateid (stateid,0 );if (!stp) { ret = nfserr_bad_stateid; goto out;
@@ -3724,7 +3724,6 @@ lock_ownerstr_hashval(struct inode *inode, u32 cl_id,static struct list_head lock_ownerid_hashtbl[LOCK_HASH_SIZE ];static struct list_head lock_ownerstr_hashtbl[LOCK_HASH_SIZE ]; -static struct list_head lockstateid_hashtbl[STATEID_HASH_SIZE ];static int same_stateid (stateid_t *id_one,stateid_t *id_two)
@@ -3734,50 +3733,21 @@ same_stateid(stateid_t *id_one, stateid_t *id_two)return id_one->si_fileid == id_two->si_fileid; } -static struct nfs4_stateid * -find_stateid (stateid_t *stid,int flags) +static struct nfs4_stateid *find_stateid (stateid_t *t,int flags) { -struct nfs4_stateid *local; -u32 st_id = stid->si_stateownerid; -u32 f_id = stid->si_fileid; +struct nfs4_stateid *s;unsigned int hashval; -dprintk ("NFSD: find_stateid flags 0x%x\n" ,flags); -if (flags & (LOCK_STATE |RD_STATE |WR_STATE )) { - hashval =stateid_hashval (st_id, f_id); -list_for_each_entry (local, &lockstateid_hashtbl[hashval], st_hash) { -if ((local->st_stateid.si_stateownerid == st_id) && - (local->st_stateid.si_fileid == f_id)) -return local; - } - } - -if (flags & (OPEN_STATE |RD_STATE |WR_STATE )) { - hashval =stateid_hashval (st_id, f_id); -list_for_each_entry (local, &stateid_hashtbl[hashval], st_hash) { -if ((local->st_stateid.si_stateownerid == st_id) && - (local->st_stateid.si_fileid == f_id)) -return local; + hashval =stateid_hashval (t->si_stateownerid, t->si_fileid); +list_for_each_entry (s, &stateid_hashtbl[hashval], st_hash) { +if (!same_stateid (&s->st_stateid, t)) +continue ; +if (flags &LOCK_STATE && s->st_type !=NFS4_LOCK_STID ) +return NULL ; +if (flags &OPEN_STATE && s->st_type !=NFS4_OPEN_STID ) +return NULL ; +return s; } - } -return NULL ; -} - -static struct nfs4_stateid * -search_for_stateid (stateid_t *stid) -{ -struct nfs4_stateid *local; -unsigned int hashval =stateid_hashval (stid->si_stateownerid, stid->si_fileid); - -list_for_each_entry (local, &lockstateid_hashtbl[hashval], st_hash) { -if (same_stateid (&local->st_stateid, stid)) -return local; - } - -list_for_each_entry (local, &stateid_hashtbl[hashval], st_hash) { -if (same_stateid (&local->st_stateid, stid)) -return local; - }return NULL ; }
@@ -3926,10 +3896,11 @@ alloc_init_lock_stateid(struct nfs4_stateowner *sop, struct nfs4_file *fp, strucINIT_LIST_HEAD (&stp->st_perfile);INIT_LIST_HEAD (&stp->st_perstateowner);INIT_LIST_HEAD (&stp->st_lockowners);/* not used */ -list_add (&stp->st_hash, &lockstateid_hashtbl[hashval]); +list_add (&stp->st_hash, &stateid_hashtbl[hashval]);list_add (&stp->st_perfile, &fp->fi_stateids);list_add (&stp->st_perstateowner, &sop->so_stateids); stp->st_stateowner = sop; + stp->st_type =NFS4_LOCK_STID ;get_nfs4_file (fp); stp->st_file = fp; stp->st_stateid.si_boot = boot_time;
@@ -4502,10 +4473,8 @@ nfs4_state_init(void)INIT_LIST_HEAD (&open_ownerstr_hashtbl[i]);INIT_LIST_HEAD (&open_ownerid_hashtbl[i]); } -for (i =0 ; i <STATEID_HASH_SIZE ; i++) { +for (i =0 ; i <STATEID_HASH_SIZE ; i++)INIT_LIST_HEAD (&stateid_hashtbl[i]); -INIT_LIST_HEAD (&lockstateid_hashtbl[i]); - }for (i =0 ; i <LOCK_HASH_SIZE ; i++) {INIT_LIST_HEAD (&lock_ownerid_hashtbl[i]);INIT_LIST_HEAD (&lock_ownerstr_hashtbl[i]);
fs/nfsd/state.h +3 -0
@@ -426,6 +426,9 @@ static inline struct file *find_any_file(struct nfs4_file *f)*/ struct nfs4_stateid { +#define NFS4_OPEN_STID 1 +#define NFS4_LOCK_STID 2 +char st_type;struct list_head st_hash;struct list_head st_perfile;struct list_head st_perstateowner;
Signature · 1 line
1.7.4.1
PATCH 13/15 nfsd4: simplify stateid generation code, fix wraparound
In reply to J. Bruce FieldsFollow the recommendation from rfc3530bis for stateid generation number wraparound, simplify some code, and fix or remove incorrect comments.
--- fs/nfsd/nfs4state.c | 52 ++++++++++++++++++++++---------------------------- fs/nfsd/state.h | 3 ++ 2 files changed, 26 insertions(+), 29 deletions(-)
fs/nfsd/nfs4state.c +23 -29
@@ -3170,6 +3170,12 @@ grace_disallows_io(struct inode *inode)return locks_in_grace () &&mandatory_lock (inode); } +/* Returns true iff a is later than b: */ +static bool stateid_generation_after (stateid_t *a,stateid_t *b) +{ +return (s32 )a->si_generation - (s32 )b->si_generation >0 ; +} +static int check_stateid_generation (stateid_t *in,stateid_t *ref,bool has_session) {/*
@@ -3177,25 +3183,25 @@ static int check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_sess* when it is zero. */ if (has_session && in->si_generation ==0 ) - goto out; +return nfs_ok; + +if (in->si_generation == ref->si_generation) +return nfs_ok;/* If the client sends us a stateid from the future, it's buggy: */ -if (in->si_generation > ref->si_generation) +if (stateid_generation_after (in, ref))return nfserr_bad_stateid;/* -* The following, however, can happen. For example, if the -* client sends an open and some IO at the same time, the open -* may bump si_generation while the IO is still in flight. -* Thanks to hard links and renames, the client never knows what -* file an open will affect. So it could avoid that situation -* only by serializing all opens and IO from the same open -* owner. To recover from the old_stateid error, the client -* will just have to retry the IO: +* However, we could see a stateid from the past, even from a +* non-buggy client. For example, if the client sends a lock +* while some IO is outstanding, the lock may bump si_generation +* while the IO is still in flight. The client could avoid that +* situation by waiting for responses on all the IO requests, +* but better performance may result in retrying IO that +* receives an old_stateid error if requests are rarely +* reordered in flight: */ -if (in->si_generation < ref->si_generation) -return nfserr_old_stateid; -out: -return nfs_ok; +return nfserr_old_stateid; }static int is_delegation_stateid (stateid_t *stateid)
@@ -3360,16 +3366,9 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, ret = nfserr_bad_stateid; goto out; } -if (stateid->si_generation !=0 ) { -if (stateid->si_generation < stp->st_stateid.si_generation) { - ret = nfserr_old_stateid; - goto out; - } -if (stateid->si_generation > stp->st_stateid.si_generation) { - ret = nfserr_bad_stateid; - goto out; - } - } + ret =check_stateid_generation (stateid, &stp->st_stateid,1 ); +if (ret) + goto out;if (is_open_stateid (stp)) { ret = nfserr_locks_held;
@@ -3446,11 +3445,6 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,return nfserr_bad_stateid; } -/* -* We now validate the seqid and stateid generation numbers. -* For the moment, we ignore the possibility of -* generation number wraparound. -*/ if (!nfsd4_has_session (cstate) && seqid != sop->so_seqid) goto check_replay;
fs/nfsd/state.h +3 -0
@@ -293,6 +293,9 @@ static inline void update_stateid(stateid_t *stateid) { stateid->si_generation++; +/* Wraparound recommendation from 3530bis-13 9.1.3.2: */ +if (stateid->si_generation ==0 ) + stateid->si_generation =1 ; }/* A reasonable value for REPLAY_ISIZE was estimated as follows:
Signature · 1 line
1.7.4.1
PATCH 14/15 nfsd4: centralize handling of replay owners
In reply to J. Bruce FieldsSet the stateowner associated with a replay in one spot in nfs4_preprocess_seqid_op() and keep it in cstate. This allows removing a few lines of boilerplate from all the nfs4_preprocess_seqid_op() callers.
Also turn ENCODE_SEQID_OP_TAIL into a function while we're here.
--- fs/nfsd/nfs4state.c | 25 ++++--------------------- fs/nfsd/nfs4xdr.c | 34 +++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 36 deletions(-)
fs/nfsd/nfs4state.c +4 -21
@@ -3433,12 +3433,15 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,/* It's not stale; let's assume it's expired: */ if (sop ==NULL )return nfserr_expired; - *sopp = sop; +nfs4_get_stateowner (sop); + cstate->replay_owner = sop; goto check_replay; } *stpp = stp; *sopp = sop = stp->st_stateowner; +nfs4_get_stateowner (sop); + cstate->replay_owner = sop;if (nfs4_check_fh (current_fh, stp)) {dprintk ("NFSD: preprocess_seqid_op: fh-stateid mismatch!\n" );
@@ -3509,10 +3512,6 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,nfsd4_create_clid_dir (sop->so_client); out: -if (oc->oc_stateowner) { -nfs4_get_stateowner (oc->oc_stateowner); - cstate->replay_owner = oc->oc_stateowner; - }nfs4_unlock_state ();return status; }
@@ -3582,10 +3581,6 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,memcpy (&od->od_stateid, &stp->st_stateid,sizeof (stateid_t)); status = nfs_ok; out: -if (od->od_stateowner) { -nfs4_get_stateowner (od->od_stateowner); - cstate->replay_owner = od->od_stateowner; - }nfs4_unlock_state ();return status; }
@@ -3626,10 +3621,6 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,if (list_empty (&close->cl_stateowner->so_stateids))move_to_close_lru (close->cl_stateowner); out: -if (close->cl_stateowner) { -nfs4_get_stateowner (close->cl_stateowner); - cstate->replay_owner = close->cl_stateowner; - }nfs4_unlock_state ();return status; }
@@ -4093,10 +4084,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, out:if (status && lock->lk_is_new && lock_sop)release_lockowner (lock_sop); -if (lock->lk_replay_owner) { -nfs4_get_stateowner (lock->lk_replay_owner); - cstate->replay_owner = lock->lk_replay_owner; - }nfs4_unlock_state ();return status; }
@@ -4251,10 +4238,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,memcpy (&locku->lu_stateid, &stp->st_stateid,sizeof (stateid_t)); out: -if (locku->lu_stateowner) { -nfs4_get_stateowner (locku->lu_stateowner); - cstate->replay_owner = locku->lu_stateowner; - }nfs4_unlock_state ();return status;
fs/nfsd/nfs4xdr.c +19 -15
@@ -1630,15 +1630,19 @@ static void write_cinfo(__be32 **p, struct nfsd4_change_info *c)* we know whether the error to be returned is a sequence id mutating error. */ -#define ENCODE_SEQID_OP_TAIL (stateowner) do { \ - if (seqid_mutating_err(ntohl(nfserr)) && stateowner) { \ - stateowner->so_seqid++; \ - stateowner->so_replay.rp_status = nfserr; \ - stateowner->so_replay.rp_buflen = \ - (((char *)(resp)->p - (char *)save)); \ - memcpy(stateowner->so_replay.rp_buf, save, \ - stateowner->so_replay.rp_buflen); \ - } } while (0); +static void encode_seqid_op_tail (struct nfsd4_compoundres *resp,__be32 *save,__be32 nfserr) +{ +struct nfs4_stateowner *stateowner = resp->cstate.replay_owner; + +if (seqid_mutating_err (ntohl (nfserr)) && stateowner) { + stateowner->so_seqid++; + stateowner->so_replay.rp_status = nfserr; + stateowner->so_replay.rp_buflen = + (char *)resp->p - (char *)save; +memcpy (stateowner->so_replay.rp_buf, save, + stateowner->so_replay.rp_buflen); + } +}/* Encode as an array of strings the string given with components * separated @sep.
@@ -2495,7 +2499,7 @@ nfsd4_encode_close(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_cif (!nfserr)nfsd4_encode_stateid (resp, &close->cl_stateid); -ENCODE_SEQID_OP_TAIL (close->cl_stateowner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
@@ -2599,7 +2603,7 @@ nfsd4_encode_lock(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_loelse if (nfserr == nfserr_denied)nfsd4_encode_lock_denied (resp, &lock->lk_denied); -ENCODE_SEQID_OP_TAIL (lock->lk_replay_owner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
@@ -2619,7 +2623,7 @@ nfsd4_encode_locku(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_lif (!nfserr)nfsd4_encode_stateid (resp, &locku->lu_stateid); -ENCODE_SEQID_OP_TAIL (locku->lu_stateowner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
@@ -2700,7 +2704,7 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_op }/* XXX save filehandle here */ out: -ENCODE_SEQID_OP_TAIL (open->op_stateowner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
@@ -2712,7 +2716,7 @@ nfsd4_encode_open_confirm(struct nfsd4_compoundres *resp, __be32 nfserr, structif (!nfserr)nfsd4_encode_stateid (resp, &oc->oc_resp_stateid); -ENCODE_SEQID_OP_TAIL (oc->oc_stateowner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
@@ -2724,7 +2728,7 @@ nfsd4_encode_open_downgrade(struct nfsd4_compoundres *resp, __be32 nfserr, strucif (!nfserr)nfsd4_encode_stateid (resp, &od->od_stateid); -ENCODE_SEQID_OP_TAIL (od->od_stateowner); +encode_seqid_op_tail (resp, save, nfserr);return nfserr; }
Signature · 1 line
1.7.4.1
PATCH 15/15 nfsd4: cleanup seqid op stateowner usage
In reply to J. Bruce FieldsNow that the replay owner is in the cstate we can remove it from a lot of other individual operations and further simplify nfs4_preprocess_seqid_op().
--- fs/nfsd/nfs4state.c | 57 +++++++++++++++++++++----------------------------- fs/nfsd/nfs4xdr.c | 5 ---- fs/nfsd/xdr4.h | 7 ------ 3 files changed, 24 insertions(+), 45 deletions(-)
fs/nfsd/nfs4state.c +24 -33
@@ -3396,7 +3396,6 @@ setlkflg (int type)static __be32 nfs4_preprocess_seqid_op (struct nfsd4_compound_state *cstate,u32 seqid,stateid_t *stateid,int flags, -struct nfs4_stateowner **sopp,struct nfs4_stateid **stpp) {struct nfs4_stateid *stp;
@@ -3408,7 +3407,6 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, seqid, STATEID_VAL(stateid)); *stpp =NULL ; - *sopp =NULL ;if (ZERO_STATEID (stateid) ||ONE_STATEID (stateid)) {dprintk ("NFSD: preprocess_seqid_op: magic stateid!\n" );
@@ -3439,7 +3437,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, } *stpp = stp; - *sopp = sop = stp->st_stateowner; + sop = stp->st_stateowner;nfs4_get_stateowner (sop); cstate->replay_owner = sop;
@@ -3475,7 +3473,6 @@ check_replay: }dprintk ("NFSD: preprocess_seqid_op: bad seqid (expected %d, got %d)\n" , sop->so_seqid, seqid); - *sopp =NULL ;return nfserr_bad_seqid; }
@@ -3497,13 +3494,13 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,nfs4_lock_state (); -if ((status =nfs4_preprocess_seqid_op (cstate, + status =nfs4_preprocess_seqid_op (cstate, oc->oc_seqid, &oc->oc_req_stateid, -CONFIRM |OPEN_STATE , - &oc->oc_stateowner, &stp))) +CONFIRM |OPEN_STATE , &stp); +if (status) goto out; - sop = oc->oc_stateowner; + sop = stp->st_stateowner; sop->so_confirmed =1 ;update_stateid (&stp->st_stateid);memcpy (&oc->oc_resp_stateid, &stp->st_stateid,sizeof (stateid_t));
@@ -3555,11 +3552,9 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,return nfserr_inval;nfs4_lock_state (); -if ((status =nfs4_preprocess_seqid_op (cstate, - od->od_seqid, - &od->od_stateid, -OPEN_STATE , - &od->od_stateowner, &stp))) + status =nfs4_preprocess_seqid_op (cstate, od->od_seqid, + &od->od_stateid,OPEN_STATE , &stp); +if (status) goto out; status = nfserr_inval;
@@ -3594,6 +3589,7 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, {__be32 status;struct nfs4_stateid *stp; +struct nfs4_stateowner *so; dprintk("NFSD: nfsd4_close on file %.*s\n", (int)cstate->current_fh.fh_dentry->d_name.len,
@@ -3601,12 +3597,12 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,nfs4_lock_state ();/* check close_lru for replay */ -if ((status =nfs4_preprocess_seqid_op (cstate, - close->cl_seqid, + status =nfs4_preprocess_seqid_op (cstate, close->cl_seqid, &close->cl_stateid, -OPEN_STATE |CLOSE_STATE , - &close->cl_stateowner, &stp))) +OPEN_STATE |CLOSE_STATE , &stp); +if (status) goto out; + so = stp->st_stateowner; status = nfs_ok;update_stateid (&stp->st_stateid);memcpy (&close->cl_stateid, &stp->st_stateid,sizeof (stateid_t));
@@ -3618,8 +3614,8 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,* released by the laundromat service after the lease period * to enable us to handle CLOSE replay */ -if (list_empty (&close->cl_stateowner->so_stateids)) -move_to_close_lru (close->cl_stateowner); +if (list_empty (&so->so_stateids)) +move_to_close_lru (so); out:nfs4_unlock_state ();return status;
@@ -3969,12 +3965,11 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, status =nfs4_preprocess_seqid_op (cstate, lock->lk_new_open_seqid, &lock->lk_new_open_stateid, -OPEN_STATE , - &lock->lk_replay_owner, &open_stp); +OPEN_STATE , &open_stp);if (status) goto out; status = nfserr_bad_stateid; - open_sop = lock->lk_replay_owner; + open_sop = open_stp->st_stateowner;if (!nfsd4_has_session (cstate) && !same_clid (&open_sop->so_client->cl_clientid, &lock->v.new.clientid))
@@ -4000,14 +3995,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, status =nfs4_preprocess_seqid_op (cstate, lock->lk_old_lock_seqid, &lock->lk_old_lock_stateid, -LOCK_STATE , - &lock->lk_replay_owner, &lock_stp); +LOCK_STATE , &lock_stp);if (status) goto out; - lock_sop = lock->lk_replay_owner; + lock_sop = lock_stp->st_stateowner; fp = lock_stp->st_file; } -/* lock->lk_replay_owner and lock_stp have been created or found */ +/* lock_sop and lock_stp have been created or found */ lkflg =setlkflg (lock->lk_type); status =nfs4_check_openmode (lock_stp, lkflg);
@@ -4198,13 +4192,10 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,nfs4_lock_state (); -if ((status =nfs4_preprocess_seqid_op (cstate, - locku->lu_seqid, - &locku->lu_stateid, -LOCK_STATE , - &locku->lu_stateowner, &stp))) + status =nfs4_preprocess_seqid_op (cstate, locku->lu_seqid, + &locku->lu_stateid,LOCK_STATE , &stp); +if (status) goto out; - filp =find_any_file (stp->st_file);if (!filp) { status = nfserr_lock_range;
@@ -4213,7 +4204,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,BUG_ON (!filp);locks_init_lock (&file_lock); file_lock.fl_type =F_UNLCK ; - file_lock.fl_owner = (fl_owner_t ) locku->lu_stateowner; + file_lock.fl_owner = (fl_owner_t ) stp->st_stateowner; file_lock.fl_pid = current->tgid; file_lock.fl_file = filp; file_lock.fl_flags =FL_POSIX ;
fs/nfsd/nfs4xdr.c +0 -5
@@ -456,7 +456,6 @@ nfsd4_decode_close(struct nfsd4_compoundargs *argp, struct nfsd4_close *close) {DECODE_HEAD ; - close->cl_stateowner =NULL ;READ_BUF (4 );READ32 (close->cl_seqid);return nfsd4_decode_stateid (argp, &close->cl_stateid);
@@ -551,7 +550,6 @@ nfsd4_decode_lock(struct nfsd4_compoundargs *argp, struct nfsd4_lock *lock) {DECODE_HEAD ; - lock->lk_replay_owner =NULL ;/* * type, reclaim(boolean), offset, length, new_lock_owner(boolean) */
@@ -611,7 +609,6 @@ nfsd4_decode_locku(struct nfsd4_compoundargs *argp, struct nfsd4_locku *locku) {DECODE_HEAD ; - locku->lu_stateowner =NULL ;READ_BUF (8 );READ32 (locku->lu_type);if ((locku->lu_type <NFS4_READ_LT ) || (locku->lu_type >NFS4_WRITEW_LT ))
@@ -739,7 +736,6 @@ nfsd4_decode_open_confirm(struct nfsd4_compoundargs *argp, struct nfsd4_open_con {DECODE_HEAD ; - open_conf->oc_stateowner =NULL ; status =nfsd4_decode_stateid (argp, &open_conf->oc_req_stateid);if (status)return status;
@@ -754,7 +750,6 @@ nfsd4_decode_open_downgrade(struct nfsd4_compoundargs *argp, struct nfsd4_open_d {DECODE_HEAD ; - open_down->od_stateowner =NULL ; status =nfsd4_decode_stateid (argp, &open_down->od_stateid);if (status)return status;
fs/nfsd/xdr4.h +0 -7
@@ -81,7 +81,6 @@ struct nfsd4_access {struct nfsd4_close {u32 cl_seqid;/* request */ stateid_t cl_stateid;/* request+response */ -struct nfs4_stateowner * cl_stateowner;/* response */ };struct nfsd4_commit {
@@ -165,9 +164,6 @@ struct nfsd4_lock { } ok;struct nfsd4_lock_denied denied; } u; -/* The lk_replay_owner is the open owner in the open_to_lock_owner -* case and the lock owner otherwise: */ -struct nfs4_stateowner *lk_replay_owner; };#define lk_new_open_seqid v.new.open_seqid#define lk_new_open_stateid v.new.open_stateid
@@ -199,7 +195,6 @@ struct nfsd4_locku {stateid_t lu_stateid;u64 lu_offset;u64 lu_length; -struct nfs4_stateowner *lu_stateowner; };
@@ -243,7 +238,6 @@ struct nfsd4_open_confirm {stateid_t oc_req_stateid/* request */ ;u32 oc_seqid/* request */ ;stateid_t oc_resp_stateid/* response */ ; -struct nfs4_stateowner * oc_stateowner;/* response */ };struct nfsd4_open_downgrade {
@@ -251,7 +245,6 @@ struct nfsd4_open_downgrade {u32 od_seqid;u32 od_share_access;u32 od_share_deny; -struct nfs4_stateowner *od_stateowner; };
Signature · 1 line
1.7.4.1