ide: don't execute the next queued command from the hard-IRQ context (v2)

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Linux Kernel Mailing List
Date: Friday, January 2, 2009 - 1:03 pm

Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=295f00...
Commit:     295f00042aaf6b553b5f37348f89bab463d4a469
Parent:     ebdab07dad3d3a008e519b0a028e1e1ad5ecaef0
Author:     Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
AuthorDate: Fri Jan 2 16:12:48 2009 +0100
Committer:  Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
CommitDate: Fri Jan 2 16:12:48 2009 +0100

    ide: don't execute the next queued command from the hard-IRQ context (v2)
    
    * Tell the block layer that we are not done handling requests by using
      blk_plug_device() in ide_do_request() (request handling function)
      and ide_timer_expiry() (timeout handler) if the queue is not empty.
    
    * Remove optimization which directly calls ide_do_request() for the next
      queued command from the ide_intr() (IRQ handler) and ide_timer_expiry().
    
    * Remove no longer needed IRQ masking from ide_do_request() - in case of
      IDE ports needing serialization disable_irq_nosync()/enable_irq() was
      used for the (possibly shared) IRQ of the other IDE port.
    
    * Put the misplaced comment in the right place in ide_do_request().
    
    * Drop no longer needed 'int masked_irq' argument from ide_do_request().
    
    * Merge ide_do_request() into do_ide_request().
    
    * Remove no longer needed IDE_NO_IRQ define.
    
    While at it:
    
    * Don't use HWGROUP() macro in do_ide_request().
    
    * Use __func__ in ide_intr().
    
    This patch reduces IRQ hadling latency for IDE and improves the system-wide
    handling of shared IRQs (which should result in more timeout resistant and
    stable IDE systems).  It also makes it possible to do some further changes
    later (i.e. replace some busy-waiting delays with sleeping equivalents).
    
    v2:
    Changes per review from Elias Oltmanns:
    - fix wrong goto statement in 'if (startstop == ide_stopped)' block
    - use spin_unlock_irq()
    - don't use obsolete HWIF() macro
    
    Cc: Elias Oltmanns <eo@nebensachen.de>
    Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c |   67 ++++++++++++++++++++++---------------------------
 include/linux/ide.h  |    7 -----
 2 files changed, 30 insertions(+), 44 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index ecacc00..23754bc 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -778,8 +778,10 @@ repeat:
  * the driver.  This makes the driver much more friendlier to shared IRQs
  * than previous designs, while remaining 100% (?) SMP safe and capable.
  */
-static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq)
+void do_ide_request(struct request_queue *q)
 {
+	ide_drive_t	*orig_drive = q->queuedata;
+	ide_hwgroup_t	*hwgroup = orig_drive->hwif->hwgroup;
 	ide_drive_t	*drive;
 	ide_hwif_t	*hwif;
 	struct request	*rq;
@@ -837,10 +839,14 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq)
 			}
 
 			/* no more work for this hwgroup (for now) */
-			return;
+			goto plug_device;
 		}
-	again:
-		hwif = HWIF(drive);
+
+		if (drive != orig_drive)
+			goto plug_device;
+again:
+		hwif = drive->hwif;
+
 		if (hwif != hwgroup->hwif) {
 			/*
 			 * set nIEN for previous hwif, drives in the
@@ -888,41 +894,26 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq)
 				goto again;
 			/* We clear busy, there should be no pending ATA command at this point. */
 			hwgroup->busy = 0;
-			break;
+			goto plug_device;
 		}
 
 		hwgroup->rq = rq;
 
-		/*
-		 * Some systems have trouble with IDE IRQs arriving while
-		 * the driver is still setting things up.  So, here we disable
-		 * the IRQ used by this interface while the request is being started.
-		 * This may look bad at first, but pretty much the same thing
-		 * happens anyway when any interrupt comes in, IDE or otherwise
-		 *  -- the kernel masks the IRQ while it is being handled.
-		 */
-		if (masked_irq != IDE_NO_IRQ && hwif->irq != masked_irq)
-			disable_irq_nosync(hwif->irq);
-		spin_unlock(&hwgroup->lock);
-		local_irq_enable_in_hardirq();
-			/* allow other IRQs while we start this request */
+		spin_unlock_irq(&hwgroup->lock);
 		startstop = start_request(drive, rq);
 		spin_lock_irq(&hwgroup->lock);
-		if (masked_irq != IDE_NO_IRQ && hwif->irq != masked_irq)
-			enable_irq(hwif->irq);
-		if (startstop == ide_stopped)
+
+		if (startstop == ide_stopped) {
 			hwgroup->busy = 0;
+			if (!elv_queue_empty(orig_drive->queue))
+				blk_plug_device(orig_drive->queue);
+		}
 	}
-}
+	return;
 
-/*
- * Passes the stuff to ide_do_request
- */
-void do_ide_request(struct request_queue *q)
-{
-	ide_drive_t *drive = q->queuedata;
-
-	ide_do_request(HWGROUP(drive), IDE_NO_IRQ);
+plug_device:
+	if (!elv_queue_empty(orig_drive->queue))
+		blk_plug_device(orig_drive->queue);
 }
 
 /*
@@ -1074,11 +1065,13 @@ void ide_timer_expiry (unsigned long data)
 			drive->service_time = jiffies - drive->service_start;
 			spin_lock_irq(&hwgroup->lock);
 			enable_irq(hwif->irq);
-			if (startstop == ide_stopped)
+			if (startstop == ide_stopped) {
 				hwgroup->busy = 0;
+				if (!elv_queue_empty(drive->queue))
+					blk_plug_device(drive->queue);
+			}
 		}
 	}
-	ide_do_request(hwgroup, IDE_NO_IRQ);
 	spin_unlock_irqrestore(&hwgroup->lock, flags);
 }
 
@@ -1271,11 +1264,11 @@ irqreturn_t ide_intr (int irq, void *dev_id)
 	if (startstop == ide_stopped) {
 		if (hwgroup->handler == NULL) {	/* paranoia */
 			hwgroup->busy = 0;
-			ide_do_request(hwgroup, hwif->irq);
-		} else {
-			printk(KERN_ERR "%s: ide_intr: huh? expected NULL handler "
-				"on exit\n", drive->name);
-		}
+			if (!elv_queue_empty(drive->queue))
+				blk_plug_device(drive->queue);
+		} else
+			printk(KERN_ERR "%s: %s: huh? expected NULL handler "
+					"on exit\n", __func__, drive->name);
 	}
 out_handled:
 	irq_ret = IRQ_HANDLED;
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 62fccae..968ca8f 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -32,13 +32,6 @@
 # define SUPPORT_VLB_SYNC 1
 #endif
 
-/*
- * Used to indicate "no IRQ", should be a value that cannot be an IRQ
- * number.
- */
- 
-#define IDE_NO_IRQ		(-1)
-
 typedef unsigned char	byte;	/* used everywhere */
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
ide: don't execute the next queued command from the hard-I ..., Linux Kernel Mailing ..., (Fri Jan 2, 1:03 pm)