[PATCH 1/1] SUNRPC: release clear_bit memory barrier

[PATCH 1/1] SUNRPC: release clear_bit memory barrier

[andros]

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

Re: [PATCH 1/1] SUNRPC: release clear_bit memory barrier

[Myklebust, Trond]

T24gRnJpLCAyMDEyLTExLTMwIGF0IDEzOjA5IC0wNTAwLCBhbmRyb3NAbmV0YXBwLmNvbSB3cm90
ZToNCj4gRnJvbTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gDQo+IFNpZ25l
ZC1vZmYtYnk6IEFuZHkgQWRhbXNvbiA8YW5kcm9zQG5ldGFwcC5jb20+DQo+IC0tLQ0KPiBBIGNs
ZWFudXAgcGF0Y2guDQo+IA0KPiAgbmV0L3N1bnJwYy9hdXRoLmMgICAgICAgICAgICAgIHwgICAg
MSArDQo+ICBuZXQvc3VucnBjL2F1dGhfZ3NzL2F1dGhfZ3NzLmMgfCAgICAxICsNCj4gIDIgZmls
ZXMgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAt
LWdpdCBhL25ldC9zdW5ycGMvYXV0aC5jIGIvbmV0L3N1bnJwYy9hdXRoLmMNCj4gaW5kZXggYjVj
MDY3Yi4uOGI3Njc1MyAxMDA2NDQNCj4gLS0tIGEvbmV0L3N1bnJwYy9hdXRoLmMNCj4gKysrIGIv
bmV0L3N1bnJwYy9hdXRoLmMNCj4gQEAgLTIyNSw2ICsyMjUsNyBAQCBycGNhdXRoX3VuaGFzaF9j
cmVkX2xvY2tlZChzdHJ1Y3QgcnBjX2NyZWQgKmNyZWQpDQo+ICAJaGxpc3RfZGVsX3JjdSgmY3Jl
ZC0+Y3JfaGFzaCk7DQo+ICAJc21wX21iX19iZWZvcmVfY2xlYXJfYml0KCk7DQo+ICAJY2xlYXJf
Yml0KFJQQ0FVVEhfQ1JFRF9IQVNIRUQsICZjcmVkLT5jcl9mbGFncyk7DQo+ICsJc21wX21iX19h
ZnRlcl9jbGVhcl9iaXQoKTsNCj4gIH0NCj4gIA0KPiAgc3RhdGljIGludA0KPiBkaWZmIC0tZ2l0
IGEvbmV0L3N1bnJwYy9hdXRoX2dzcy9hdXRoX2dzcy5jIGIvbmV0L3N1bnJwYy9hdXRoX2dzcy9h
dXRoX2dzcy5jDQo+IGluZGV4IDkwOWRjMGMuLjkyYTdkNmQgMTAwNjQ0DQo+IC0tLSBhL25ldC9z
dW5ycGMvYXV0aF9nc3MvYXV0aF9nc3MuYw0KPiArKysgYi9uZXQvc3VucnBjL2F1dGhfZ3NzL2F1
dGhfZ3NzLmMNCj4gQEAgLTEyNiw2ICsxMjYsNyBAQCBnc3NfY3JlZF9zZXRfY3R4KHN0cnVjdCBy
cGNfY3JlZCAqY3JlZCwgc3RydWN0IGdzc19jbF9jdHggKmN0eCkNCj4gIAlzZXRfYml0KFJQQ0FV
VEhfQ1JFRF9VUFRPREFURSwgJmNyZWQtPmNyX2ZsYWdzKTsNCj4gIAlzbXBfbWJfX2JlZm9yZV9j
bGVhcl9iaXQoKTsNCj4gIAljbGVhcl9iaXQoUlBDQVVUSF9DUkVEX05FVywgJmNyZWQtPmNyX2Zs
YWdzKTsNCj4gKwlzbXBfbWJfX2FmdGVyX2NsZWFyX2JpdCgpOw0KPiAgfQ0KPiAgDQo+ICBzdGF0
aWMgY29uc3Qgdm9pZCAqDQoNCldoeSBhcmUgdGhlc2UgbmVlZGVkPw0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQo=

Re: [PATCH 1/1] SUNRPC: release clear_bit memory barrier

[Adamson, Andy]

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

Re: [PATCH 1/1] SUNRPC: release clear_bit memory barrier

[Myklebust, Trond]

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