Re: libata: Implement disk shock protection support

[Elias Oltmanns <eo@nebensachen.de>]

Hi Dan,

On 17 February 2016 at 20:24 CET, Dan Carpenter wrote:
> Hello Elias Oltmanns,
> The patch 45fabbb77bd9: "libata: Implement disk shock protection
> support" from Sep 21, 2008, leads to the following Smatch
> warning:
> 
>   drivers/ata/libata-scsi.c:206 ata_scsi_park_show()
>   warn: inconsistent returns 'irqsave:flags'.
> 	  Locked on:   line 206
> 	  Unlocked on: line 206
> 
> drivers/ata/libata-scsi.c
>    170  static ssize_t ata_scsi_park_show(struct device *device,
>    171                                    struct device_attribute *attr, char *buf)
>    172  {
[...]
>    183          spin_lock_irqsave(ap->lock, flags);
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
>    204          spin_unlock_irq(ap->lock);
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This should almost certainly be spin_unlock_irqrestore().

since this is an accessor function for a sysfs attribute (see:
DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
	    ata_scsi_park_show, ata_scsi_park_store);
at line 269), it will only ever be called from process context, in my
estimation. So, I suggest using spin_lock_irq() rather than the
_irqsave() variant. The same goes for the ata_scsi_park_store()
function.

Thanks for pointing out the inconsistency. Even though the patch itself
is quite straight forward, I won't have time to even compile test it
before the weekend.

Regards,

Elias