On Oct 28, 2013, at 5:06 PM, Myklebust, Trond <Trond.Myklebust@xxxxxxxxxx> wrote: > On Mon, 2013-10-21 at 13:10 -0400, Weston Andros Adamson wrote: >> There's already a valid state (the one being recovered), so just >> reference it. Also clean up error paths to avoid ref leaks. >> >> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxx> >> --- >> fs/nfs/nfs4proc.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index 8140366..8ae1589 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -1323,14 +1323,14 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data) >> goto err; >> } >> >> - ret = -ENOMEM; >> - state = nfs4_get_open_state(inode, data->owner); >> - if (state == NULL) >> + /* referenced the passed state */ >> + ret = -EINVAL; >> + if (state == NULL || !atomic_inc_not_zero(&state->count)) >> goto err; > > We already know that state != NULL, and that state->count != 0 here, so > I applied a simplified version of this patch that just does an > atomic_inc(&state->count) just before the function return. I just checked out the simplified patch - it looks good. Acked-by: Weston Andros Adamson <dros@xxxxxxxxxx> -dros > >> >> ret = nfs_refresh_inode(inode, &data->f_attr); >> if (ret) >> - goto err; >> + goto err_put; >> >> nfs_setsecurity(inode, &data->f_attr, data->f_label); >> >> @@ -1340,9 +1340,12 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data) >> data->o_arg.fmode); >> >> return state; >> + >> +err_put: >> + nfs4_put_open_state(state); >> + >> err: >> return ERR_PTR(ret); >> - >> } >> >> static struct nfs4_state * > > -- > Trond Myklebust > Linux NFS client maintainer > > NetApp > Trond.Myklebust@xxxxxxxxxx > www.netapp.com -- 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