[PATCH] barrier rework updates

As promised to Andrew, here are the latest bits that fixup the block io
barrier handling.

- Add io scheduler ->deactivate hook to tell the io scheduler is a
  request is suspended from the block layer. cfq and as needs this hook.

- Locking updates

- Make sure a driver doesn't reuse the flush rq before a previous one
  has completed

- Typo in the scsi_io_completion() function, the bit shift was wrong

- sd needs proper timeout on the flush

- remove silly debug leftover in ide-disk wrt "hdc"

Signed-off-by: Jens Axboe <axboe@suse.de>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

9 files changed, 91 insertions, 47 deletions

diff --git a/drivers/block/as-iosched.c b/drivers/block/as-iosched.c
index fb625b49b831..17f02aa95271 100644
--- a/drivers/block/as-iosched.c
+++ b/drivers/block/as-iosched.c
@@ -1463,35 +1463,36 @@ static void as_add_request(struct as_data *ad, struct as_rq *arq)
 	arq->state = AS_RQ_QUEUED;
 }
 
-/*
- * requeue the request. The request has not been completed, nor is it a
- * new request, so don't touch accounting.
- */
-static void as_requeue_request(request_queue_t *q, struct request *rq)
+static void as_deactivate_request(request_queue_t *q, struct request *rq)
 {
 	struct as_data *ad = q->elevator->elevator_data;
 	struct as_rq *arq = RQ_DATA(rq);
 
 	if (arq) {
-		if (arq->state != AS_RQ_REMOVED) {
-			printk("arq->state %d\n", arq->state);
-			WARN_ON(1);
+		if (arq->state == AS_RQ_REMOVED) {
+			arq->state = AS_RQ_DISPATCHED;
+			if (arq->io_context && arq->io_context->aic)
+				atomic_inc(&arq->io_context->aic->nr_dispatched);
 		}
-
-		arq->state = AS_RQ_DISPATCHED;
-		if (arq->io_context && arq->io_context->aic)
-			atomic_inc(&arq->io_context->aic->nr_dispatched);
 	} else
 		WARN_ON(blk_fs_request(rq)
 			&& (!(rq->flags & (REQ_HARDBARRIER|REQ_SOFTBARRIER))) );
 
-	list_add(&rq->queuelist, ad->dispatch);
-
 	/* Stop anticipating - let this request get through */
 	as_antic_stop(ad);
 }
 
 /*
+ * requeue the request. The request has not been completed, nor is it a
+ * new request, so don't touch accounting.
+ */
+static void as_requeue_request(request_queue_t *q, struct request *rq)
+{
+	as_deactivate_request(q, rq);
+	list_add(&rq->queuelist, &q->queue_head);
+}
+
+/*
  * Account a request that is inserted directly onto the dispatch queue.
  * arq->io_context->aic->nr_dispatched should not need to be incremented
  * because only new requests should come through here: requeues go through
@@ -2080,6 +2081,7 @@ static struct elevator_type iosched_as = {
 		.elevator_add_req_fn =		as_insert_request,
 		.elevator_remove_req_fn =	as_remove_request,
 		.elevator_requeue_req_fn = 	as_requeue_request,
+		.elevator_deactivate_req_fn = 	as_deactivate_request,
 		.elevator_queue_empty_fn =	as_queue_empty,
 		.elevator_completed_req_fn =	as_completed_request,
 		.elevator_former_req_fn =	as_former_request,
diff --git a/drivers/block/cfq-iosched.c b/drivers/block/cfq-iosched.c
index 4234508565d6..d9aaa42500d0 100644
--- a/drivers/block/cfq-iosched.c
+++ b/drivers/block/cfq-iosched.c
@@ -607,10 +607,7 @@ out:
 	return NULL;
 }
 
-/*
- * make sure the service time gets corrected on reissue of this request
- */
-static void cfq_requeue_request(request_queue_t *q, struct request *rq)
+static void cfq_deactivate_request(request_queue_t *q, struct request *rq)
 {
 	struct cfq_rq *crq = RQ_DATA(rq);
 
@@ -627,6 +624,14 @@ static void cfq_requeue_request(request_queue_t *q, struct request *rq)
 			cfqq->cfqd->rq_in_driver--;
 		}
 	}
