Re: [PATCH v2 04/10] nfsd: Ensure stateids remain unique until they are freed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 17 Jul 2014 14:44:17 -0400
"J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:

> On Wed, Jul 16, 2014 at 10:31:59AM -0400, Jeff Layton wrote:
> > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > 
> > Add an extra delegation state to allow the stateid to remain in the idr
> > tree until the last reference has been released. This will be necessary
> > to ensure uniqueness once the client_mutex is removed.
> 
> Cyclic idr allocation should make reuse extremely unlikely, shouldn't
> it?
> 
> Probably needs to be fixed anyway, applying.
> 
> --b.
> 

Yes, for the most part, but reuse is still possible once the counter
wraps if you had a long-lived stateid (for instance). Not a likely
problem, but we might as well fix it while we're in here.
 
> > 
> > [jlayton: reset the sc_type under the state_lock in
> > unhash_delegation]
> > 
> > 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 | 8 ++++----
> >  fs/nfsd/state.h     | 1 +
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 1b01a20827ab..fd4deb049ddf 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -616,6 +616,7 @@ void
> >  nfs4_put_delegation(struct nfs4_delegation *dp)
> >  {
> >  	if (atomic_dec_and_test(&dp->dl_count)) {
> > +		remove_stid(&dp->dl_stid);
> >  		nfs4_free_stid(deleg_slab, &dp->dl_stid);
> >  		num_delegations--;
> >  	}
> > @@ -657,6 +658,7 @@ unhash_delegation(struct nfs4_delegation *dp)
> >  	struct nfs4_file *fp = dp->dl_file;
> >  
> >  	spin_lock(&state_lock);
> > +	dp->dl_stid.sc_type = NFS4_CLOSED_DELEG_STID;
> >  	list_del_init(&dp->dl_perclnt);
> >  	list_del_init(&dp->dl_recall_lru);
> >  	spin_lock(&fp->fi_lock);
> > @@ -670,19 +672,15 @@ unhash_delegation(struct nfs4_delegation *dp)
> >  	}
> >  }
> >  
> > -
> > -
> >  static void destroy_revoked_delegation(struct nfs4_delegation *dp)
> >  {
> >  	list_del_init(&dp->dl_recall_lru);
> > -	remove_stid(&dp->dl_stid);
> >  	nfs4_put_delegation(dp);
> >  }
> >  
> >  static void destroy_delegation(struct nfs4_delegation *dp)
> >  {
> >  	unhash_delegation(dp);
> > -	remove_stid(&dp->dl_stid);
> >  	nfs4_put_delegation(dp);
> >  }
> >  
> > @@ -4036,7 +4034,9 @@ static __be32 nfsd4_validate_stateid(struct
> > nfs4_client *cl, stateid_t *stateid) return nfs_ok;
> >  	default:
> >  		printk("unknown stateid type %x\n", s->sc_type);
> > +		/* Fallthrough */
> >  	case NFS4_CLOSED_STID:
> > +	case NFS4_CLOSED_DELEG_STID:
> >  		return nfserr_bad_stateid;
> >  	}
> >  }
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 81b7522e3f67..996d61eeb357 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -80,6 +80,7 @@ struct nfs4_stid {
> >  #define NFS4_CLOSED_STID 8
> >  /* For a deleg stateid kept around only to process free_stateid's:
> > */ #define NFS4_REVOKED_DELEG_STID 16
> > +#define NFS4_CLOSED_DELEG_STID 32
> >  	unsigned char sc_type;
> >  	stateid_t sc_stateid;
> >  	struct nfs4_client *sc_client;
> > -- 
> > 1.9.3
> > 


-- 
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
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




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux