From: "J. Bruce Fields" <bfields@fieldses.org>
On Wed, Apr 07, 2004 at 11:45:41PM -0700, Andrew Morton wrote:
> NeilBrown <neilb@cse.unsw.edu.au> 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 | 70 +++++++++++---------------------------------
1 files changed, 18 insertions(+), 52 deletions(-)
diff -puN fs/nfsd/nfs4state.c~nfsd_list_cleanup fs/nfsd/nfs4state.c
--- 25/fs/nfsd/nfs4state.c~nfsd_list_cleanup Thu Apr 8 14:32:45 2004
+++ 25-akpm/fs/nfsd/nfs4state.c Thu Apr 8 14:32:45 2004
@@ -380,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))
@@ -395,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;
/*
@@ -426,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 */
@@ -553,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;
@@ -566,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;
@@ -586,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;
@@ -933,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;
@@ -950,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;
@@ -968,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);
@@ -1028,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;
@@ -1171,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;
@@ -1190,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;
@@ -1291,7 +1275,6 @@ int
nfsd4_renew(clientid_t *clid)
{
struct nfs4_client *clp;
- struct list_head *pos, *next;
unsigned int idhashval;
int status;
@@ -1303,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);
@@ -1387,14 +1368,11 @@ laundromat_main(void *not_used)
*/
struct nfs4_stateowner *
find_openstateowner_id(u32 st_id, int flags) {
- struct list_head *pos, *next;
struct nfs4_stateowner *local = NULL;
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);
+ list_for_each_entry(local, &close_lru, so_close_lru) {
if(local->so_id == st_id)
return local;
}
@@ -1767,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;
@@ -1776,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;
@@ -1785,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;
@@ -1817,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;
@@ -1855,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;
@@ -2305,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;
@@ -2326,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;
}
@@ -2339,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;
_