[PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sanjeev Sharma]

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 <Sanjeev_Sharma@mentor.com>
---
 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

RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sharma, Sanjeev]

Ping for review the patch.

thanks,

Sanjeev Sharma
-----Original Message-----
From: Sanjeev Sharma [mailto:sanjeev_sharma-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org] 
Sent: Tuesday, August 12, 2014 12:36 PM
To: dsd-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org; kune-hUSrv6EASfkEnNRfnnE9gw@public.gmane.org
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org; linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 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 <Sanjeev_Sharma-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Johannes Berg]

On Tue, 2014-08-19 at 06:39 +0000, Sharma, Sanjeev wrote:
> Ping for review the patch.

Make sure it compiles ...

RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sharma, Sanjeev]

Yes, it compiling successfully.


-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Friday, August 22, 2014 5:01 PM
To: Sharma, Sanjeev
Cc: dsd@gentoo.org; kune@deine-taler.de; linville@tuxdriver.com; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
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 ...

RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sharma, Sanjeev]

-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Friday, August 22, 2014 5:01 PM
To: Sharma, Sanjeev
Cc: dsd@gentoo.org; kune@deine-taler.de; linville@tuxdriver.com; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
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 ?

Regards
Sanjeev Sharma

Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Johannes Berg]

On Thu, 2014-09-11 at 04:02 +0000, Sharma, Sanjeev wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net] 
> Sent: Friday, August 22, 2014 5:01 PM
> To: Sharma, Sanjeev
> Cc: dsd@gentoo.org; kune@deine-taler.de; linville@tuxdriver.com; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> 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

[PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sanjeev Sharma]

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 <Sanjeev_Sharma@mentor.com>
---
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

Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Johannes Berg]

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 <Sanjeev_Sharma@mentor.com>
> ---
> 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

RE: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sharma, Sanjeev]

-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Thursday, September 11, 2014 3:42 PM
To: Sharma, Sanjeev
Cc: dsd@gentoo.org; kune@deine-taler.de; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
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 <Sanjeev_Sharma@mentor.com>
> ---
> 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  ?

Regards
Sanjeev Sharma

Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Johannes Berg]

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

Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Julian Calaby]

Hi Sanjeev,

On Thu, Sep 11, 2014 at 8:36 PM, Sharma, Sanjeev
<Sanjeev_Sharma@mentor.com> wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net]
> Sent: Thursday, September 11, 2014 3:42 PM
> To: Sharma, Sanjeev
> Cc: dsd@gentoo.org; kune@deine-taler.de; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> 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 <Sanjeev_Sharma@mentor.com>
>> ---
>> 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: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

RE: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

[Sharma, Sanjeev]

-----Original Message-----
From: Julian Calaby [mailto:julian.calaby@gmail.com] 
Sent: Monday, September 15, 2014 11:32 AM
To: Sharma, Sanjeev
Cc: Johannes Berg; dsd@gentoo.org; kune@deine-taler.de; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()

Hi Sanjeev,

On Thu, Sep 11, 2014 at 8:36 PM, Sharma, Sanjeev <Sanjeev_Sharma@mentor.com> wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net]
> Sent: Thursday, September 11, 2014 3:42 PM
> To: Sharma, Sanjeev
> Cc: dsd@gentoo.org; kune@deine-taler.de; 
> linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; 
> netdev@vger.kernel.org
> 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 <Sanjeev_Sharma@mentor.com>
>> ---
>> 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.

 I  feel logic is absolutely correct and if you expand it will looks like ..

+#define lockdep_assert_held(l)	do {	\ 
+	WARN_ON(debug_locks && !lockdep_is_held(l));	\ 
+	} while (0)

Please refer http://lkml.iu.edu/hypermail/linux/kernel/1203.2/00369.html and also see include/linux/lockdep.h for more detail.

Thanks
Sanjeev Sharma