RE: [PATCH 01/21] uas: replace WARN_ON_ONCE() with lockdep_assert_held()

Sharma, Sanjeev
2014-09-11T03:42:54Z

-----Original Message-----
From: Hans de Goede [mailto:hdegoede@redhat.com]
Sent: Wednesday, September 10, 2014 8:33 PM
To: Peter Hurley; Greg Kroah-Hartman
Cc: linux-usb@vger.kernel.org; linux-scsi@vger.kernel.org; stable@vger.kernel.org; Sharma, Sanjeev; Peter Zijlstra; Ingo Molnar
Subject: Re: [PATCH 01/21] uas: replace WARN_ON_ONCE() with lockdep_assert_held()

Hi,

On 09/10/2014 04:38 PM, Peter Hurley wrote:
> [ +cc Peter Zijlstra, Ingo Molnar ]
>
> On 09/10/2014 07:46 AM, Hans de Goede wrote:
>> From: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
>>
>> 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>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/usb/storage/uas.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
>> index 3f42785..05b2d8e 100644
>> --- a/drivers/usb/storage/uas.c
>> +++ b/drivers/usb/storage/uas.c
>> @@ -154,7 +154,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
>> struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp);
>>
>> uas_log_cmd_state(cmnd, caller);
>> - WARN_ON_ONCE(!spin_is_locked(&devinfo->lock));
>> + lockdep_assert_held(&devinfo->lock);
>
> This change isn't equivalent.
>
> lockdep_assert_held() will continue to emit warnings; ie., there is no
> "once" functionality. Same for the other changes below.

>Given that these should really never ever happen, that is not really a problem IMHO. The idea ws to replace the wrong use of spin_is_locked with some other sanity check, preferably a light weight one, the once behavior is not that important.

I am totally agree with the hans and that was my intention for UP system.

Regards
Sanjeev Sharma