[PATCH] IDE 107

From: Marcin Dalecki (dalecki@evision.ag)
Date: Fri Jul 26 2002 - 08:34:31 EST


- Fix "temporal anomaly" in do_ide_request pointed out by Petr
   Vandrovec. Thanks Petr!

diff -durNp -x '*.[ao]' -x '*~' -x '*.cmd' -x '*.orig' -x '*.rej' -x 'vmlinu*' -x bzImage -x bootsect -x conmakehash -x setup -x build -x asm -x config -x '.*' -x consolemap_deftbl.c -x defkeymap.c -x devlist.h -x classlist.h -x autoconf.h -x compile.h -x version.h -x System.map -x gen-devlist -x fixdep -x split-include linux-2.5.28/drivers/ide/ide.c linux/drivers/ide/ide.c
--- linux-2.5.28/drivers/ide/ide.c 2002-07-26 09:50:43.000000000 +0200
+++ linux/drivers/ide/ide.c 2002-07-26 14:30:05.000000000 +0200
@@ -517,258 +517,251 @@ void ide_stall_queue(struct ata_device *
  * Issue a new request.
  * Caller must have already done spin_lock_irqsave(channel->lock, ...)
  */
-static void do_request(struct ata_channel *channel)
+void do_ide_request(request_queue_t *q)
 {
- struct ata_channel *ch;
- struct ata_device *drive = NULL;
- unsigned int unit;
- ide_startstop_t ret;
+ struct ata_channel *channel = q->queuedata;
 
- local_irq_disable(); /* necessary paranoia */
+ while (!test_and_set_bit(IDE_BUSY, channel->active)) {
+ struct ata_channel *ch;
+ struct ata_device *drive = NULL;
+ unsigned int unit;
+ ide_startstop_t ret;
 
- /*
- * Select the next device which will be serviced. This selects
- * only between devices on the same channel, since everything
- * else will be scheduled on the queue level.
- */
+ /*
+ * Select the next device which will be serviced. This selects
+ * only between devices on the same channel, since everything
+ * else will be scheduled on the queue level.
+ */
 
- for (unit = 0; unit < MAX_DRIVES; ++unit) {
- struct ata_device *tmp = &channel->drives[unit];
+ for (unit = 0; unit < MAX_DRIVES; ++unit) {
+ struct ata_device *tmp = &channel->drives[unit];
 
- if (!tmp->present)
- continue;
+ if (!tmp->present)
+ continue;
 
- /* There are no requests pending for this device.
- */
- if (blk_queue_empty(&tmp->queue))
- continue;
+ /* There are no requests pending for this device.
+ */
+ if (blk_queue_empty(&tmp->queue))
+ continue;
 
 
- /* This device still wants to remain idle.
- */
- if (tmp->sleep && time_after(tmp->sleep, jiffies))
- continue;
+ /* This device still wants to remain idle.
+ */
+ if (tmp->sleep && time_after(tmp->sleep, jiffies))
+ continue;
 
- /* Take this device, if there is no device choosen thus
- * far or which is more urgent.
- */
- if (!drive || (tmp->sleep && (!drive->sleep || time_after(drive->sleep, tmp->sleep)))) {
- if (!blk_queue_plugged(&tmp->queue))
- drive = tmp;
+ /* Take this device, if there is no device choosen thus
+ * far or which is more urgent.
+ */
+ if (!drive || (tmp->sleep && (!drive->sleep || time_after(drive->sleep, tmp->sleep)))) {
+ if (!blk_queue_plugged(&tmp->queue))
+ drive = tmp;
+ }
                 }
- }
 
- if (!drive) {
- unsigned long sleep = 0;
+ if (!drive) {
+ unsigned long sleep = 0;
 
- for (unit = 0; unit < MAX_DRIVES; ++unit) {
- struct ata_device *tmp = &channel->drives[unit];
+ for (unit = 0; unit < MAX_DRIVES; ++unit) {
+ struct ata_device *tmp = &channel->drives[unit];
 
- if (!tmp->present)
- continue;
+ if (!tmp->present)
+ continue;
 
- /* This device is sleeping and waiting to be serviced
- * earlier than any other device we checked thus far.
- */
- if (tmp->sleep && (!sleep || time_after(sleep, tmp->sleep)))
- sleep = tmp->sleep;
- }
+ /* This device is sleeping and waiting to be serviced
+ * earlier than any other device we checked thus far.
+ */
+ if (tmp->sleep && (!sleep || time_after(sleep, tmp->sleep)))
+ sleep = tmp->sleep;
+ }
 
- if (sleep) {
- /*
- * Take a short snooze, and then wake up again. Just
- * in case there are big differences in relative
- * throughputs.. don't want to hog the cpu too much.
- */
+ if (sleep) {
+ /*
+ * Take a short snooze, and then wake up again. Just
+ * in case there are big differences in relative
+ * throughputs.. don't want to hog the cpu too much.
+ */
 
- if (time_after(jiffies, sleep - WAIT_MIN_SLEEP))
- sleep = jiffies + WAIT_MIN_SLEEP;
+ if (time_after(jiffies, sleep - WAIT_MIN_SLEEP))
+ sleep = jiffies + WAIT_MIN_SLEEP;
 #if 1
- if (timer_pending(&channel->timer))
- printk(KERN_ERR "%s: timer already active\n", __FUNCTION__);
+ if (timer_pending(&channel->timer))
+ printk(KERN_ERR "%s: timer already active\n", __FUNCTION__);
 #endif
- set_bit(IDE_SLEEP, channel->active);
- mod_timer(&channel->timer, sleep);
-
- /*
- * We purposely leave us busy while sleeping becouse we
- * are prepared to handle the IRQ from it.
- *
- * FIXME: Make sure sleeping can't interferre with
- * operations of other devices on the same channel.
- */
- } else {
- /* FIXME: use queue plugging instead of active to block
- * upper layers from stomping on us */
- /* Ugly, but how can we sleep for the lock otherwise?
- * */
+ set_bit(IDE_SLEEP, channel->active);
+ mod_timer(&channel->timer, sleep);
 
- ide_release_lock(&ide_irq_lock);/* for atari only */
- clear_bit(IDE_BUSY, channel->active);
+ /*
+ * We purposely leave us busy while sleeping becouse we
+ * are prepared to handle the IRQ from it.
+ *
+ * FIXME: Make sure sleeping can't interferre with
+ * operations of other devices on the same channel.
+ */
+ } else {
+ /* FIXME: use queue plugging instead of active to block
+ * upper layers from stomping on us */
+ /* Ugly, but how can we sleep for the lock otherwise?
+ * */
 
- /* All requests are done.
- *
- * Disable IRQs from the last drive on this channel, to
- * make sure that it wan't throw stones at us when we
- * are not prepared to take them.
- */
+ ide_release_lock(&ide_irq_lock);/* for atari only */
+ clear_bit(IDE_BUSY, channel->active);
 
- if (channel->drive && !channel->drive->using_tcq)
- ata_irq_enable(channel->drive, 0);
- }
+ /* All requests are done.
+ *
+ * Disable IRQs from the last drive on this channel, to
+ * make sure that it wan't throw stones at us when we
+ * are not prepared to take them.
+ */
 
- return;
- }
+ if (channel->drive && !channel->drive->using_tcq)
+ ata_irq_enable(channel->drive, 0);
+ }
 
- /* Remember the last drive we where acting on.
- */
- ch = drive->channel;
- ch->drive = drive;
+ return;
+ }
 
- /* Feed commands to a drive until it barfs.
- */
- do {
- struct request *rq = NULL;
- sector_t block;
+ /* Remember the last drive we where acting on.
+ */
+ ch = drive->channel;
+ ch->drive = drive;
 
- /* Abort early if we can't queue another command. for non tcq,
- * ata_can_queue is always 1 since we never get here unless the
- * drive is idle.
+ /* Feed commands to a drive until it barfs.
                  */
+ do {
+ struct request *rq = NULL;
+ sector_t block;
 
- if (!ata_can_queue(drive)) {
- if (!ata_pending_commands(drive)) {
- clear_bit(IDE_BUSY, ch->active);
- if (drive->using_tcq)
- ata_irq_enable(drive, 0);
- }
- break;
- }
+ /* Abort early if we can't queue another command. for non tcq,
+ * ata_can_queue is always 1 since we never get here unless the
+ * drive is idle.
+ */
 
- drive->sleep = 0;
+ if (!ata_can_queue(drive)) {
+ if (!ata_pending_commands(drive)) {
+ clear_bit(IDE_BUSY, ch->active);
+ if (drive->using_tcq)
+ ata_irq_enable(drive, 0);
+ }
+ break;
+ }
 
- if (test_bit(IDE_DMA, ch->active)) {
- printk(KERN_ERR "%s: error: DMA in progress...\n", drive->name);
- break;
- }
+ drive->sleep = 0;
 
- /* There's a small window between where the queue could be
- * replugged while we are in here when using tcq (in which case
- * the queue is probably empty anyways...), so check and leave
- * if appropriate. When not using tcq, this is still a severe
- * BUG!
- */
+ if (test_bit(IDE_DMA, ch->active)) {
+ printk(KERN_ERR "%s: error: DMA in progress...\n", drive->name);
+ break;
+ }
 
- if (blk_queue_plugged(&drive->queue)) {
- BUG_ON(!drive->using_tcq);
- break;
- }
+ /* There's a small window between where the queue could be
+ * replugged while we are in here when using tcq (in which case
+ * the queue is probably empty anyways...), so check and leave
+ * if appropriate. When not using tcq, this is still a severe
+ * BUG!
+ */
 
- if (!(rq = elv_next_request(&drive->queue))) {
- if (!ata_pending_commands(drive)) {
- clear_bit(IDE_BUSY, ch->active);
- if (drive->using_tcq)
- ata_irq_enable(drive, 0);
+ if (blk_queue_plugged(&drive->queue)) {
+ BUG_ON(!drive->using_tcq);
+ break;
                         }
- drive->rq = NULL;
 
- break;
- }
+ if (!(rq = elv_next_request(&drive->queue))) {
+ if (!ata_pending_commands(drive)) {
+ clear_bit(IDE_BUSY, ch->active);
+ if (drive->using_tcq)
+ ata_irq_enable(drive, 0);
+ }
+ drive->rq = NULL;
 
- /* If there are queued commands, we can't start a
- * non-fs request (really, a non-queuable command)
- * until the queue is empty.
- */
- if (!(rq->flags & REQ_CMD) && ata_pending_commands(drive))
- break;
+ break;
+ }
 
- drive->rq = rq;
+ /* If there are queued commands, we can't start a
+ * non-fs request (really, a non-queuable command)
+ * until the queue is empty.
+ */
+ if (!(rq->flags & REQ_CMD) && ata_pending_commands(drive))
+ break;
 
- spin_unlock(ch->lock);
- /* allow other IRQs while we start this request */
- local_irq_enable();
+ drive->rq = rq;
 
- /*
- * This initiates handling of a new I/O request.
- */
+ spin_unlock(ch->lock);
+ /* allow other IRQs while we start this request */
+ local_irq_enable();
 
- BUG_ON(!(rq->flags & REQ_STARTED));
+ /*
+ * This initiates handling of a new I/O request.
+ */
+
+ BUG_ON(!(rq->flags & REQ_STARTED));
 
 #ifdef DEBUG
- printk("%s: %s: current=0x%08lx\n", ch->name, __FUNCTION__, (unsigned long) rq);
+ printk("%s: %s: current=0x%08lx\n", ch->name, __FUNCTION__, (unsigned long) rq);
 #endif
 
- /* bail early if we've exceeded max_failures */
- if (drive->max_failures && (drive->failures > drive->max_failures))
- goto kill_rq;
+ /* bail early if we've exceeded max_failures */
+ if (drive->max_failures && (drive->failures > drive->max_failures))
+ goto kill_rq;
 
- block = rq->sector;
+ block = rq->sector;
 
- /* Strange disk manager remap.
- */
- if (rq->flags & REQ_CMD)
- if (drive->type == ATA_DISK || drive->type == ATA_FLOPPY)
- block += drive->sect0;
+ /* Strange disk manager remap.
+ */
+ if (rq->flags & REQ_CMD)
+ if (drive->type == ATA_DISK || drive->type == ATA_FLOPPY)
+ block += drive->sect0;
 
- /* Yecch - this will shift the entire interval, possibly killing some
- * innocent following sector.
- */
- if (block == 0 && drive->remap_0_to_1 == 1)
- block = 1; /* redirect MBR access to EZ-Drive partn table */
+ /* Yecch - this will shift the entire interval, possibly killing some
+ * innocent following sector.
+ */
+ if (block == 0 && drive->remap_0_to_1 == 1)
+ block = 1; /* redirect MBR access to EZ-Drive partn table */
 
- ata_select(drive, 0);
- ret = ata_status_poll(drive, drive->ready_stat, BUSY_STAT | DRQ_STAT,
- WAIT_READY, rq);
+ ata_select(drive, 0);
+ ret = ata_status_poll(drive, drive->ready_stat, BUSY_STAT | DRQ_STAT,
+ WAIT_READY, rq);
 
- if (ret != ATA_OP_READY) {
- printk(KERN_ERR "%s: drive not ready for command\n", drive->name);
+ if (ret != ATA_OP_READY) {
+ printk(KERN_ERR "%s: drive not ready for command\n", drive->name);
 
- goto kill_rq;
- }
+ goto kill_rq;
+ }
 
- if (!ata_ops(drive)) {
- printk(KERN_WARNING "%s: device type %d not supported\n",
- drive->name, drive->type);
- goto kill_rq;
- }
+ if (!ata_ops(drive)) {
+ printk(KERN_WARNING "%s: device type %d not supported\n",
+ drive->name, drive->type);
+ goto kill_rq;
+ }
 
- /* The normal way of execution is to pass and execute the request
- * handler down to the device type driver.
- */
+ /* The normal way of execution is to pass and execute the request
+ * handler down to the device type driver.
+ */
 
- if (ata_ops(drive)->do_request) {
- ret = ata_ops(drive)->do_request(drive, rq, block);
- } else {
+ if (ata_ops(drive)->do_request) {
+ ret = ata_ops(drive)->do_request(drive, rq, block);
+ } else {
 kill_rq:
- if (ata_ops(drive) && ata_ops(drive)->end_request)
- ata_ops(drive)->end_request(drive, rq, 0);
- else
- ata_end_request(drive, rq, 0, 0);
- ret = ATA_OP_FINISHED;
-
- }
- spin_lock_irq(ch->lock);
-
- /* continue if command started, so we are busy */
- } while (ret != ATA_OP_CONTINUES);
- /* make sure the BUSY bit is set */
- /* FIXME: perhaps there is some place where we miss to set it? */
-// set_bit(IDE_BUSY, ch->active);
-}
+ if (ata_ops(drive) && ata_ops(drive)->end_request)
+ ata_ops(drive)->end_request(drive, rq, 0);
+ else
+ ata_end_request(drive, rq, 0, 0);
+ ret = ATA_OP_FINISHED;
 
-void do_ide_request(request_queue_t *q)
-{
- struct ata_channel *ch = q->queuedata;
+ }
+ spin_lock_irq(ch->lock);
 
- while (!test_and_set_bit(IDE_BUSY, ch->active)) {
- do_request(ch);
+ /* continue if command started, so we are busy */
+ } while (ret != ATA_OP_CONTINUES);
+ /* make sure the BUSY bit is set */
+ /* FIXME: perhaps there is some place where we miss to set it? */
+ // set_bit(IDE_BUSY, ch->active);
         }
 }
 
 /*
  * This is our timeout function for all drive operations. But note that it can
  * also be invoked as a result of a "sleep" operation triggered by the
- * mod_timer() call in do_request.
+ * mod_timer() call in do_ide_request.
  *
  * FIXME: This should take a drive context instead of a channel.
  * FIXME: This should not explicitly reenter the request handling engine.
@@ -893,7 +886,8 @@ void ide_timer_expiry(unsigned long data
 
                 if (ret == ATA_OP_FINISHED) {
                         /* Reenter the request handling engine. */
- do_request(ch);
+ clear_bit(IDE_BUSY, ch->active);
+ do_ide_request(&drive->queue);
                 }
         }
         spin_unlock_irqrestore(ch->lock, flags);
@@ -1050,9 +1044,10 @@ void ata_irq_request(int irq, void *data
                  * another interrupt.
                  */
 
- if (!ch->handler)
- do_request(ch);
- else
+ if (!ch->handler) {
+ clear_bit(IDE_BUSY, ch->active);
+ do_ide_request(&drive->queue);
+ } else
                         printk("%s: %s: huh? expected NULL handler on exit\n",
                                         drive->name, __FUNCTION__);
         }

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Jul 30 2002 - 14:00:24 EST