Re: [PATCH, RFC 02/30] scsi: atari_scsi: fix sleep_on race

Michael Schmitz
2014-01-19T22:04:59Z

Arnd, Geert,

On Tue, Jan 14, 2014 at 9:29 PM, Michael Schmitz <schmitzmic@gmail.com> wrote:
> Arnd,
>
> this one looks like it should be changed as well:
>
>>>> @@ -562,7 +565,10 @@ static void falcon_get_lock(void)
>>>> falcon_trying_lock = 0;
>>>> wake_up(&falcon_try_wait);
>>>> } else {
>>>> - sleep_on(&falcon_try_wait);
>>>> + wait_event_cmd(falcon_try_wait,
>>>> + !falcon_got_lock &&
>>>> !falcon_trying_lock,
>
>
> I think using falcon_got_lock && !falcon_trying_lock here reflects what we
> try to achieve - the if branch above this will set that exact
> condition, then wake the wait queue. Any other calls to falcon_get_lock that
> have been waiting on the lock should then continue on
> as the lock is now held by the driver. Thoughts?

I can confirm that the condition (falcon_got_lock &&
!falcon_trying_lock) does work here - no problems for basic
operations. Arnd, do you want to handle this, or should I send the
updated patch via Geert?

A bit of stress testing using concurrent SCSI and IDE I/O has revealed
the driver still deadlocks when SCSI commands are queued from softirq
context and the ST-DMA lock is held by the IDE or floppy driver
(presumably; haven't verified that). I will try a few likely
workarounds that had worked in the past, but the real question is
this:

Does stdma_lock need the uninterruptible wait? The comment there says
it is needed to protect locked buffers, but this was all set up back
in the dark ages. IDE takes the host lock in addition to the ST-DMA
lock, SCSI still locks out interrupts where critical (I need to change
that) so it appears all should be safe? Comments?

Cheers,

Michael