Re: Correct use of ap->lock versus ap->host->lock ?

[Jeff Garzik <jgarzik@pobox.com>]

Mark Lord wrote:
> Mark Lord wrote:
>> Jeff Garzik wrote:
>>> Mark Lord wrote:
>>>> Jeff / Tejun / Alan,
>>>>
>>>> I'm trying to sort out the spinlocks in sata_mv.
>>>>
>>>> In some places, the existing code uses ap->lock.
>>>> But in others, notably the interrupt handling, it uses ap->host->lock.
>>>>
>>>> This looks buggy to me, and I'm wondering how to make it bulletproof.
>>>
>>> Look closely, there is only one lock.  ata_port does not have a 
>>> spinlock, just a pointer...
>> ..
>>
>> Ahh.. in ata_port_alloc().  Thanks.
> ..
> 
> Okay.  Does the LLD even need to bother with this lock
> in the various pre/soft/hard reset routines ?
> 
> I don't think so, but sata_mv currently tries to lock there.

It depends on what needs to be locked, obviously...

(and no, that is not a sarcastic answer)

You need to perform your own LLD-specific locking analysis to see if it 
is safe to do, e.g. __mv_stop_dma() rather than mv_stop_dma().

All the msleep() calls in the reset routines should tell you that 
sleeping is OK, which means locking during EH is mostly accordingly to 
the needs of your driver.

EH makes sure various command submission machinery is shut down, thus 
creating the _possibility_ for lockless work, but your driver may need 
additional guarantees (and thus additional locking).

	Jeff