On Mon, Jul 21, 2014 at 09:34:59AM -0400, Jeff Layton wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> Don't allow stateids to clear the open file pointer until they are
> being destroyed. In a later patch we'll want to move the putting of
> the nfs4_file into generic stateid handling code and this will help
> facilitate that change.
>
> Also, move to allocating stateids with kzalloc and get rid of the
> explicit zeroing of fields.
There's a regression here: we end up holding an unnecessary reference to
the inode a long time just because some client isn't responding to a
recall.
When I complained about this before Trond said something about replacing
fi_inode by a filehandle? But I can't remember if we've seen code to do
that.
--b.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/nfsd/nfs4state.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a47c97586c76..8ff5c97a2c5a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -459,7 +459,7 @@ kmem_cache *slab)
> struct nfs4_stid *stid;
> int new_id;
>
> - stid = kmem_cache_alloc(slab, GFP_KERNEL);
> + stid = kmem_cache_zalloc(slab, GFP_KERNEL);
> if (!stid)
> return NULL;
>
> @@ -467,11 +467,9 @@ kmem_cache *slab)
> if (new_id < 0)
> goto out_free;
> stid->sc_client = cl;
> - stid->sc_type = 0;
> stid->sc_stateid.si_opaque.so_id = new_id;
> stid->sc_stateid.si_opaque.so_clid = cl->cl_clientid;
> /* Will be incremented before return to client: */
> - stid->sc_stateid.si_generation = 0;
> atomic_set(&stid->sc_count, 1);
>
> /*
> @@ -592,10 +590,8 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
> INIT_LIST_HEAD(&dp->dl_perfile);
> INIT_LIST_HEAD(&dp->dl_perclnt);
> INIT_LIST_HEAD(&dp->dl_recall_lru);
> - dp->dl_file = NULL;
> dp->dl_type = NFS4_OPEN_DELEGATE_READ;
> fh_copy_shallow(&dp->dl_fh, ¤t_fh->fh_handle);
> - dp->dl_time = 0;
> INIT_WORK(&dp->dl_recall.cb_work, nfsd4_run_cb_recall);
> return dp;
> }
> @@ -616,6 +612,8 @@ void
> nfs4_put_delegation(struct nfs4_delegation *dp)
> {
> if (atomic_dec_and_test(&dp->dl_stid.sc_count)) {
> + if (dp->dl_file)
> + put_nfs4_file(dp->dl_file);
> remove_stid(&dp->dl_stid);
> nfs4_free_stid(deleg_slab, &dp->dl_stid);
> num_delegations--;
> @@ -665,13 +663,9 @@ unhash_delegation(struct nfs4_delegation *dp)
> list_del_init(&dp->dl_recall_lru);
> list_del_init(&dp->dl_perfile);
> spin_unlock(&fp->fi_lock);
> - if (fp) {
> + if (fp)
> nfs4_put_deleg_lease(fp);
> - dp->dl_file = NULL;
> - }
> spin_unlock(&state_lock);
> - if (fp)
> - put_nfs4_file(fp);
> }
>
> static void destroy_revoked_delegation(struct nfs4_delegation *dp)
> @@ -879,12 +873,12 @@ static void unhash_generic_stateid(struct nfs4_ol_stateid *stp)
> static void close_generic_stateid(struct nfs4_ol_stateid *stp)
> {
> release_all_access(stp);
> - put_nfs4_file(stp->st_file);
> - stp->st_file = NULL;
> }
>
> static void free_generic_stateid(struct nfs4_ol_stateid *stp)
> {
> + if (stp->st_file)
> + put_nfs4_file(stp->st_file);
> remove_stid(&stp->st_stid);
> nfs4_free_stid(stateid_slab, &stp->st_stid);
> }
> @@ -4462,6 +4456,10 @@ static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
> if (list_empty(&oo->oo_owner.so_stateids))
> release_openowner(oo);
> } else {
> + if (s->st_file) {
> + put_nfs4_file(s->st_file);
> + s->st_file = NULL;
> + }
> oo->oo_last_closed_stid = s;
> /*
> * In the 4.0 case we need to keep the owners around a
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
