[PATCH] 2.5.11 IDE 47

- Rewrite choose_drive() to iterate explicitely over the channels and devices
    on them. It is not performance critical to iterate over this typically quite
    small array of disks and allows us to let them act on the natural entity,
    namely the channel as well as to remove the drive->next field from struct
    ata_device.  Make the device eviction code in ide_do_request() more
    intelliglible.  Add some comments explaining the reasoning behind the code
    there.

- Now finally since the code for choosing the drive which will be serviced next
    is intelliglibly it became obvious that the attempt to choose the next drive
    based on the duration of the last request was entierly bogous. (Because for
    example wakeups can take a long time, but this doesn't indicate that the
    drive is slow.) Remove this criterium and the corresponding accounting
    therefore. Threat all drives fairly right now.

Surprise surprise the overall system throughput increased :-).

drivers/ide/ide-probe.c

@@ -438,10 +438,11 @@ static inline void probe_for_drive (ide_drive_t *drive)
  * This routine only knows how to look for drive units 0 and 1
  * on an interface, so any setting of MAX_DRIVES > 2 won't work here.
  */
-static void probe_hwif(struct ata_channel *ch)
+static void channel_probe(struct ata_channel *ch)
 {
 	unsigned int unit;
 	unsigned long flags;
+	int error;
 
 	if (ch->noprobe)
 		return;
@@ -454,19 +455,20 @@ static void probe_hwif(struct ata_channel *ch)
 	/*
 	 * Check for the presence of a channel by probing for drives on it.
 	 */
-
 	for (unit = 0; unit < MAX_DRIVES; ++unit) {
 		struct ata_device *drive = &ch->drives[unit];
 
 		probe_for_drive(drive);
 
-		if (drive->present && !ch->present) {
+		/* drive found, there is a channel it is attached too. */
+		if (drive->present)
 			ch->present = 1;
 	}
-	}
 
-	if (ch->present) {
-		int error = 0;
+	if (!ch->present)
+		goto not_found;
+
+	error = 0;
 
 	if (((unsigned long)ch->io_ports[IDE_DATA_OFFSET] | 7) ==
 			((unsigned long)ch->io_ports[IDE_STATUS_OFFSET])) {
@@ -498,24 +500,26 @@ static void probe_hwif(struct ata_channel *ch)
 		error += !request_region(ch->io_ports[IDE_IRQ_OFFSET], 1, ch->name);
 #endif
 
-		/* Some neccessary register area was already used. Skip this
-		 * device.
+	/* Some neccessary register area was already used. Skip this device.
 	 */
+
 	if (
 #if CONFIG_BLK_DEV_PDC4030
 			(ch->chipset != ide_pdc4030 || ch->unit == 0) &&
 #endif
 			error) {
-			/* FIXME: We should be dealing properly with partial IO
-			 * region allocations here.
+
+		/* FIXME: We should be dealing properly with partial IO region
+		 * allocations here.
 		 */
+
 		ch->present = 0;
 		printk("%s: error: ports already in use!\n", ch->name);
-
-		}
 	}
 
-	if (ch->present) {
+	if (!ch->present)
+		goto not_found;
+
 	/* Register this hardware interface within the global device tree.
 	 */
 	sprintf(ch->dev.bus_id, "%04x", ch->io_ports[IDE_DATA_OFFSET]);
@@ -542,8 +546,6 @@ static void probe_hwif(struct ata_channel *ch)
 			ide_delay_50ms();
 			stat = IN_BYTE(ch->io_ports[IDE_STATUS_OFFSET]);
 		} while ((stat & BUSY_STAT) && 0 < (signed long)(timeout - jiffies));
-
-		}
 	}
 
 	__restore_flags(flags);	/* local CPU only */
@@ -551,7 +553,6 @@ static void probe_hwif(struct ata_channel *ch)
 	/*
 	 * Now setup the PIO transfer modes of the drives on this channel.
 	 */
