ide: fix locking for manual DMA enable/disable ("hdparm -d")

PATCH New Bartlomiej Zolnierkiewicz 1 participant 1 message Mar 21, 2007 libata

Share & tools

Bartlomiej Zolnierkiewicz

author

2007-03-21 10:02 UTC #0 lore raw

PATCH 1/1 ide: fix locking for manual DMA enable/disable ("hdparm -d")

I would like to merge it for 2.6.21-final because this issue become quite urgent thanks to /etc/udev/scripts/hdparm.conf (the race is triggered by udev/hdparm tuning master device while the slave device is still being probed by the driver). Reviews/comments are greatly appreciated.

[PATCH] ide: fix locking for manual DMA enable/disable ("hdparm -d")

Since hwif->ide_dma_check and hwif->ide_dma_on never queue any commands (ide_config_drive_speed() sets transfer mode using polling and has no error recovery) we are safe with setting hwgroup->busy for the time while DMA setting for a drive is changed (so it won't race against I/O commands in fly).

I audited briefly all ->ide_dma_check/->ide_dma_on/->tuneproc/->speedproc implementations and they all look OK wrt to this change.

Cc Sergei Shtylyov <[email protected]>

Cc Alan Cox <[email protected]>

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---

 drivers/ide/ide.c |   37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

drivers/ide/ide.c +30 -7

Index: b/drivers/ide/ide.c
===================================================================
@@ -1124,17 +1124,40 @@ static int set_io_32bit(ide_drive_t *dri
 static int set_using_dma (ide_drive_t *drive, int arg)
 {
 #ifdef CONFIG_BLK_DEV_IDEDMA
+	ide_hwif_t *hwif = drive->hwif;
+	int err = -EPERM;
+
 	if (!drive->id || !(drive->id->capability & 1))
-		return -EPERM;
-	if (HWIF(drive)->ide_dma_check == NULL)
-		return -EPERM;
+		goto out;
+
+	if (hwif->ide_dma_check == NULL)
+		goto out;
+
+	err = -EBUSY;
+	if (ide_spin_wait_hwgroup(drive))
+		goto out;
+	/*
+	 * set ->busy flag, unlock and let it ride
+	 */
+	hwif->hwgroup->busy = 1;
+	spin_unlock_irq(&ide_lock);
+
+	err = 0;
+
 	if (arg) {
-		if (ide_set_dma(drive))
-			return -EIO;
-		if (HWIF(drive)->ide_dma_on(drive)) return -EIO;
+		if (ide_set_dma(drive) || hwif->ide_dma_on(drive))
+			err = -EIO;
 	} else
 		ide_dma_off(drive);
-	return 0;
+
+	/*
+	 * lock, clear ->busy flag and unlock before leaving
+	 */
+	spin_lock_irq(&ide_lock);
+	hwif->hwgroup->busy = 0;
+	spin_unlock_irq(&ide_lock);
+out:
+	return err;
 #else
 	return -EPERM;
 #endif