on some architecture spin_is_locked() always return false in
uniprocessor configuration and therefore it would be advise
to replace with lockdep_assert_held().
Signed-off-by: Sanjeev Sharma <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index e7af261..72c0bc2 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac)
{
flush_workqueue(zd_workqueue);
zd_chip_clear(&mac->chip);
- ZD_ASSERT(!spin_is_locked(&mac->lock));
+ lockdep_assert_held(!spin_is_locked(&mac->lock));
ZD_MEMCLEAR(mac, sizeof(struct zd_mac));
}
--
1.7.11.7
Ping for review the patch.
thanks,
Sanjeev Sharma
-----Original Message-----
From: Sanjeev Sharma [mailto:[email protected]]
Sent: Tuesday, August 12, 2014 12:36 PM
To: [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Sharma, Sanjeev; Sharma, Sanjeev
Subject: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
on some architecture spin_is_locked() always return false in uniprocessor configuration and therefore it would be advise to replace with lockdep_assert_held().
Signed-off-by: Sanjeev Sharma <[email protected]>
---
drivers/net/wireless/zd1211rw/zd_mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index e7af261..72c0bc2 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) {
flush_workqueue(zd_workqueue);
zd_chip_clear(&mac->chip);
- ZD_ASSERT(!spin_is_locked(&mac->lock));
+ lockdep_assert_held(!spin_is_locked(&mac->lock));
ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); }
--
1.7.11.7
On Tue, 2014-08-19 at 06:39 +0000, Sharma, Sanjeev wrote:
> Ping for review the patch.
Make sure it compiles ...
WWVzLCBpdCBjb21waWxpbmcgc3VjY2Vzc2Z1bGx5Lg0KDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2Fn
ZS0tLS0tDQpGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWlsdG86am9oYW5uZXNAc2lwc29sdXRpb25z
Lm5ldF0gDQpTZW50OiBGcmlkYXksIEF1Z3VzdCAyMiwgMjAxNCA1OjAxIFBNDQpUbzogU2hhcm1h
LCBTYW5qZWV2DQpDYzogZHNkQGdlbnRvby5vcmc7IGt1bmVAZGVpbmUtdGFsZXIuZGU7IGxpbnZp
bGxlQHR1eGRyaXZlci5jb207IGxpbnV4LXdpcmVsZXNzQHZnZXIua2VybmVsLm9yZzsgbmV0ZGV2
QHZnZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZw0KU3ViamVjdDog
UmU6IFtQQVRDSF0gemQxMjExcnc6IHJlcGxhY2UgWkRfQVNTRVJUIHdpdGggbG9ja2RlcF9hc3Nl
cnRfaGVsZCgpDQoNCk9uIFR1ZSwgMjAxNC0wOC0xOSBhdCAwNjozOSArMDAwMCwgU2hhcm1hLCBT
YW5qZWV2IHdyb3RlOg0KPiBQaW5nIGZvciByZXZpZXcgdGhlIHBhdGNoLg0KDQpNYWtlIHN1cmUg
aXQgY29tcGlsZXMgLi4uDQoNCg0KDQo=
On Thu, 2014-09-11 at 15:39 +0530, Sanjeev Sharma wrote:
> on some architecture spin_is_locked() always return false in
> uniprocessor configuration and therefore it would be advise
> to replace with lockdep_assert_held().
>
> Signed-off-by: Sanjeev Sharma <[email protected]>
> ---
> Changes in v2:
> - corrected the typo
Now it compiles, but you got the logic wrong.
> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac)
> {
> flush_workqueue(zd_workqueue);
> zd_chip_clear(&mac->chip);
> - ZD_ASSERT(!spin_is_locked(&mac->lock));
> + lockdep_assert_held(&mac->lock);
> ZD_MEMCLEAR(mac, sizeof(struct zd_mac));
> }
Look closely at this again.
johannes
On Thu, 2014-09-11 at 04:02 +0000, Sharma, Sanjeev wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:[email protected]]
> Sent: Friday, August 22, 2014 5:01 PM
> To: Sharma, Sanjeev
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
>
> On Tue, 2014-08-19 at 06:39 +0000, Sharma, Sanjeev wrote:
> > Ping for review the patch.
>
> > Make sure it compiles ...
>
> Any update on this patch ?
I don't know, do you have any update for us?
Since your patch hasn't changed, it still cannot be compiled (when
lockdep is enabled). Make sure you test your patch before you submit it
again please.
johannes
Hi Sanjeev,
On Thu, Sep 11, 2014 at 8:36 PM, Sharma, Sanjeev
<[email protected]> wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:[email protected]]
> Sent: Thursday, September 11, 2014 3:42 PM
> To: Sharma, Sanjeev
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
>
> On Thu, 2014-09-11 at 15:39 +0530, Sanjeev Sharma wrote:
>> on some architecture spin_is_locked() always return false in
>> uniprocessor configuration and therefore it would be advise to replace
>> with lockdep_assert_held().
>>
>> Signed-off-by: Sanjeev Sharma <[email protected]>
>> ---
>> Changes in v2:
>> - corrected the typo
>
>> Now it compiles, but you got the logic wrong.
>
>> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
>> @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) {
>> flush_workqueue(zd_workqueue);
>> zd_chip_clear(&mac->chip);
>> - ZD_ASSERT(!spin_is_locked(&mac->lock));
>> + lockdep_assert_held(&mac->lock);
>> ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); }
>
>>Look closely at this again.
>
> I didn't understand where I put wrong logic ?
I find it helps to spell out what code is doing in words.
E.g. the line you're removing is:
ZD_ASSERT(!spin_is_locked(&mac->lock));
So, we'll assert when spin_is_locked(&mac->lock) is false, i.e. when
mac->lock is not spin locked.
This isn't the same as what you're replacing it with.
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEp1bGlhbiBDYWxhYnkgW21haWx0bzpq
dWxpYW4uY2FsYWJ5QGdtYWlsLmNvbV0gDQpTZW50OiBNb25kYXksIFNlcHRlbWJlciAxNSwgMjAx
NCAxMTozMiBBTQ0KVG86IFNoYXJtYSwgU2FuamVldg0KQ2M6IEpvaGFubmVzIEJlcmc7IGRzZEBn
ZW50b28ub3JnOyBrdW5lQGRlaW5lLXRhbGVyLmRlOyBsaW51eC13aXJlbGVzc0B2Z2VyLmtlcm5l
bC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IG5ldGRldkB2Z2VyLmtlcm5lbC5v
cmcNClN1YmplY3Q6IFJlOiBbUEFUQ0ggdjJdIHpkMTIxMXJ3OiByZXBsYWNlIFpEX0FTU0VSVCB3
aXRoIGxvY2tkZXBfYXNzZXJ0X2hlbGQoKQ0KDQpIaSBTYW5qZWV2LA0KDQpPbiBUaHUsIFNlcCAx
MSwgMjAxNCBhdCA4OjM2IFBNLCBTaGFybWEsIFNhbmplZXYgPFNhbmplZXZfU2hhcm1hQG1lbnRv
ci5jb20+IHdyb3RlOg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2hh
bm5lcyBCZXJnIFttYWlsdG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0NCj4gU2VudDogVGh1
cnNkYXksIFNlcHRlbWJlciAxMSwgMjAxNCAzOjQyIFBNDQo+IFRvOiBTaGFybWEsIFNhbmplZXYN
Cj4gQ2M6IGRzZEBnZW50b28ub3JnOyBrdW5lQGRlaW5lLXRhbGVyLmRlOyANCj4gbGludXgtd2ly
ZWxlc3NAdmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyANCj4g
bmV0ZGV2QHZnZXIua2VybmVsLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIHYyXSB6ZDEyMTFy
dzogcmVwbGFjZSBaRF9BU1NFUlQgd2l0aCANCj4gbG9ja2RlcF9hc3NlcnRfaGVsZCgpDQo+DQo+
IE9uIFRodSwgMjAxNC0wOS0xMSBhdCAxNTozOSArMDUzMCwgU2FuamVldiBTaGFybWEgd3JvdGU6
DQo+PiBvbiBzb21lIGFyY2hpdGVjdHVyZSBzcGluX2lzX2xvY2tlZCgpIGFsd2F5cyByZXR1cm4g
ZmFsc2UgaW4gDQo+PiB1bmlwcm9jZXNzb3IgY29uZmlndXJhdGlvbiBhbmQgdGhlcmVmb3JlIGl0
IHdvdWxkIGJlIGFkdmlzZSB0byANCj4+IHJlcGxhY2Ugd2l0aCBsb2NrZGVwX2Fzc2VydF9oZWxk
KCkuDQo+Pg0KPj4gU2lnbmVkLW9mZi1ieTogU2FuamVldiBTaGFybWEgPFNhbmplZXZfU2hhcm1h
QG1lbnRvci5jb20+DQo+PiAtLS0NCj4+IENoYW5nZXMgaW4gdjI6DQo+PiAgIC0gY29ycmVjdGVk
IHRoZSB0eXBvDQo+DQo+PiBOb3cgaXQgY29tcGlsZXMsIGJ1dCB5b3UgZ290IHRoZSBsb2dpYyB3
cm9uZy4NCj4NCj4+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL3pkMTIxMXJ3L3pkX21hYy5j
DQo+PiBAQCAtMjM1LDcgKzIzNSw3IEBAIHZvaWQgemRfbWFjX2NsZWFyKHN0cnVjdCB6ZF9tYWMg
Km1hYykgIHsNCj4+ICAgICAgIGZsdXNoX3dvcmtxdWV1ZSh6ZF93b3JrcXVldWUpOw0KPj4gICAg
ICAgemRfY2hpcF9jbGVhcigmbWFjLT5jaGlwKTsNCj4+IC0gICAgIFpEX0FTU0VSVCghc3Bpbl9p
c19sb2NrZWQoJm1hYy0+bG9jaykpOw0KPj4gKyAgICAgbG9ja2RlcF9hc3NlcnRfaGVsZCgmbWFj
LT5sb2NrKTsNCj4+ICAgICAgIFpEX01FTUNMRUFSKG1hYywgc2l6ZW9mKHN0cnVjdCB6ZF9tYWMp
KTsgIH0NCj4NCj4+TG9vayBjbG9zZWx5IGF0IHRoaXMgYWdhaW4uDQo+DQo+IEkgZGlkbid0IHVu
ZGVyc3RhbmQgd2hlcmUgSSAgcHV0IHdyb25nIGxvZ2ljICA/DQoNCkkgZmluZCBpdCBoZWxwcyB0
byBzcGVsbCBvdXQgd2hhdCBjb2RlIGlzIGRvaW5nIGluIHdvcmRzLg0KDQpFLmcuIHRoZSBsaW5l
IHlvdSdyZSByZW1vdmluZyBpczoNClpEX0FTU0VSVCghc3Bpbl9pc19sb2NrZWQoJm1hYy0+bG9j
aykpOw0KDQpTbywgd2UnbGwgYXNzZXJ0IHdoZW4gc3Bpbl9pc19sb2NrZWQoJm1hYy0+bG9jaykg
aXMgZmFsc2UsIGkuZS4gd2hlbg0KbWFjLT5sb2NrIGlzIG5vdCBzcGluIGxvY2tlZC4NCg0KVGhp
cyBpc24ndCB0aGUgc2FtZSBhcyB3aGF0IHlvdSdyZSByZXBsYWNpbmcgaXQgd2l0aC4NCg0KIEkg
IGZlZWwgbG9naWMgaXMgYWJzb2x1dGVseSBjb3JyZWN0IGFuZCBpZiB5b3UgZXhwYW5kIGl0IHdp
bGwgbG9va3MgbGlrZSAuLg0KDQorI2RlZmluZSBsb2NrZGVwX2Fzc2VydF9oZWxkKGwpCWRvIHsJ
XCANCisJV0FSTl9PTihkZWJ1Z19sb2NrcyAmJiAhbG9ja2RlcF9pc19oZWxkKGwpKTsJXCANCisJ
fSB3aGlsZSAoMCkNCg0KUGxlYXNlIHJlZmVyIGh0dHA6Ly9sa21sLml1LmVkdS9oeXBlcm1haWwv
bGludXgva2VybmVsLzEyMDMuMi8wMDM2OS5odG1sIGFuZCBhbHNvIHNlZSBpbmNsdWRlL2xpbnV4
L2xvY2tkZXAuaCBmb3IgbW9yZSBkZXRhaWwuDQoNClRoYW5rcw0KU2FuamVldiBTaGFybWENCg==
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEpvaGFubmVzIEJlcmcgW21haWx0bzpq
b2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XSANClNlbnQ6IEZyaWRheSwgQXVndXN0IDIyLCAyMDE0
IDU6MDEgUE0NClRvOiBTaGFybWEsIFNhbmplZXYNCkNjOiBkc2RAZ2VudG9vLm9yZzsga3VuZUBk
ZWluZS10YWxlci5kZTsgbGludmlsbGVAdHV4ZHJpdmVyLmNvbTsgbGludXgtd2lyZWxlc3NAdmdl
ci5rZXJuZWwub3JnOyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5r
ZXJuZWwub3JnDQpTdWJqZWN0OiBSZTogW1BBVENIXSB6ZDEyMTFydzogcmVwbGFjZSBaRF9BU1NF
UlQgd2l0aCBsb2NrZGVwX2Fzc2VydF9oZWxkKCkNCg0KT24gVHVlLCAyMDE0LTA4LTE5IGF0IDA2
OjM5ICswMDAwLCBTaGFybWEsIFNhbmplZXYgd3JvdGU6DQo+IFBpbmcgZm9yIHJldmlldyB0aGUg
cGF0Y2guDQoNCj4gTWFrZSBzdXJlIGl0IGNvbXBpbGVzIC4uLg0KDQpBbnkgdXBkYXRlIG9uIHRo
aXMgcGF0Y2ggPw0KDQpSZWdhcmRzDQpTYW5qZWV2IFNoYXJtYQ0KDQo=
on some architecture spin_is_locked() always return false in
uniprocessor configuration and therefore it would be advise
to replace with lockdep_assert_held().
Signed-off-by: Sanjeev Sharma <[email protected]>
---
Changes in v2:
- corrected the typo
drivers/net/wireless/zd1211rw/zd_mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
index e7af261..adce2ee 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac)
{
flush_workqueue(zd_workqueue);
zd_chip_clear(&mac->chip);
- ZD_ASSERT(!spin_is_locked(&mac->lock));
+ lockdep_assert_held(&mac->lock);
ZD_MEMCLEAR(mac, sizeof(struct zd_mac));
}
--
1.7.11.7
On Thu, 2014-09-11 at 10:36 +0000, Sharma, Sanjeev wrote:
> > - ZD_ASSERT(!spin_is_locked(&mac->lock));
> > + lockdep_assert_held(&mac->lock);
> >Look closely at this again.
>
> I didn't understand where I put wrong logic ?
Ok, that's fine. But please send a new patch only when you've understood
the logic.
johannes
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEpvaGFubmVzIEJlcmcgW21haWx0bzpq
b2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XSANClNlbnQ6IFRodXJzZGF5LCBTZXB0ZW1iZXIgMTEs
IDIwMTQgMzo0MiBQTQ0KVG86IFNoYXJtYSwgU2FuamVldg0KQ2M6IGRzZEBnZW50b28ub3JnOyBr
dW5lQGRlaW5lLXRhbGVyLmRlOyBsaW51eC13aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4
LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IG5ldGRldkB2Z2VyLmtlcm5lbC5vcmcNClN1YmplY3Q6
IFJlOiBbUEFUQ0ggdjJdIHpkMTIxMXJ3OiByZXBsYWNlIFpEX0FTU0VSVCB3aXRoIGxvY2tkZXBf
YXNzZXJ0X2hlbGQoKQ0KDQpPbiBUaHUsIDIwMTQtMDktMTEgYXQgMTU6MzkgKzA1MzAsIFNhbmpl
ZXYgU2hhcm1hIHdyb3RlOg0KPiBvbiBzb21lIGFyY2hpdGVjdHVyZSBzcGluX2lzX2xvY2tlZCgp
IGFsd2F5cyByZXR1cm4gZmFsc2UgaW4gDQo+IHVuaXByb2Nlc3NvciBjb25maWd1cmF0aW9uIGFu
ZCB0aGVyZWZvcmUgaXQgd291bGQgYmUgYWR2aXNlIHRvIHJlcGxhY2UgDQo+IHdpdGggbG9ja2Rl
cF9hc3NlcnRfaGVsZCgpLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogU2FuamVldiBTaGFybWEgPFNh
bmplZXZfU2hhcm1hQG1lbnRvci5jb20+DQo+IC0tLQ0KPiBDaGFuZ2VzIGluIHYyOg0KPiAgIC0g
Y29ycmVjdGVkIHRoZSB0eXBvDQoNCj4gTm93IGl0IGNvbXBpbGVzLCBidXQgeW91IGdvdCB0aGUg
bG9naWMgd3JvbmcuDQoNCj4gKysrIGIvZHJpdmVycy9uZXQvd2lyZWxlc3MvemQxMjExcncvemRf
bWFjLmMNCj4gQEAgLTIzNSw3ICsyMzUsNyBAQCB2b2lkIHpkX21hY19jbGVhcihzdHJ1Y3QgemRf
bWFjICptYWMpICB7DQo+ICAJZmx1c2hfd29ya3F1ZXVlKHpkX3dvcmtxdWV1ZSk7DQo+ICAJemRf
Y2hpcF9jbGVhcigmbWFjLT5jaGlwKTsNCj4gLQlaRF9BU1NFUlQoIXNwaW5faXNfbG9ja2VkKCZt
YWMtPmxvY2spKTsNCj4gKwlsb2NrZGVwX2Fzc2VydF9oZWxkKCZtYWMtPmxvY2spOw0KPiAgCVpE
X01FTUNMRUFSKG1hYywgc2l6ZW9mKHN0cnVjdCB6ZF9tYWMpKTsgIH0NCg0KPkxvb2sgY2xvc2Vs
eSBhdCB0aGlzIGFnYWluLg0KDQpJIGRpZG4ndCB1bmRlcnN0YW5kIHdoZXJlIEkgIHB1dCB3cm9u
ZyBsb2dpYyAgPw0KDQpSZWdhcmRzDQpTYW5qZWV2IFNoYXJtYQ0KDQo=