-	if (ch->present) {
 	for (unit = 0; unit < MAX_DRIVES; ++unit) {
 		struct ata_device *drive = &ch->drives[unit];
 
@@ -560,7 +561,11 @@ static void probe_hwif(struct ata_channel *ch)
 				drive->channel->tuneproc(drive, 255);	/* auto-tune PIO mode */
 		}
 	}
-	}
+
+	return;
+
+not_found:
+	__restore_flags(flags);
 }
 
 /*
@@ -682,12 +687,7 @@ static int init_irq(struct ata_channel *ch)
 			spin_unlock_irqrestore(&ide_lock, flags);
 			return 1;
 		}
-		memset(hwgroup, 0, sizeof(ide_hwgroup_t));
-		hwgroup->rq       = NULL;
-		hwgroup->handler  = NULL;
-		hwgroup->drive    = NULL;
-		hwgroup->flags	  = 0;
-
+		memset(hwgroup, 0, sizeof(*hwgroup));
 		init_timer(&hwgroup->timer);
 		hwgroup->timer.function = &ide_timer_expiry;
 		hwgroup->timer.data = (unsigned long) hwgroup;
@@ -716,21 +716,17 @@ static int init_irq(struct ata_channel *ch)
 	}
 
 	/*
-	 * Everything is okay.
+	 * Everything is okay. Tag us as member of this hardware group.
 	 */
 	ch->hwgroup = hwgroup;
-
 	for (i = 0; i < MAX_DRIVES; ++i) {
 		struct ata_device *drive = &ch->drives[i];
 
 		if (!drive->present)
 			continue;
 
-		if (!hwgroup->drive)
-			hwgroup->drive = drive;
-
-		drive->next = hwgroup->drive->next;
-		hwgroup->drive->next = drive;
+		if (!hwgroup->XXX_drive)
+			hwgroup->XXX_drive = drive;
 
 		init_device_queue(drive);
 	}
@@ -833,58 +829,65 @@ static void init_gendisk(struct ata_channel *hwif)
 	return;
 }
 
