diff mbox

[1/3] nfsd: return correct openowner when there is a race to put one in the hash

Message ID 20150325084516.GC25682@infradead.org (mailing list archive)
State New, archived
Headers show

Commit Message

Christoph Hellwig March 25, 2015, 8:45 a.m. UTC
Good catch!

Reviewed-by: Christoph Hellwig <hch@lst.de>

But maybe it's better to write the code so that the intent is obvious:

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

Comments

Jeff Layton March 25, 2015, 10:28 a.m. UTC | #1
On Wed, 25 Mar 2015 01:45:16 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> Good catch!
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But maybe it's better to write the code so that the intent is obvious:
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f6b2a09..3c4c3d2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -3215,11 +3215,13 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
>  	INIT_LIST_HEAD(&oo->oo_close_lru);
>  	spin_lock(&clp->cl_lock);
>  	ret = find_openstateowner_str_locked(strhashval, open, clp);
> -	if (ret == NULL) {
> -		hash_openowner(oo, clp, strhashval);
> -		ret = oo;
> -	} else
> +	if (ret) {
>  		nfs4_free_openowner(&oo->oo_owner);
> +		spin_unlock(&clp->cl_lock);
> +		return ret;
> +	}
> +
> +	hash_openowner(oo, clp, strhashval);
>  	spin_unlock(&clp->cl_lock);
>  	return oo;
>  }

Sure, that'd be fine too. Care to write up a changelog entry and send
it to Bruce? I think that would likely apply just fine to stable
kernels.
Christoph Hellwig March 25, 2015, 1:51 p.m. UTC | #2
Let's just keep your version for simplicity for now..
--
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/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f6b2a09..3c4c3d2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3215,11 +3215,13 @@  alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
 	INIT_LIST_HEAD(&oo->oo_close_lru);
 	spin_lock(&clp->cl_lock);
 	ret = find_openstateowner_str_locked(strhashval, open, clp);
-	if (ret == NULL) {
-		hash_openowner(oo, clp, strhashval);
-		ret = oo;
-	} else
+	if (ret) {
 		nfs4_free_openowner(&oo->oo_owner);
+		spin_unlock(&clp->cl_lock);
+		return ret;
+	}
+
+	hash_openowner(oo, clp, strhashval);
 	spin_unlock(&clp->cl_lock);
 	return oo;
 }