All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>
Subject: serialize access to ide device
Date: Mon, 2 Aug 2004 15:11:51 +0200	[thread overview]
Message-ID: <20040802131150.GR10496@suse.de> (raw)

Hi Bart,

2.6 breaks really really easily if you have any traffic on a device and
issue a hdparm (or similar) command to it. Things like set_using_dma()
and ide_set_xfer_rate() just stomp all over the drive regardless of what
it's doing right now.

I hacked something up for the SUSE kernel to fix this _almost_, it still
doesn't handle cases where you want to serialize across more than a
single channel. Not a common case, but I think there is such hardware
out there (which?).

Clearly something needs to be done about this, it's extremely
frustrating not to be able to reliably turn on dma on a drive at all.
I'm just tossing this one out there to solve 99% of the case, I'd like
some input from you on what you feel we should do.

(Note patch is for the SLES9 base kernel, should be easy to adapt. I
didn't do that, since I'm just sparking conversation - I will if you
want it, though).

diff -urp /opt/kernel/linux-2.6.5/drivers/ide/ide.c linux-2.6.5/drivers/ide/ide.c
--- /opt/kernel/linux-2.6.5/drivers/ide/ide.c	2004-05-25 11:50:14.797583926 +0200
+++ linux-2.6.5/drivers/ide/ide.c	2004-05-25 11:52:40.367855970 +0200
@@ -1289,18 +1289,28 @@ 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
+	int ret = -EPERM;
+
+	ide_pin_hwgroup(drive);
+
 	if (!drive->id || !(drive->id->capability & 1))
-		return -EPERM;
+		goto out;
 	if (HWIF(drive)->ide_dma_check == NULL)
-		return -EPERM;
+		goto out;
+	ret = -EIO;
 	if (arg) {
-		if (HWIF(drive)->ide_dma_check(drive)) return -EIO;
-		if (HWIF(drive)->ide_dma_on(drive)) return -EIO;
+		if (HWIF(drive)->ide_dma_check(drive))
+			goto out;
+		if (HWIF(drive)->ide_dma_on(drive))
+			goto out;
 	} else {
 		if (__ide_dma_off(drive))
-			return -EIO;
+			goto out;
 	}
-	return 0;
+	ret = 0;
+out:
+	ide_unpin_hwgroup(drive);
+	return ret;
 #else
 	return -EPERM;
 #endif
diff -urp /opt/kernel/linux-2.6.5/drivers/ide/ide-io.c linux-2.6.5/drivers/ide/ide-io.c
--- /opt/kernel/linux-2.6.5/drivers/ide/ide-io.c	2004-05-25 11:50:15.174543192 +0200
+++ linux-2.6.5/drivers/ide/ide-io.c	2004-05-25 11:53:34.606000019 +0200
@@ -881,6 +881,46 @@ void ide_stall_queue (ide_drive_t *drive
 	drive->sleep = timeout + jiffies;
 }
 
+void ide_unpin_hwgroup(ide_drive_t *drive)
+{
+	ide_hwgroup_t *hwgroup = HWGROUP(drive);
+
+	if (hwgroup) {
+		spin_lock_irq(&ide_lock);
+		HWGROUP(drive)->busy = 0;
+		drive->blocked = 0;
+		do_ide_request(drive->queue);
+		spin_unlock_irq(&ide_lock);
+	}
+}
+
+void ide_pin_hwgroup(ide_drive_t *drive)
+{
+	ide_hwgroup_t *hwgroup = HWGROUP(drive);
+
+	/*
+	 * should only happen very early, so not a problem
+	 */
+	if (!hwgroup)
+		return;
+
+	spin_lock_irq(&ide_lock);
+	do {
+		if (!hwgroup->busy && !drive->blocked && !drive->doing_barrier)
+			break;
+		spin_unlock_irq(&ide_lock);
+		schedule_timeout(HZ/100);
+		spin_lock_irq(&ide_lock);
+	} while (hwgroup->busy || drive->blocked || drive->doing_barrier);
+
+	/*
+	 * we've now secured exclusive access to this hwgroup
+	 */
+	hwgroup->busy = 1;
+	drive->blocked = 1;
+	spin_unlock_irq(&ide_lock);
+}
+
 EXPORT_SYMBOL(ide_stall_queue);
 
 #define WAKEUP(drive)	((drive)->service_start + 2 * (drive)->service_time)
diff -urp /opt/kernel/linux-2.6.5/drivers/ide/ide-lib.c linux-2.6.5/drivers/ide/ide-lib.c
--- /opt/kernel/linux-2.6.5/drivers/ide/ide-lib.c	2004-05-25 11:50:15.204539951 +0200
+++ linux-2.6.5/drivers/ide/ide-lib.c	2004-05-25 11:52:40.433848845 +0200
@@ -436,13 +436,17 @@ EXPORT_SYMBOL(ide_toggle_bounce);
  
 int ide_set_xfer_rate(ide_drive_t *drive, u8 rate)
 {
+	int ret;
 #ifndef CONFIG_BLK_DEV_IDEDMA
 	rate = min(rate, (u8) XFER_PIO_4);
 #endif
-	if(HWIF(drive)->speedproc)
-		return HWIF(drive)->speedproc(drive, rate);
+	ide_pin_hwgroup(drive);
+	if (HWIF(drive)->speedproc)
+		ret = HWIF(drive)->speedproc(drive, rate);
 	else
-		return -1;
+		ret = -1;
+	ide_unpin_hwgroup(drive);
+	return ret;
 }
 
 EXPORT_SYMBOL_GPL(ide_set_xfer_rate);
diff -urp /opt/kernel/linux-2.6.5/include/linux/ide.h linux-2.6.5/include/linux/ide.h
--- /opt/kernel/linux-2.6.5/include/linux/ide.h	2004-05-25 11:50:29.701973356 +0200
+++ linux-2.6.5/include/linux/ide.h	2004-05-25 11:52:40.457846254 +0200
@@ -1474,6 +1474,9 @@ extern irqreturn_t ide_intr(int irq, voi
 extern void do_ide_request(request_queue_t *);
 extern void ide_init_subdrivers(void);
 
+extern void ide_pin_hwgroup(ide_drive_t *);
+extern void ide_unpin_hwgroup(ide_drive_t *);
+
 extern struct block_device_operations ide_fops[];
 extern ide_proc_entry_t generic_subdriver_entries[];
 

-- 
Jens Axboe


             reply	other threads:[~2004-08-02 13:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-08-02 13:11 Jens Axboe [this message]
2004-08-02 13:41 ` serialize access to ide device Geert Uytterhoeven
2004-08-02 13:49   ` Jens Axboe
2004-08-19 13:14 ` Bartlomiej Zolnierkiewicz
2004-08-19 12:32   ` Alan Cox
2004-08-21 10:32   ` Jens Axboe
2004-08-21 14:43     ` Bartlomiej Zolnierkiewicz
2004-08-21 16:21       ` Jens Axboe
2004-08-21 17:13         ` Bartlomiej Zolnierkiewicz
2004-08-23 12:15           ` Jens Axboe
2004-08-23 15:02             ` Bartlomiej Zolnierkiewicz
2004-08-23 14:50               ` Alan Cox
2004-08-23 15:10               ` Jens Axboe
2004-08-23 16:07               ` Jeff Garzik
2004-08-23 16:59                 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040802131150.GR10496@suse.de \
    --to=axboe@suse.de \
    --cc=B.Zolnierkiewicz@elka.pw.edu.pl \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.