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@xxxxxxxxxxxxxxx> > > 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. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@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
