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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Arnd, Geert,


On Tue, Jan 14, 2014 at 9:29 PM, Michael Schmitz <schmitzmic@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux