diff mbox

[5/5] NFSv4: clean up nfs4_state reference counting

Message ID 1382375414-5854-6-git-send-email-dros@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Weston Andros Adamson Oct. 21, 2013, 5:10 p.m. UTC 
Use atomic_inc_not_zero() to avoid referencing a state that is currently
being freed.

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c  | 8 ++++++--
 fs/nfs/nfs4state.c | 3 ++-
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Weston Andros Adamson Oct. 28, 2013, 6:53 p.m. UTC | #1 
On Oct 28, 2013, at 2:45 PM, Myklebust, Trond <Trond.Myklebust@netapp.com> wrote:

> On Mon, 2013-10-21 at 13:10 -0400, Weston Andros Adamson wrote:
>> Use atomic_inc_not_zero() to avoid referencing a state that is currently
>> being freed.
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> fs/nfs/nfs4proc.c  | 8 ++++++--
>> fs/nfs/nfs4state.c | 3 ++-
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index a3b78df..005543d 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -1275,7 +1275,8 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
>> out:
>> 	return ERR_PTR(ret);
>> out_return_state:
>> -	atomic_inc(&state->count);
>> +	if (!atomic_inc_not_zero(&state->count))
>> +		return ERR_PTR(-EINVAL);
>> 	return state;
>> }
>> 
>> @@ -1429,7 +1430,10 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
>> 	if (opendata == NULL)
>> 		return ERR_PTR(-ENOMEM);
>> 	opendata->state = state;
>> -	atomic_inc(&state->count);
>> +	if (!atomic_inc_not_zero(&state->count)) {
>> +		nfs4_opendata_put(opendata);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> 	return opendata;
>> }
>> 
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index cc14cbb..1c71907 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1416,7 +1416,8 @@ restart:
>> 			continue;
>> 		if (state->state == 0)
>> 			continue;
>> -		atomic_inc(&state->count);
>> +		if (!atomic_inc_not_zero(&state->count))
>> +			continue;
>> 		spin_unlock(&sp->so_lock);
>> 		status = ops->recover_open(sp, state);
>> 		if (status >= 0) {
> 
> Is this patch needed? We should already be holding a reference to the
> open_context, so AFAICS we know that the state->count is non-zero.

I have no evidence that this is needed, it was purely for consistency.  Feel free to drop this patch.

-dros

> 
> Cheers
>  Trond
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com

--
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
diff mbox

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a3b78df..005543d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1275,7 +1275,8 @@  static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
 out:
 	return ERR_PTR(ret);
 out_return_state:
-	atomic_inc(&state->count);
+	if (!atomic_inc_not_zero(&state->count))
+		return ERR_PTR(-EINVAL);
 	return state;
 }
 
@@ -1429,7 +1430,10 @@  static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
 	if (opendata == NULL)
 		return ERR_PTR(-ENOMEM);
 	opendata->state = state;
-	atomic_inc(&state->count);
+	if (!atomic_inc_not_zero(&state->count)) {
+		nfs4_opendata_put(opendata);
+		return ERR_PTR(-EINVAL);
+	}
 	return opendata;
 }
 
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index cc14cbb..1c71907 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1416,7 +1416,8 @@  restart:
 			continue;
 		if (state->state == 0)
 			continue;
-		atomic_inc(&state->count);
+		if (!atomic_inc_not_zero(&state->count))
+			continue;
 		spin_unlock(&sp->so_lock);
 		status = ops->recover_open(sp, state);
 		if (status >= 0) {