-static int hwif_init(struct ata_channel *hwif)
+static void channel_init(struct ata_channel *ch)
 {
-	if (!hwif->present)
-		return 0;
-	if (!hwif->irq) {
-		if (!(hwif->irq = ide_default_irq(hwif->io_ports[IDE_DATA_OFFSET])))
-		{
-			printk("%s: DISABLED, NO IRQ\n", hwif->name);
-			return (hwif->present = 0);
+	if (!ch->present)
+		return;
+
+	/* we set it back to 1 if all is ok below */
+	ch->present = 0;
+
+	if (!ch->irq) {
+		if (!(ch->irq = ide_default_irq(ch->io_ports[IDE_DATA_OFFSET]))) {
+			printk("%s: DISABLED, NO IRQ\n", ch->name);
+
+			return;
 		}
 	}
 #ifdef CONFIG_BLK_DEV_HD
-	if (hwif->irq == HD_IRQ && hwif->io_ports[IDE_DATA_OFFSET] != HD_DATA) {
-		printk("%s: CANNOT SHARE IRQ WITH OLD HARDDISK DRIVER (hd.c)\n", hwif->name);
-		return (hwif->present = 0);
+	if (ch->irq == HD_IRQ && ch->io_ports[IDE_DATA_OFFSET] != HD_DATA) {
+		printk("%s: CANNOT SHARE IRQ WITH OLD HARDDISK DRIVER (hd.c)\n", ch->name);
+
+		return;
 	}
 #endif
 
-	hwif->present = 0; /* we set it back to 1 if all is ok below */
+	if (devfs_register_blkdev (ch->major, ch->name, ide_fops)) {
+		printk("%s: UNABLE TO GET MAJOR NUMBER %d\n", ch->name, ch->major);
 
-	if (devfs_register_blkdev (hwif->major, hwif->name, ide_fops)) {
-		printk("%s: UNABLE TO GET MAJOR NUMBER %d\n", hwif->name, hwif->major);
-		return (hwif->present = 0);
+		return;
 	}
 
-	if (init_irq(hwif)) {
-		int i = hwif->irq;
+	if (init_irq(ch)) {
+		int irq = ch->irq;
 		/*
 		 * It failed to initialise. Find the default IRQ for
 		 * this port and try that.
 		 */
-		if (!(hwif->irq = ide_default_irq(hwif->io_ports[IDE_DATA_OFFSET]))) {
-			printk("%s: Disabled unable to get IRQ %d.\n", hwif->name, i);
-			(void) unregister_blkdev (hwif->major, hwif->name);
-			return (hwif->present = 0);
+		if (!(ch->irq = ide_default_irq(ch->io_ports[IDE_DATA_OFFSET]))) {
+			printk(KERN_INFO "%s: disabled; unable to get IRQ %d.\n", ch->name, irq);
+			(void) unregister_blkdev (ch->major, ch->name);
+
+			return;
 		}
-		if (init_irq(hwif)) {
-			printk("%s: probed IRQ %d and default IRQ %d failed.\n",
-				hwif->name, i, hwif->irq);
-			(void) unregister_blkdev (hwif->major, hwif->name);
-			return (hwif->present = 0);
+		if (init_irq(ch)) {
+			printk(KERN_INFO "%s: probed IRQ %d and default IRQ %d failed.\n",
+				ch->name, irq, ch->irq);
+			(void) unregister_blkdev(ch->major, ch->name);
+
+			return;
 		}
-		printk("%s: probed IRQ %d failed, using default.\n",
-			hwif->name, hwif->irq);
+		printk(KERN_INFO "%s: probed IRQ %d failed, using default.\n", ch->name, ch->irq);
 	}
 
-	init_gendisk(hwif);
-	blk_dev[hwif->major].data = hwif;
-	blk_dev[hwif->major].queue = ide_get_queue;
-	hwif->present = 1;	/* success */
+	init_gendisk(ch);
+	blk_dev[ch->major].data = ch;
+	blk_dev[ch->major].queue = ide_get_queue;
+
+	/* all went well, flag this channel entry as valid */
+	ch->present = 1;
 
-	return hwif->present;
+	return;
 }
 
 int ideprobe_init (void)
@@ -901,9 +904,9 @@ int ideprobe_init (void)
 	 */
 	for (index = 0; index < MAX_HWIFS; ++index)
 		if (probe[index])
-			probe_hwif(&ide_hwifs[index]);
+			channel_probe(&ide_hwifs[index]);
 	for (index = 0; index < MAX_HWIFS; ++index)
 		if (probe[index])
-			hwif_init(&ide_hwifs[index]);
+			channel_init(&ide_hwifs[index]);
 	return 0;
 }

include/linux/ide.h

@@ -283,16 +283,10 @@ struct ata_device {
 	 */
 	request_queue_t	queue;	/* per device request queue */
 
-	struct ata_device	*next;	/* circular list of hwgroup drives */
-
 	/* Those are directly injected jiffie values. They should go away and
 	 * we should use generic timers instead!!!
 	 */
-
 	unsigned long PADAM_sleep;	/* sleep until this time */
-	unsigned long PADAM_service_start;	/* time we started last request */
-	unsigned long PADAM_service_time;	/* service time of last request */
-	unsigned long PADAM_timeout;		/* max time to wait for irq */
 
 	/* Flags requesting/indicating one of the following special commands
 	 * executed on the request queue.
@@ -512,7 +506,7 @@ struct ata_taskfile;
 typedef struct hwgroup_s {
 	ide_startstop_t (*handler)(struct ata_device *, struct request *);	/* irq handler, if active */
 	unsigned long flags;		/* BUSY, SLEEPING */
-	struct ata_device *drive;	/* current drive */
+	struct ata_device *XXX_drive;	/* current drive */
 	struct request *rq;		/* current request */
 	struct timer_list timer;	/* failsafe timer */
 	unsigned long poll_timeout;	/* timeout value during long polls */