From: NeilBrown From: "J. Bruce Fields" From: Andros: Idea is to keep around a list of openowners recently released by closes, and make sure they stay around long enough so that replays still work. DESC Subject: Re: [PATCH] kNFSdv4 - 6 of 10 - Keep state to allow replays for 'close' to work. EDESC From: "J. Bruce Fields" On Wed, Apr 07, 2004 at 11:45:41PM -0700, Andrew Morton wrote: > NeilBrown wrote: > > > > + dprintk("NFSD: find_openstateowner_id %d\n", st_id); > > + if (flags & CLOSE_STATE) { > > + list_for_each_safe(pos, next, &close_lru) { > > + local = list_entry(pos, struct nfs4_stateowner, > > + so_close_lru); > > + if(local->so_id == st_id) > > + return local; > > + } > > } > > list_for_each() is adequate here. list_for_each_safe() is only needed if > you're removing things from the list inside the body of the loop. Someone was very fond of list_for_each_safe(). The following (applies on top of the above) removes all the list_for_each_safe()'s in nfs4state.c except for the two that were actually needed. > The locking in here is, err, a little coarse-grained? Yeah, everything on the server that deals with nfsv4 open and locking state takes a single big semaphore. We should fix that some day. --b. --- 25-akpm/fs/nfsd/nfs4state.c | 149 +++++++++++++++++++------------------ 25-akpm/include/linux/nfsd/state.h | 6 + 2 files changed, 84 insertions(+), 71 deletions(-) diff -puN fs/nfsd/nfs4state.c~kNFSdv4-6-of-10-Keep-state-to-allow-replays-for-close-to-work fs/nfsd/nfs4state.c --- 25/fs/nfsd/nfs4state.c~kNFSdv4-6-of-10-Keep-state-to-allow-replays-for-close-to-work 2004-04-12 22:51:16.442275472 -0700 +++ 25-akpm/fs/nfsd/nfs4state.c 2004-04-12 22:51:19.585797584 -0700 @@ -136,12 +136,16 @@ static void release_file(struct nfs4_fil * * client_lru holds client queue ordered by nfs4_client.cl_time * for lease renewal. + * + * close_lru holds (open) stateowner queue ordered by nfs4_stateowner.so_time + * for last close replay. */ static struct list_head conf_id_hashtbl[CLIENT_HASH_SIZE]; static struct list_head conf_str_hashtbl[CLIENT_HASH_SIZE]; static struct list_head unconf_str_hashtbl[CLIENT_HASH_SIZE]; static struct list_head unconf_id_hashtbl[CLIENT_HASH_SIZE]; static struct list_head client_lru; +static struct list_head close_lru; static inline void renew_client(struct nfs4_client *clp) @@ -376,7 +380,6 @@ nfsd4_setclientid(struct svc_rqst *rqstp unsigned int strhashval; struct nfs4_client * conf, * unconf, * new, * clp; int status; - struct list_head *pos, *next; status = nfserr_inval; if (!check_name(clname)) @@ -391,8 +394,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp conf = NULL; nfs4_lock_state(); - list_for_each_safe(pos, next, &conf_str_hashtbl[strhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_strhash); + list_for_each_entry(clp, &conf_str_hashtbl[strhashval], cl_strhash) { if (!cmp_name(&clp->cl_name, &clname)) continue; /* @@ -422,8 +424,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp break; } unconf = NULL; - list_for_each_safe(pos, next, &unconf_str_hashtbl[strhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_strhash); + list_for_each_entry(clp, &unconf_str_hashtbl[strhashval], cl_strhash) { if (!cmp_name(&clp->cl_name, &clname)) continue; /* cl_name match from a previous SETCLIENTID operation */ @@ -549,7 +550,6 @@ nfsd4_setclientid_confirm(struct svc_rqs struct nfs4_client *clp, *conf = NULL, *unconf = NULL; nfs4_verifier confirm = setclientid_confirm->sc_confirm; clientid_t * clid = &setclientid_confirm->sc_clientid; - struct list_head *pos, *next; int status; status = nfserr_stale_clientid; @@ -562,8 +562,7 @@ nfsd4_setclientid_confirm(struct svc_rqs idhashval = clientid_hashval(clid->cl_id); nfs4_lock_state(); - list_for_each_safe(pos, next, &conf_id_hashtbl[idhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_idhash); + list_for_each_entry(clp, &conf_id_hashtbl[idhashval], cl_idhash) { if (!cmp_clid(&clp->cl_clientid, clid)) continue; @@ -582,8 +581,7 @@ nfsd4_setclientid_confirm(struct svc_rqs conf = clp; break; } - list_for_each_safe(pos, next, &unconf_id_hashtbl[idhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_idhash); + list_for_each_entry(clp, &unconf_id_hashtbl[idhashval], cl_idhash) { if (!cmp_clid(&clp->cl_clientid, clid)) continue; status = nfserr_inval; @@ -774,6 +772,8 @@ alloc_init_open_stateowner(unsigned int INIT_LIST_HEAD(&sop->so_perclient); INIT_LIST_HEAD(&sop->so_perfilestate); INIT_LIST_HEAD(&sop->so_perlockowner); /* 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_perclient, &clp->cl_perclient); @@ -814,6 +814,7 @@ release_stateowner(struct nfs4_stateowne list_del(&sop->so_strhash); list_del(&sop->so_perclient); list_del(&sop->so_perlockowner); + list_del(&sop->so_close_lru); del_perclient++; while (!list_empty(&sop->so_perfilestate)) { stp = list_entry(sop->so_perfilestate.next, @@ -882,6 +883,19 @@ release_file(struct nfs4_file *fp) } void +move_to_close_lru(struct nfs4_stateowner *sop) +{ + dprintk("NFSD: move_to_close_lru nfs4_stateowner %p\n", sop); + /* remove stateowner from all other hash lists except perclient */ + list_del_init(&sop->so_idhash); + list_del_init(&sop->so_strhash); + list_del_init(&sop->so_perlockowner); + + list_add_tail(&sop->so_close_lru, &close_lru); + sop->so_time = get_seconds(); +} + +void release_state_owner(struct nfs4_stateid *stp, struct nfs4_stateowner **sopp, int flag) { @@ -890,16 +904,13 @@ release_state_owner(struct nfs4_stateid dprintk("NFSD: release_state_owner\n"); release_stateid(stp, flag); - /* - * release unused nfs4_stateowners. - * XXX will need to be placed on an open_stateid_lru list to be + + /* place unused nfs4_stateowners on so_close_lru list to be * released by the laundromat service after the lease period * to enable us to handle CLOSE replay */ - if (sop->so_confirmed && list_empty(&sop->so_perfilestate)) { - release_stateowner(sop); - *sopp = NULL; - } + if (sop->so_confirmed && list_empty(&sop->so_perfilestate)) + move_to_close_lru(sop); /* unused nfs4_file's are releseed. XXX slab cache? */ if (list_empty(&fp->fi_perfile)) { release_file(fp); @@ -916,11 +927,9 @@ cmp_owner_str(struct nfs4_stateowner *so /* search ownerstr_hashtbl[] for owner */ static int find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open, struct nfs4_stateowner **op) { - struct list_head *pos, *next; struct nfs4_stateowner *local = NULL; - list_for_each_safe(pos, next, &ownerstr_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateowner, so_strhash); + list_for_each_entry(local, &ownerstr_hashtbl[hashval], so_strhash) { if(!cmp_owner_str(local, &open->op_owner, &open->op_clientid)) continue; *op = local; @@ -933,12 +942,10 @@ find_openstateowner_str(unsigned int has static int verify_clientid(struct nfs4_client **client, clientid_t *clid) { - struct list_head *pos, *next; struct nfs4_client *clp; unsigned int idhashval = clientid_hashval(clid->cl_id); - list_for_each_safe(pos, next, &conf_id_hashtbl[idhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_idhash); + list_for_each_entry(clp, &conf_id_hashtbl[idhashval], cl_idhash) { if (!cmp_clid(&clp->cl_clientid, clid)) continue; *client = clp; @@ -951,11 +958,9 @@ verify_clientid(struct nfs4_client **cli /* search file_hashtbl[] for file */ static int find_file(unsigned int hashval, struct inode *ino, struct nfs4_file **fp) { - struct list_head *pos, *next; struct nfs4_file *local = NULL; - list_for_each_safe(pos, next, &file_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_file, fi_hash); + list_for_each_entry(local, &file_hashtbl[hashval], fi_hash) { if (local->fi_inode == ino) { *fp = local; return(1); @@ -1011,15 +1016,13 @@ nfs4_share_conflict(struct svc_fh *curre unsigned int fi_hashval; struct nfs4_file *fp; struct nfs4_stateid *stp; - struct list_head *pos, *next; dprintk("NFSD: nfs4_share_conflict\n"); fi_hashval = file_hashval(ino); if (find_file(fi_hashval, ino, &fp)) { /* Search for conflicting share reservations */ - list_for_each_safe(pos, next, &fp->fi_perfile) { - stp = list_entry(pos, struct nfs4_stateid, st_perfile); + list_for_each_entry(stp, &fp->fi_perfile, st_perfile) { if (test_bit(deny_type, &stp->st_deny_bmap) || test_bit(NFS4_SHARE_DENY_BOTH, &stp->st_deny_bmap)) return nfserr_share_denied; @@ -1154,7 +1157,6 @@ nfsd4_process_open2(struct svc_rqst *rqs struct nfs4_file *fp; struct inode *ino; unsigned int fi_hashval; - struct list_head *pos, *next; struct nfs4_stateid *stq, *stp = NULL; int status; @@ -1173,8 +1175,7 @@ nfsd4_process_open2(struct svc_rqst *rqs if (find_file(fi_hashval, ino, &fp)) { /* Search for conflicting share reservations */ status = nfserr_share_denied; - list_for_each_safe(pos, next, &fp->fi_perfile) { - stq = list_entry(pos, struct nfs4_stateid, st_perfile); + list_for_each_entry(stq, &fp->fi_perfile, st_perfile) { if(stq->st_stateowner == sop) { stp = stq; continue; @@ -1274,7 +1275,6 @@ int nfsd4_renew(clientid_t *clid) { struct nfs4_client *clp; - struct list_head *pos, *next; unsigned int idhashval; int status; @@ -1286,15 +1286,13 @@ nfsd4_renew(clientid_t *clid) goto out; status = nfs_ok; idhashval = clientid_hashval(clid->cl_id); - list_for_each_safe(pos, next, &conf_id_hashtbl[idhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_idhash); + list_for_each_entry(clp, &conf_id_hashtbl[idhashval], cl_idhash) { if (!cmp_clid(&clp->cl_clientid, clid)) continue; renew_client(clp); goto out; } - list_for_each_safe(pos, next, &unconf_id_hashtbl[idhashval]) { - clp = list_entry(pos, struct nfs4_client, cl_idhash); + list_for_each_entry(clp, &unconf_id_hashtbl[idhashval], cl_idhash) { if (!cmp_clid(&clp->cl_clientid, clid)) continue; renew_client(clp); @@ -1316,9 +1314,11 @@ time_t nfs4_laundromat(void) { struct nfs4_client *clp; + struct nfs4_stateowner *sop; struct list_head *pos, *next; time_t cutoff = get_seconds() - NFSD_LEASE_TIME; - time_t t, return_val = NFSD_LEASE_TIME; + time_t t, clientid_val = NFSD_LEASE_TIME; + time_t u, close_val = NFSD_LEASE_TIME; nfs4_lock_state(); @@ -1327,18 +1327,30 @@ nfs4_laundromat(void) clp = list_entry(pos, struct nfs4_client, cl_lru); if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) { t = clp->cl_time - cutoff; - if (return_val > t) - return_val = t; + if (clientid_val > t) + clientid_val = t; break; } dprintk("NFSD: purging unused client (clientid %08x)\n", clp->cl_clientid.cl_id); expire_client(clp); } - if (return_val < NFSD_LAUNDROMAT_MINTIMEOUT) - return_val = NFSD_LAUNDROMAT_MINTIMEOUT; + list_for_each_safe(pos, next, &close_lru) { + sop = list_entry(pos, struct nfs4_stateowner, so_close_lru); + if (time_after((unsigned long)sop->so_time, (unsigned long)cutoff)) { + u = sop->so_time - cutoff; + if (close_val > u) + close_val = u; + break; + } + dprintk("NFSD: purging unused open stateowner (so_id %d)\n", + sop->so_id); + release_stateowner(sop); + } + if (clientid_val < NFSD_LAUNDROMAT_MINTIMEOUT) + clientid_val = NFSD_LAUNDROMAT_MINTIMEOUT; nfs4_unlock_state(); - return return_val; + return clientid_val; } void @@ -1351,17 +1363,19 @@ laundromat_main(void *not_used) schedule_delayed_work(&laundromat_work, t*HZ); } -/* search ownerid_hashtbl[] for stateid owner (stateid->si_stateownerid) */ +/* search ownerid_hashtbl[] and close_lru for stateid owner + * (stateid->si_stateownerid) + */ struct nfs4_stateowner * -find_openstateowner_id(u32 st_id) { - struct list_head *pos, *next; +find_openstateowner_id(u32 st_id, int flags) { struct nfs4_stateowner *local = NULL; - unsigned int hashval = ownerid_hashval(st_id); - list_for_each_safe(pos, next, &ownerid_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateowner, so_idhash); - if(local->so_id == st_id) - return local; + dprintk("NFSD: find_openstateowner_id %d\n", st_id); + if (flags & CLOSE_STATE) { + list_for_each_entry(local, &close_lru, so_close_lru) { + if(local->so_id == st_id) + return local; + } } return NULL; } @@ -1547,11 +1561,12 @@ no_nfs4_stateid: * starting by trying to look up the stateowner. * If stateowner is not found - stateid is bad. */ - if (!(sop = find_openstateowner_id(stateid->si_stateownerid))) { + if (!(sop = find_openstateowner_id(stateid->si_stateownerid, flags))) { printk("NFSD: preprocess_seqid_op: no stateowner or nfs4_stateid!\n"); status = nfserr_bad_stateid; goto out; } + *sopp = sop; check_replay: if (seqid == sop->so_seqid) { @@ -1690,9 +1705,10 @@ nfsd4_close(struct svc_rqst *rqstp, stru current_fh->fh_dentry->d_name.name); nfs4_lock_state(); + /* check close_lru for replay */ if ((status = nfs4_preprocess_seqid_op(current_fh, close->cl_seqid, &close->cl_stateid, - CHECK_FH | OPEN_STATE, + CHECK_FH | OPEN_STATE | CLOSE_STATE, &close->cl_stateowner, &stp, NULL))) goto out; /* @@ -1729,7 +1745,6 @@ static struct list_head lockstateid_hash struct nfs4_stateid * find_stateid(stateid_t *stid, int flags) { - struct list_head *pos, *next; struct nfs4_stateid *local = NULL; u32 st_id = stid->si_stateownerid; u32 f_id = stid->si_fileid; @@ -1738,8 +1753,7 @@ find_stateid(stateid_t *stid, int flags) dprintk("NFSD: find_stateid flags 0x%x\n",flags); if ((flags & LOCK_STATE) || (flags & RDWR_STATE)) { hashval = stateid_hashval(st_id, f_id); - list_for_each_safe(pos, next, &lockstateid_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateid, st_hash); + 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; @@ -1747,8 +1761,7 @@ find_stateid(stateid_t *stid, int flags) } if ((flags & OPEN_STATE) || (flags & RDWR_STATE)) { hashval = stateid_hashval(st_id, f_id); - list_for_each_safe(pos, next, &stateid_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateid, st_hash); + 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; @@ -1779,14 +1792,12 @@ nfs4_transform_lock_offset(struct file_l int nfs4_verify_lock_stateowner(struct nfs4_stateowner *sop, unsigned int hashval) { - struct list_head *pos, *next; struct nfs4_stateowner *local = NULL; int status = 0; if (hashval >= LOCK_HASH_SIZE) goto out; - list_for_each_safe(pos, next, &lock_ownerid_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateowner, so_idhash); + list_for_each_entry(local, &lock_ownerid_hashtbl[hashval], so_idhash) { if (local == sop) { status = 1; goto out; @@ -1817,11 +1828,9 @@ nfs4_set_lock_denied(struct file_lock *f static int find_lockstateowner_str(unsigned int hashval, struct xdr_netobj *owner, clientid_t *clid, struct nfs4_stateowner **op) { - struct list_head *pos, *next; struct nfs4_stateowner *local = NULL; - list_for_each_safe(pos, next, &lock_ownerstr_hashtbl[hashval]) { - local = list_entry(pos, struct nfs4_stateowner, so_strhash); + list_for_each_entry(local, &lock_ownerstr_hashtbl[hashval], so_strhash) { if(!cmp_owner_str(local, owner, clid)) continue; *op = local; @@ -1854,6 +1863,8 @@ alloc_init_lock_stateowner(unsigned int INIT_LIST_HEAD(&sop->so_perclient); INIT_LIST_HEAD(&sop->so_perfilestate); INIT_LIST_HEAD(&sop->so_perlockowner); + 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_perclient, &clp->cl_perclient); @@ -2265,7 +2276,6 @@ int nfsd4_release_lockowner(struct svc_rqst *rqstp, struct nfsd4_release_lockowner *rlockowner) { clientid_t *clid = &rlockowner->rl_clientid; - struct list_head *pos, *next; struct nfs4_stateowner *local = NULL; struct xdr_netobj *owner = &rlockowner->rl_owner; int status, i; @@ -2286,9 +2296,7 @@ nfsd4_release_lockowner(struct svc_rqst /* find the lockowner */ status = nfs_ok; for (i=0; i < LOCK_HASH_SIZE; i++) { - list_for_each_safe(pos, next, &lock_ownerstr_hashtbl[i]) { - local = list_entry(pos, struct nfs4_stateowner, - so_strhash); + list_for_each_entry(local, &lock_ownerstr_hashtbl[i], so_strhash) { if(cmp_owner_str(local, owner, clid)) break; } @@ -2299,9 +2307,7 @@ nfsd4_release_lockowner(struct svc_rqst /* check for any locks held by any stateid associated with the * (lock) stateowner */ status = nfserr_locks_held; - list_for_each_safe(pos, next, &local->so_perfilestate) { - stp = list_entry(pos, struct nfs4_stateid, - st_perfilestate); + list_for_each_entry(stp, &local->so_perfilestate, st_perfilestate) { if(stp->st_vfs_set) { if (check_for_locks(&stp->st_vfs_file, local)) goto out; @@ -2351,6 +2357,7 @@ nfs4_state_init(void) memset(&zerostateid, 0, sizeof(stateid_t)); memset(&onestateid, ~0, sizeof(stateid_t)); + INIT_LIST_HEAD(&close_lru); INIT_LIST_HEAD(&client_lru); init_MUTEX(&client_sema); boot_time = get_seconds(); diff -puN include/linux/nfsd/state.h~kNFSdv4-6-of-10-Keep-state-to-allow-replays-for-close-to-work include/linux/nfsd/state.h --- 25/include/linux/nfsd/state.h~kNFSdv4-6-of-10-Keep-state-to-allow-replays-for-close-to-work 2004-04-12 22:51:16.444275168 -0700 +++ 25-akpm/include/linux/nfsd/state.h 2004-04-12 22:51:16.450274256 -0700 @@ -132,6 +132,9 @@ struct nfs4_replay { * release a stateowner. * so_perlockowner: (open) nfs4_stateid->st_perlockowner entry - used when * close is called to reap associated byte-range locks +* so_close_lru: (open) stateowner is placed on this list instead of being +* reaped (when so_perfilestate is empty) to hold the last close replay. +* reaped by laundramat thread after lease period. */ struct nfs4_stateowner { struct list_head so_idhash; /* hash by so_id */ @@ -139,6 +142,8 @@ struct nfs4_stateowner { struct list_head so_perclient; /* nfs4_client->cl_perclient */ struct list_head so_perfilestate; /* list: nfs4_stateid */ struct list_head so_perlockowner; /* nfs4_stateid->st_perlockowner */ + struct list_head so_close_lru; /* tail queue */ + time_t so_time; /* time of placement on so_close_lru */ int so_is_open_owner; /* 1=openowner,0=lockowner */ u32 so_id; struct nfs4_client * so_client; @@ -194,6 +199,7 @@ struct nfs4_stateid { #define OPEN_STATE 0x00000004 #define LOCK_STATE 0x00000008 #define RDWR_STATE 0x00000010 +#define CLOSE_STATE 0x00000020 #define seqid_mutating_err(err) \ (((err) != nfserr_stale_clientid) && \ _