| Message ID | 1397846704-14567-35-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 18, 2014 at 02:44:28PM -0400, Trond Myklebust wrote: > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> I think at this point nfs4_lock_state() is always held so it's not quite a fix yet. If that's not true the patch should be moved earlier in the series. > -static int num_delegations; > +static atomic_long_t num_delegations; Why the switch from a int to an (atomic) long here? If that was intentional it should be documented in the patch description. > - if (num_delegations > max_delegations) > - return NULL; > + atomic_long_inc(&num_delegations); > + smp_mb__after_atomic_inc(); > + if (atomic_long_read(&num_delegations) > max_delegations) > + goto out_dec; Just use atomic_long_inc_return here. > +out_dec: > + atomic_long_dec(&num_delegations); > + smp_mb__after_atomic_dec(); I can't see any point for having these barriers. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2014-04-19 at 08:51 -0700, Christoph Hellwig wrote: > On Fri, Apr 18, 2014 at 02:44:28PM -0400, Trond Myklebust wrote: > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > > I think at this point nfs4_lock_state() is always held so it's not > quite a fix yet. If that's not true the patch should be moved earlier > in the series. Fair enough... > > > -static int num_delegations; > > +static atomic_long_t num_delegations; > > Why the switch from a int to an (atomic) long here? If that was > intentional it should be documented in the patch description. It is intentional. The max_delegations that we're comparing to below is of type 'unsigned long'. > > > - if (num_delegations > max_delegations) > > - return NULL; > > + atomic_long_inc(&num_delegations); > > + smp_mb__after_atomic_inc(); > > + if (atomic_long_read(&num_delegations) > max_delegations) > > + goto out_dec; > > Just use atomic_long_inc_return here. > > > +out_dec: > > + atomic_long_dec(&num_delegations); > > + smp_mb__after_atomic_dec(); > > I can't see any point for having these barriers. Agreed. I can't remember why I thought they were necessary.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index f463aaf9cd13..87464802126a 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -304,7 +304,7 @@ static struct file *find_any_file(struct nfs4_file *f) return ret; } -static int num_delegations; +static atomic_long_t num_delegations; unsigned long max_delegations; /* @@ -439,18 +439,19 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv struct nfs4_delegation *dp; dprintk("NFSD alloc_init_deleg\n"); - if (num_delegations > max_delegations) - return NULL; + atomic_long_inc(&num_delegations); + smp_mb__after_atomic_inc(); + if (atomic_long_read(&num_delegations) > max_delegations) + goto out_dec; dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab)); if (dp == NULL) - return dp; + goto out_dec; /* * delegation seqid's are never incremented. The 4.1 special * meaning of seqid 0 isn't meaningful, really, but let's avoid * 0 anyway just for consistency and use 1: */ dp->dl_stid.sc_stateid.si_generation = 1; - num_delegations++; INIT_LIST_HEAD(&dp->dl_perfile); INIT_LIST_HEAD(&dp->dl_perclnt); INIT_LIST_HEAD(&dp->dl_recall_lru); @@ -459,6 +460,10 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv dp->dl_time = 0; nfsd4_init_callback(&dp->dl_recall); return dp; +out_dec: + atomic_long_dec(&num_delegations); + smp_mb__after_atomic_dec(); + return NULL; } static void remove_stid_locked(struct nfs4_client *clp, struct nfs4_stid *s) @@ -490,8 +495,10 @@ static bool nfs4_put_stid(struct kmem_cache *slab, struct nfs4_stid *s) void nfs4_put_delegation(struct nfs4_delegation *dp) { - if (nfs4_put_stid(deleg_slab, &dp->dl_stid)) - num_delegations--; + if (nfs4_put_stid(deleg_slab, &dp->dl_stid)) { + atomic_long_dec(&num_delegations); + smp_mb__after_atomic_dec(); + } } /* Call under fp->fi_lock */
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfsd/nfs4state.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)