+}
+
+/*
+ * make sure the service time gets corrected on reissue of this request
+ */
+static void cfq_requeue_request(request_queue_t *q, struct request *rq)
+{
+	cfq_deactivate_request(q, rq);
 	list_add(&rq->queuelist, &q->queue_head);
 }
 
@@ -1804,6 +1809,7 @@ static struct elevator_type iosched_cfq = {
 		.elevator_add_req_fn =		cfq_insert_request,
 		.elevator_remove_req_fn =	cfq_remove_request,
 		.elevator_requeue_req_fn =	cfq_requeue_request,
+		.elevator_deactivate_req_fn =	cfq_deactivate_request,
 		.elevator_queue_empty_fn =	cfq_queue_empty,
 		.elevator_completed_req_fn =	cfq_completed_request,
 		.elevator_former_req_fn =	cfq_former_request,
diff --git a/drivers/block/elevator.c b/drivers/block/elevator.c
index 241fdbcb29bc..85c72dd3dfee 100644
--- a/drivers/block/elevator.c
+++ b/drivers/block/elevator.c
@@ -255,8 +255,16 @@ void elv_merge_requests(request_queue_t *q, struct request *rq,
 		e->ops->elevator_merge_req_fn(q, rq, next);
 }
 
-void elv_requeue_request(request_queue_t *q, struct request *rq)
+/*
+ * For careful internal use by the block layer. Essentially the same as
+ * a requeue in that it tells the io scheduler that this request is not
+ * active in the driver or hardware anymore, but we don't want the request
+ * added back to the scheduler. Function is not exported.
+ */
+void elv_deactivate_request(request_queue_t *q, struct request *rq)
 {
+	elevator_t *e = q->elevator;
+
 	/*
 	 * it already went through dequeue, we need to decrement the
 	 * in_flight count again
@@ -264,6 +272,24 @@ void elv_requeue_request(request_queue_t *q, struct request *rq)
 	if (blk_account_rq(rq))
 		q->in_flight--;
 
+	rq->flags &= ~REQ_STARTED;
+
+	if (e->ops->elevator_deactivate_req_fn)
+		e->ops->elevator_deactivate_req_fn(q, rq);
+}
+
+void elv_requeue_request(request_queue_t *q, struct request *rq)
+{
+	elv_deactivate_request(q, rq);
+
+	/*
+	 * if this is the flush, requeue the original instead and drop the flush
+	 */
+	if (rq->flags & REQ_BAR_FLUSH) {
+		clear_bit(QUEUE_FLAG_FLUSH, &q->queue_flags);
+		rq = rq->end_io_data;
+	}
+
 	/*
 	 * if iosched has an explicit requeue hook, then use that. otherwise
 	 * just put the request at the front of the queue
diff --git a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c
index 28977a7bad16..0135ec79dc83 100644
--- a/drivers/block/ll_rw_blk.c
+++ b/drivers/block/ll_rw_blk.c
@@ -356,6 +356,7 @@ static void blk_pre_flush_end_io(struct request *flush_rq)
 	else {
 		q->end_flush_fn(q, flush_rq);
 		clear_bit(QUEUE_FLAG_FLUSH, &q->queue_flags);
+		q->request_fn(q);
 	}
 }
 
@@ -366,15 +367,9 @@ static void blk_post_flush_end_io(struct request *flush_rq)
 
 	rq->flags |= REQ_BAR_POSTFLUSH;
 
-	/*
-	 * called from end_that_request_last(), so we know that the queue
-	 * lock is held
-	 */
-	spin_unlock(q->queue_lock);
 	q->end_flush_fn(q, flush_rq);
-	spin_lock(q->queue_lock);
-
 	clear_bit(QUEUE_FLAG_FLUSH, &q->queue_flags);
+	q->request_fn(q);
 }
 
 struct request *blk_start_pre_flush(request_queue_t *q, struct request *rq)
@@ -383,9 +378,12 @@ struct request *blk_start_pre_flush(request_queue_t *q, struct request *rq)
 
 	BUG_ON(!blk_barrier_rq(rq));
 
+	if (test_and_set_bit(QUEUE_FLAG_FLUSH, &q->queue_flags))
+		return NULL;
+
 	rq_init(q, flush_rq);
 	flush_rq->elevator_private = NULL;
-	flush_rq->flags = 0;
+	flush_rq->flags = REQ_BAR_FLUSH;
 	flush_rq->rq_disk = rq->rq_disk;
 	flush_rq->rl = NULL;
 
@@ -395,11 +393,10 @@ struct request *blk_start_pre_flush(request_queue_t *q, struct request *rq)
 	 */
 	if (!q->prepare_flush_fn(q, flush_rq)) {
 		rq->flags |= REQ_BAR_PREFLUSH | REQ_BAR_POSTFLUSH;
+		clear_bit(QUEUE_FLAG_FLUSH, &q->queue_flags);
 		return rq;
 	}
 
-	set_bit(QUEUE_FLAG_FLUSH, &q->queue_flags);
-
 	/*
 	 * some drivers dequeue requests right away, some only after io
 	 * completion. make sure the request is dequeued.
@@ -407,6 +404,8 @@ struct request *blk_start_pre_flush(request_queue_t *q, struct request *rq)
 	if (!list_empty(&rq->queuelist))
 		blkdev_dequeue_request(rq);
 
+	elv_deactivate_request(q, rq);
+
 	flush_rq->end_io_data = rq;
 	flush_rq->end_io = blk_pre_flush_end_io;
 
@@ -422,7 +421,7 @@ static void blk_start_post_flush(request_queue_t *q, struct request *rq)
 
 	rq_init(q, flush_rq);
 	flush_rq->elevator_private = NULL;
-	flush_rq->flags = 0;
+	flush_rq->flags = REQ_BAR_FLUSH;
 	flush_rq->rq_disk = rq->rq_disk;
 	flush_rq->rl = NULL;
 
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 36b90f85968c..c256feebd1bd 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -712,12 +712,13 @@ static void idedisk_end_flush(request_queue_t *q, struct request *flush_rq)
 		blk_queue_issue_flush_fn(drive->queue, NULL);
 		good_sectors = 0;
 	} else if (flush_rq->errors) {
-		sector = ide_get_error_location(drive, flush_rq->buffer);
-		if ((sector >= rq->hard_sector) &&
-		    (sector < rq->hard_sector + rq->hard_nr_sectors))
-			good_sectors = sector - rq->hard_sector;
-		else
-			good_sectors = 0;
+		good_sectors = 0;
+		if (blk_barrier_preflush(rq)) {
+			sector = ide_get_error_location(drive,flush_rq->buffer);
+			if ((sector >= rq->hard_sector) &&
+			    (sector < rq->hard_sector + rq->hard_nr_sectors))
+				good_sectors = sector - rq->hard_sector;
+		}
 	}
 
 	if (flush_rq->errors)
@@ -729,14 +730,10 @@ static void idedisk_end_flush(request_queue_t *q, struct request *flush_rq)
 
 	bad_sectors = rq->hard_nr_sectors - good_sectors;
 
-	spin_lock(&ide_lock);
-
 	if (good_sectors)
 		__ide_end_request(drive, rq, 1, good_sectors);
 	if (bad_sectors)
 		__ide_end_request(drive, rq, 0, bad_sectors);
-
-	spin_unlock(&ide_lock);
 }
 
 static int idedisk_prepare_flush(request_queue_t *q, struct request *rq)
@@ -1150,9 +1147,6 @@ static void idedisk_setup (ide_drive_t *drive)
 			barrier = 0;
 	}
 
-	if (!strncmp(drive->name, "hdc", 3))
-		barrier = 1;
-
 	printk(KERN_INFO "%s: cache flushes %ssupported\n",
 		drive->name, barrier ? "" : "not ");
 	if (barrier) {
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e8405375877f..da63045c165d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -697,7 +697,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
 	int sense_valid = 0;
 	int sense_deferred = 0;
 
-	if (blk_complete_barrier_rq(q, req, good_bytes << 9))
+	if (blk_complete_barrier_rq(q, req, good_bytes >> 9))
 		return;
 
 	/*
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index fe8a079b00fb..0fb36703292f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -745,10 +745,21 @@ static void sd_end_flush(request_queue_t *q, struct request *flush_rq)
 	struct scsi_cmnd *cmd = rq->special;
 	unsigned int bytes = rq->hard_nr_sectors << 9;
 
-	if (!flush_rq->errors)
+	if (!flush_rq->errors) {
+		spin_unlock(q->queue_lock);
 		scsi_io_completion(cmd, bytes, 0);
-	else
+		spin_lock(q->queue_lock);
+	} else if (blk_barrier_postflush(rq)) {
+		spin_unlock(q->queue_lock);
 		scsi_io_completion(cmd, 0, bytes);
+		spin_lock(q->queue_lock);
+	} else {
+		/*
+		 * force journal abort of barriers
+		 */
+		end_that_request_first(rq, -EOPNOTSUPP, rq->hard_nr_sectors);
+		end_that_request_last(rq);
+	}
 }
 
 static int sd_prepare_flush(request_queue_t *q, struct request *rq)
