Re: Possibly wrong BIO usage in ide_multwrite

[Christophe Saout]

On Mon, Jan 05, 2004 at 06:08:49PM +0100, Bartlomiej Zolnierkiewicz wrote:

> On Monday 05 of January 2004 04:52, Christophe Saout wrote:
> 
> > BTW, what was ide_multwrite expected to return? These if clauses in
> > multwrite_intr are never executed.
> 
> Dunno.  It can't fail so it should be made void.

Ok, done.

> Please also add bio->bi_idx restoring for failed requests.
> Put it before DRIVER(drive)->error()

Whoops, overlooked this one. Right.

> (and remember about if (bio) check).

Remember? Can bio be NULL somewhere? Or what do you mean? It's our
scratchpad and ide_multwrite never puts a NULL bio on it.

> Otherwise I patch is OK for me.

Ok, take two.

I also did legacy/pdc4030.c, it's more or less the same though I'm not
able to test it.


--- linux.orig/drivers/ide/ide-disk.c	2004-01-04 23:29:01.000000000 +0100
+++ linux/drivers/ide/ide-disk.c	2004-01-05 23:35:35.258199832 +0100
@@ -251,7 +251,7 @@
  * is shorter or smaller than the BH segment then we should be OKAY.
  * This is only valid if we can rewind the rq->current_nr_sectors counter.
  */
-int ide_multwrite (ide_drive_t *drive, unsigned int mcount)
+void ide_multwrite (ide_drive_t *drive, unsigned int mcount)
 {
  	ide_hwgroup_t *hwgroup	= HWGROUP(drive);
  	struct request *rq	= &hwgroup->wrq;
@@ -279,7 +279,7 @@
 			 * all bvecs in this one.
 			 */
 			if (++bio->bi_idx >= bio->bi_vcnt) {
-				bio->bi_idx = 0;
+				bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
 				bio = bio->bi_next;
 			}
 
@@ -288,7 +288,8 @@
 				mcount = 0;
 			} else {
 				rq->bio = bio;
-				rq->current_nr_sectors = bio_iovec(bio)->bv_len >> 9;
+				rq->nr_cbio_segments = bio_segments(bio);
+				rq->current_nr_sectors = bio_cur_sectors(bio);
 				rq->hard_cur_sectors = rq->current_nr_sectors;
 			}
 		}
@@ -300,8 +301,6 @@
 		taskfile_output_data(drive, buffer, nsect<<7);
 		ide_unmap_buffer(rq, buffer, &flags);
 	} while (mcount);
-
-        return 0;
 }
 
 /*
@@ -312,6 +311,7 @@
 	ide_hwgroup_t *hwgroup	= HWGROUP(drive);
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct request *rq	= &hwgroup->wrq;
+	struct bio *bio		= rq->bio;
 	u8 stat;
 
 	stat = hwif->INB(IDE_STATUS_REG);
@@ -322,8 +322,7 @@
 			 *	of the request
 			 */
 			if (rq->nr_sectors) {
-				if (ide_multwrite(drive, drive->mult_count))
-					return ide_stopped;
+				ide_multwrite(drive, drive->mult_count);
 				ide_set_handler(drive, &multwrite_intr, WAIT_CMD, NULL);
 				return ide_started;
 			}
@@ -333,14 +332,17 @@
 			 *	we can end the original request.
 			 */
 			if (!rq->nr_sectors) {	/* all done? */
+				bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
 				rq = hwgroup->rq;
 				ide_end_request(drive, 1, rq->nr_sectors);
 				return ide_stopped;
 			}
 		}
 		/* the original code did this here (?) */
+		bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
 		return ide_stopped;
 	}
+	bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
 	return DRIVER(drive)->error(drive, "multwrite_intr", stat);
 }
 
@@ -519,14 +521,7 @@
 	 */
 			hwgroup->wrq = *rq; /* scratchpad */
 			ide_set_handler(drive, &multwrite_intr, WAIT_CMD, NULL);
-			if (ide_multwrite(drive, drive->mult_count)) {
-				unsigned long flags;
-				spin_lock_irqsave(&ide_lock, flags);
-				hwgroup->handler = NULL;
-				del_timer(&hwgroup->timer);
-				spin_unlock_irqrestore(&ide_lock, flags);
-				return ide_stopped;
-			}
+			ide_multwrite(drive, drive->mult_count);
 		} else {
 			unsigned long flags;
 			char *to = ide_map_buffer(rq, &flags);
--- linux.orig/drivers/ide/legacy/pdc4030.c	2004-01-05 20:34:29.000000000 +0100
+++ linux/drivers/ide/legacy/pdc4030.c	2004-01-05 23:34:41.895312224 +0100
@@ -443,7 +443,12 @@
 static ide_startstop_t promise_complete_pollfunc(ide_drive_t *drive)
 {
 	ide_hwgroup_t *hwgroup = HWGROUP(drive);
+#ifdef CONFIG_IDE_TASKFILE_IO
 	struct request *rq = hwgroup->rq;
+#else
+	struct request *rq = &hwgroup->wrq;
+	struct bio *bio = rq->bio;
+#endif
 
 	if ((HWIF(drive)->INB(IDE_STATUS_REG)) & BUSY_STAT) {
 		if (time_before(jiffies, hwgroup->poll_timeout)) {
@@ -472,6 +477,8 @@
 	while (rq->bio != rq->cbio)
 		(void) DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->bio));
 #else
+	bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
+	rq = hwgroup->rq;
 	DRIVER(drive)->end_request(drive, 1, rq->hard_nr_sectors);
 #endif
 	return ide_stopped;
@@ -530,7 +537,7 @@
 			 * all bvecs in this one.
 			 */
 			if (++bio->bi_idx >= bio->bi_vcnt) {
-				bio->bi_idx = 0;
+				bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
 				bio = bio->bi_next;
 			}
 
@@ -539,7 +546,8 @@
 				mcount = 0;
 			} else {
 				rq->bio = bio;
-				rq->current_nr_sectors = bio_iovec(bio)->bv_len >> 9;
+				rq->nr_cbio_segments = bio_segments(bio);
+				rq->current_nr_sectors = bio_cur_sectors(bio);
 				rq->hard_cur_sectors = rq->current_nr_sectors;
 			}
 		}
@@ -561,6 +569,9 @@
 	ide_hwgroup_t *hwgroup = HWGROUP(drive);
 #ifdef CONFIG_IDE_TASKFILE_IO
 	struct request *rq = hwgroup->rq;
+#else
+	struct request *rq = &hwgroup->wrq;
+	struct bio *bio = rq->bio;
 #endif
 
 	if (HWIF(drive)->INB(IDE_NSECTOR_REG) != 0) {
@@ -575,6 +586,9 @@
 		}
 		hwgroup->poll_timeout = 0;
 		printk(KERN_ERR "%s: write timed-out!\n",drive->name);
+#ifndef CONFIG_IDE_TASKFILE_IO
+		bio->bi_idx = bio->bi_vcnt - rq->nr_cbio_segments;
+#endif
 		return DRIVER(drive)->error(drive, "write timeout",
 				HWIF(drive)->INB(IDE_STATUS_REG));
 	}

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