Re: [PATCH 02/16] scsi: atari_scsi: fix sleep_on race

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

 



Arnd Bergmann wrote:
sleep_on is known broken and going away. The atari_scsi driver is one of
two remaining users in the falcon_get_lock() function, which is a rather
crazy piece of code. This does not attempt to fix the driver's locking
scheme in general, but at least prevents falcon_get_lock from going to
sleep when no other thread holds the same lock or tries to get it,
and we no longer schedule with irqs disabled.

Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Michael Schmitz <schmitzmic@xxxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: James E.J. Bottomley <JBottomley@xxxxxxxxxxxxx>
Cc: linux-scsi@xxxxxxxxxxxxxxx
---
 drivers/scsi/atari_scsi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index a3e6c8a..b33ce34 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -90,6 +90,7 @@
 #include <linux/init.h>
 #include <linux/nvram.h>
 #include <linux/bitops.h>
+#include <linux/wait.h>
#include <asm/setup.h>
 #include <asm/atarihw.h>
@@ -549,8 +550,10 @@ static void falcon_get_lock(void)
local_irq_save(flags); - while (!in_irq() && falcon_got_lock && stdma_others_waiting())
-		sleep_on(&falcon_fairness_wait);
+	wait_event_cmd(falcon_fairness_wait,
+		       !in_irq() && falcon_got_lock && stdma_others_waiting(),
+		       local_irq_restore(flags),
+		       local_irq_save(flags));
while (!falcon_got_lock) {
 		if (in_irq())
@@ -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,
+				       local_irq_restore(flags),
+				       local_irq_save(flags));
 		}
 	}
Nack - the completion condition in the first hunk has its logic reversed. Try this instead (while() loops while condition true, do {} until () loops while condition false, no?)

I'm 99% confident I had tested your current version of the patch before and found it still attempts to schedule while in interrupt. I can retest if you prefer, but that'll have to wait a few days.

diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index a3e6c8a..cc1b013 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -90,6 +90,7 @@
#include <linux/init.h>
#include <linux/nvram.h>
#include <linux/bitops.h>
+#include <linux/wait.h>

#include <asm/setup.h>
#include <asm/atarihw.h>
@@ -549,8 +550,10 @@ static void falcon_get_lock(void)

       local_irq_save(flags);

-       while (!in_irq() && falcon_got_lock && stdma_others_waiting())
-               sleep_on(&falcon_fairness_wait);
+       wait_event_cmd(falcon_fairness_wait,
+               in_irq() || !falcon_got_lock || !stdma_others_waiting(),
+               local_irq_restore(flags),
+               local_irq_save(flags));

       while (!falcon_got_lock) {
               if (in_irq())
@@ -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,
+                               local_irq_restore(flags),
+                               local_irq_save(flags));
               }
       }


Cheers,

   Michael

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux