diff mbox

[1/1] SUNRPC: release clear_bit memory barrier

Message ID 1354298985-2344-1-git-send-email-andros@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Adamson Nov. 30, 2012, 6:09 p.m. UTC 
From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
A cleanup patch.

 net/sunrpc/auth.c              |    1 +
 net/sunrpc/auth_gss/auth_gss.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

Comments

Trond Myklebust Dec. 10, 2012, 8:02 p.m. UTC | #1 
On Fri, 2012-11-30 at 13:09 -0500, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>

> 

> Signed-off-by: Andy Adamson <andros@netapp.com>

> ---

> A cleanup patch.

> 

>  net/sunrpc/auth.c              |    1 +

>  net/sunrpc/auth_gss/auth_gss.c |    1 +

>  2 files changed, 2 insertions(+), 0 deletions(-)

> 

> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c

> index b5c067b..8b76753 100644

> --- a/net/sunrpc/auth.c

> +++ b/net/sunrpc/auth.c

> @@ -225,6 +225,7 @@ rpcauth_unhash_cred_locked(struct rpc_cred *cred)

>  	hlist_del_rcu(&cred->cr_hash);

>  	smp_mb__before_clear_bit();

>  	clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);

> +	smp_mb__after_clear_bit();

>  }

>  

>  static int

> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c

> index 909dc0c..92a7d6d 100644

> --- a/net/sunrpc/auth_gss/auth_gss.c

> +++ b/net/sunrpc/auth_gss/auth_gss.c

> @@ -126,6 +126,7 @@ gss_cred_set_ctx(struct rpc_cred *cred, struct gss_cl_ctx *ctx)

>  	set_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags);

>  	smp_mb__before_clear_bit();

>  	clear_bit(RPCAUTH_CRED_NEW, &cred->cr_flags);

> +	smp_mb__after_clear_bit();

>  }

>  

>  static const void *


Why are these needed?

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
Adamson, Andy Dec. 11, 2012, 3:32 p.m. UTC | #2 
On Dec 10, 2012, at 3:02 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Fri, 2012-11-30 at 13:09 -0500, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>> 
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> A cleanup patch.
>> 
>> net/sunrpc/auth.c              |    1 +
>> net/sunrpc/auth_gss/auth_gss.c |    1 +
>> 2 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index b5c067b..8b76753 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -225,6 +225,7 @@ rpcauth_unhash_cred_locked(struct rpc_cred *cred)
>> 	hlist_del_rcu(&cred->cr_hash);
>> 	smp_mb__before_clear_bit();
>> 	clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
>> +	smp_mb__after_clear_bit();
>> }
>> 
>> static int
>> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
>> index 909dc0c..92a7d6d 100644
>> --- a/net/sunrpc/auth_gss/auth_gss.c
>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>> @@ -126,6 +126,7 @@ gss_cred_set_ctx(struct rpc_cred *cred, struct gss_cl_ctx *ctx)
>> 	set_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags);
>> 	smp_mb__before_clear_bit();
>> 	clear_bit(RPCAUTH_CRED_NEW, &cred->cr_flags);
>> +	smp_mb__after_clear_bit();
>> }
>> 
>> static const void *
> 
> Why are these needed?

AFAICS the documentation says to either call both smp_mb__before_clear_bit and smp_mp__after_clear_bit to ensure that memory operations before and after the clear_bit call are strongly ordered, or don't call either. Since there can be multiple threads looking at a gss_cred cr_flags, it seems to me that we want the memory barriers to get the correct value of the bit.

-->Andy


from Documentation/atomic_ops.txt:

If explicit memory barriers are required around clear_bit() (which
does not return a value, and thus does not need to provide memory
barrier semantics), two interfaces are provided:

        void smp_mb__before_clear_bit(void);
        void smp_mb__after_clear_bit(void);

They are used as follows, and are akin to their atomic_t operation
brothers:

        /* All memory operations before this call will
         * be globally visible before the clear_bit().
         */
        smp_mb__before_clear_bit();
        clear_bit( ... );

        /* The clear_bit() will be visible before all
         * subsequent memory operations.
         */
         smp_mb__after_clear_bit();

> 
> -- 
> 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
Trond Myklebust Dec. 11, 2012, 3:41 p.m. UTC | #3 
T24gVHVlLCAyMDEyLTEyLTExIGF0IDE1OjMyICswMDAwLCBBZGFtc29uLCBBbmR5IHdyb3RlOg0K
PiBPbiBEZWMgMTAsIDIwMTIsIGF0IDM6MDIgUE0sICJNeWtsZWJ1c3QsIFRyb25kIiA8VHJvbmQu
TXlrbGVidXN0QG5ldGFwcC5jb20+IHdyb3RlOg0KPiANCj4gPiBPbiBGcmksIDIwMTItMTEtMzAg
YXQgMTM6MDkgLTA1MDAsIGFuZHJvc0BuZXRhcHAuY29tIHdyb3RlOg0KPiA+PiBGcm9tOiBBbmR5
IEFkYW1zb24gPGFuZHJvc0BuZXRhcHAuY29tPg0KPiA+PiANCj4gPj4gU2lnbmVkLW9mZi1ieTog
QW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gPj4gLS0tDQo+ID4+IEEgY2xlYW51
cCBwYXRjaC4NCj4gPj4gDQo+ID4+IG5ldC9zdW5ycGMvYXV0aC5jICAgICAgICAgICAgICB8ICAg
IDEgKw0KPiA+PiBuZXQvc3VucnBjL2F1dGhfZ3NzL2F1dGhfZ3NzLmMgfCAgICAxICsNCj4gPj4g
MiBmaWxlcyBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25zKC0pDQo+ID4+IA0K
PiA+PiBkaWZmIC0tZ2l0IGEvbmV0L3N1bnJwYy9hdXRoLmMgYi9uZXQvc3VucnBjL2F1dGguYw0K
PiA+PiBpbmRleCBiNWMwNjdiLi44Yjc2NzUzIDEwMDY0NA0KPiA+PiAtLS0gYS9uZXQvc3VucnBj
L2F1dGguYw0KPiA+PiArKysgYi9uZXQvc3VucnBjL2F1dGguYw0KPiA+PiBAQCAtMjI1LDYgKzIy
NSw3IEBAIHJwY2F1dGhfdW5oYXNoX2NyZWRfbG9ja2VkKHN0cnVjdCBycGNfY3JlZCAqY3JlZCkN
Cj4gPj4gCWhsaXN0X2RlbF9yY3UoJmNyZWQtPmNyX2hhc2gpOw0KPiA+PiAJc21wX21iX19iZWZv
cmVfY2xlYXJfYml0KCk7DQo+ID4+IAljbGVhcl9iaXQoUlBDQVVUSF9DUkVEX0hBU0hFRCwgJmNy
ZWQtPmNyX2ZsYWdzKTsNCj4gPj4gKwlzbXBfbWJfX2FmdGVyX2NsZWFyX2JpdCgpOw0KPiA+PiB9
DQo+ID4+IA0KPiA+PiBzdGF0aWMgaW50DQo+ID4+IGRpZmYgLS1naXQgYS9uZXQvc3VucnBjL2F1
dGhfZ3NzL2F1dGhfZ3NzLmMgYi9uZXQvc3VucnBjL2F1dGhfZ3NzL2F1dGhfZ3NzLmMNCj4gPj4g
aW5kZXggOTA5ZGMwYy4uOTJhN2Q2ZCAxMDA2NDQNCj4gPj4gLS0tIGEvbmV0L3N1bnJwYy9hdXRo
X2dzcy9hdXRoX2dzcy5jDQo+ID4+ICsrKyBiL25ldC9zdW5ycGMvYXV0aF9nc3MvYXV0aF9nc3Mu
Yw0KPiA+PiBAQCAtMTI2LDYgKzEyNiw3IEBAIGdzc19jcmVkX3NldF9jdHgoc3RydWN0IHJwY19j
cmVkICpjcmVkLCBzdHJ1Y3QgZ3NzX2NsX2N0eCAqY3R4KQ0KPiA+PiAJc2V0X2JpdChSUENBVVRI
X0NSRURfVVBUT0RBVEUsICZjcmVkLT5jcl9mbGFncyk7DQo+ID4+IAlzbXBfbWJfX2JlZm9yZV9j
bGVhcl9iaXQoKTsNCj4gPj4gCWNsZWFyX2JpdChSUENBVVRIX0NSRURfTkVXLCAmY3JlZC0+Y3Jf
ZmxhZ3MpOw0KPiA+PiArCXNtcF9tYl9fYWZ0ZXJfY2xlYXJfYml0KCk7DQo+ID4+IH0NCj4gPj4g
DQo+ID4+IHN0YXRpYyBjb25zdCB2b2lkICoNCj4gPiANCj4gPiBXaHkgYXJlIHRoZXNlIG5lZWRl
ZD8NCj4gDQo+IEFGQUlDUyB0aGUgZG9jdW1lbnRhdGlvbiBzYXlzIHRvIGVpdGhlciBjYWxsIGJv
dGggc21wX21iX19iZWZvcmVfY2xlYXJfYml0IGFuZCBzbXBfbXBfX2FmdGVyX2NsZWFyX2JpdCB0
byBlbnN1cmUgdGhhdCBtZW1vcnkgb3BlcmF0aW9ucyBiZWZvcmUgYW5kIGFmdGVyIHRoZSBjbGVh
cl9iaXQgY2FsbCBhcmUgc3Ryb25nbHkgb3JkZXJlZCwgb3IgZG9uJ3QgY2FsbCBlaXRoZXIuIFNp
bmNlIHRoZXJlIGNhbiBiZSBtdWx0aXBsZSB0aHJlYWRzIGxvb2tpbmcgYXQgYSBnc3NfY3JlZCBj
cl9mbGFncywgaXQgc2VlbXMgdG8gbWUgdGhhdCB3ZSB3YW50IHRoZSBtZW1vcnkgYmFycmllcnMg
dG8gZ2V0IHRoZSBjb3JyZWN0IHZhbHVlIG9mIHRoZSBiaXQuDQoNCk5vLiBJdCBkb2Vzbid0IHNh
eSBkbyBlaXRoZXIgYm90aCBvciBub3RoaW5nLiBJdCBzYXlzIHRoYXQgaWYgeW91IGNhbGwNCnNt
cF9tYl9fYWZ0ZXJfY2xlYXJfYml0KCksIHRoZW4gdGhlIGNsZWFyX2JpdCgpIHdpbGwgYmUgc3Ry
b25nbHkgb3JkZXJlZA0Kdy5yLnQuIGZ1dHVyZSBtZW1vcnkgb3BlcmF0aW9ucyBkb25lIGJ5IHRo
aXMgdGhyZWFkLiBNeSBxdWVzdGlvbiBpcyB3aHkNCndlIG5lZWQgdGhpcyBzdHJvbmcgb3JkZXJp
bmc/DQoNClRoZSBjdXJyZW50IGNvZGUgYWxyZWFkeSBvcmRlcnMgdGhlIGhsaXN0X2RlbF9yY3Uo
KSBhbmQgdGhlDQpzZXRfYml0KFJQQ0FVVEhfQ1JFRF9VUFRPREFURSkgdy5yLnQuIHRoZSBjbGVh
cl9iaXQoKSwgc28gYW55IHRocmVhZA0KdGhhdCBzZWVzIHRoZSBSUENBVVRIX0NSRURfSEFTSEVE
L1JQQ0FVVEhfQ1JFRF9ORVcgYml0cyBjbGVhcmVkIHdpbGwNCmFsc28gc2VlIHRoZSBsaXN0IHJl
bW92YWwvdXB0b2RhdGUgY3JlZC4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBj
bGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3
d3cubmV0YXBwLmNvbQ0K
--
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/net/sunrpc/auth.c b/net/sunrpc/auth.c
index b5c067b..8b76753 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -225,6 +225,7 @@  rpcauth_unhash_cred_locked(struct rpc_cred *cred)
 	hlist_del_rcu(&cred->cr_hash);
 	smp_mb__before_clear_bit();
 	clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
+	smp_mb__after_clear_bit();
 }
 
 static int
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 909dc0c..92a7d6d 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -126,6 +126,7 @@  gss_cred_set_ctx(struct rpc_cred *cred, struct gss_cl_ctx *ctx)
 	set_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags);
 	smp_mb__before_clear_bit();
 	clear_bit(RPCAUTH_CRED_NEW, &cred->cr_flags);
+	smp_mb__after_clear_bit();
 }
 
 static const void *