@@ -758,7 +769,8 @@ static int sd_prepare_flush(request_queue_t *q, struct request *rq)
 
 	if (sdkp->WCE) {
 		memset(rq->cmd, 0, sizeof(rq->cmd));
-		rq->flags = REQ_BLOCK_PC | REQ_SOFTBARRIER;
+		rq->flags |= REQ_BLOCK_PC | REQ_SOFTBARRIER;
+		rq->timeout = SD_TIMEOUT;
 		rq->cmd[0] = SYNCHRONIZE_CACHE;
 		return 1;
 	}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 83eef4fde873..266b44fcfaa0 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -219,6 +219,7 @@ enum rq_flag_bits {
 	__REQ_PM_SHUTDOWN,	/* shutdown request */
 	__REQ_BAR_PREFLUSH,	/* barrier pre-flush done */
 	__REQ_BAR_POSTFLUSH,	/* barrier post-flush */
+	__REQ_BAR_FLUSH,	/* rq is the flush request */
 	__REQ_NR_BITS,		/* stops here */
 };
 
@@ -246,6 +247,7 @@ enum rq_flag_bits {
 #define REQ_PM_SHUTDOWN	(1 << __REQ_PM_SHUTDOWN)
 #define REQ_BAR_PREFLUSH	(1 << __REQ_BAR_PREFLUSH)
 #define REQ_BAR_POSTFLUSH	(1 << __REQ_BAR_POSTFLUSH)
+#define REQ_BAR_FLUSH	(1 << __REQ_BAR_FLUSH)
 
 /*
  * State information carried for REQ_PM_SUSPEND and REQ_PM_RESUME
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 8cf0e3f290bf..ee54f81faad5 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -20,6 +20,7 @@ typedef int (elevator_may_queue_fn) (request_queue_t *, int);
 
 typedef int (elevator_set_req_fn) (request_queue_t *, struct request *, int);
 typedef void (elevator_put_req_fn) (request_queue_t *, struct request *);
+typedef void (elevator_deactivate_req_fn) (request_queue_t *, struct request *);
 
 typedef int (elevator_init_fn) (request_queue_t *, elevator_t *);
 typedef void (elevator_exit_fn) (elevator_t *);
@@ -34,6 +35,7 @@ struct elevator_ops
 	elevator_add_req_fn *elevator_add_req_fn;
 	elevator_remove_req_fn *elevator_remove_req_fn;
 	elevator_requeue_req_fn *elevator_requeue_req_fn;
+	elevator_deactivate_req_fn *elevator_deactivate_req_fn;
 
 	elevator_queue_empty_fn *elevator_queue_empty_fn;
 	elevator_completed_req_fn *elevator_completed_req_fn;
@@ -87,6 +89,7 @@ extern void elv_merge_requests(request_queue_t *, struct request *,
 extern void elv_merged_request(request_queue_t *, struct request *);
 extern void elv_remove_request(request_queue_t *, struct request *);
 extern void elv_requeue_request(request_queue_t *, struct request *);
+extern void elv_deactivate_request(request_queue_t *, struct request *);
 extern int elv_queue_empty(request_queue_t *);
 extern struct request *elv_next_request(struct request_queue *q);
 extern struct request *elv_former_request(request_queue_t *, struct request *);