All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] remove q->prepare_flush_fn hook
@ 2010-07-03  8:45 FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 1/9] block: introduce REQ_FLUSH flag FUJITA Tomonori
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori

This removes q->prepare_flush_fn. Except for ide and scsi, all the
users just mark flush requests. They can use REQ_FLUSH flag
instead.

SCSI and ide can use q->prep_rq_fn to build flush requests.

This can be applied to the block's for-2.6.36.

=
 block/blk-barrier.c          |   14 +---------
 drivers/block/brd.c          |    2 +-
 drivers/block/loop.c         |    2 +-
 drivers/block/osdblk.c       |   12 +--------
 drivers/block/ps3disk.c      |   25 +++++---------------
 drivers/block/virtio_blk.c   |   52 +++++++++++++++++------------------------
 drivers/block/xen-blkfront.c |    3 +-
 drivers/ide/ide-disk.c       |   16 +++++++++----
 drivers/md/dm.c              |   16 +-----------
 drivers/mmc/card/queue.c     |    2 +-
 drivers/s390/block/dasd.c    |    2 +-
 drivers/scsi/sd.c            |   26 +++++++++++++--------
 include/linux/bio.h          |    2 +
 include/linux/blkdev.h       |    4 +--
 14 files changed, 69 insertions(+), 109 deletions(-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] block: introduce REQ_FLUSH flag
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn FUJITA Tomonori
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe
  Cc: hch, linux-kernel, fujita.tomonori, James Bottomley,
	David S. Miller, Rusty Russell, Alasdair G Kergon

SCSI-ml needs a way to mark a request as flush request in
q->prepare_flush_fn because it needs to identify them later (e.g. in
q->request_fn or prep_rq_fn).

queue_flush sets REQ_HARDBARRIER in rq->cmd_flags however the block
layer also sends normal REQ_TYPE_FS requests with REQ_HARDBARRIER. So
SCSI-ml can't use REQ_HARDBARRIER to identify flush requests.

We could change the block layer to clear REQ_HARDBARRIER bit before
sending non flush requests to the lower layers. However, intorudcing
the new flag looks cleaner (surely easier).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: James Bottomley <James.Bottomley@suse.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 block/blk-barrier.c |    2 +-
 include/linux/bio.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 7c6f4a7..a348242 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -143,7 +143,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	}
 
 	blk_rq_init(q, rq);
-	rq->cmd_flags = REQ_HARDBARRIER;
+	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->bar_rq.rq_disk;
 	rq->end_io = end_io;
 	q->prepare_flush_fn(q, rq);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4d379c8..f655b54 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -174,6 +174,7 @@ enum rq_flag_bits {
 	__REQ_ALLOCED,		/* request came from our alloc pool */
 	__REQ_COPY_USER,	/* contains copies of user pages */
 	__REQ_INTEGRITY,	/* integrity metadata has been remapped */
+	__REQ_FLUSH,		/* request for cache flush */
 	__REQ_IO_STAT,		/* account I/O stat */
 	__REQ_MIXED_MERGE,	/* merge of different types, fail separately */
 	__REQ_NR_BITS,		/* stops here */
@@ -213,6 +214,7 @@ enum rq_flag_bits {
 #define REQ_ALLOCED		(1 << __REQ_ALLOCED)
 #define REQ_COPY_USER		(1 << __REQ_COPY_USER)
 #define REQ_INTEGRITY		(1 << __REQ_INTEGRITY)
+#define REQ_FLUSH		(1 << __REQ_FLUSH)
 #define REQ_IO_STAT		(1 << __REQ_IO_STAT)
 #define REQ_MIXED_MERGE		(1 << __REQ_MIXED_MERGE)
 
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 1/9] block: introduce REQ_FLUSH flag FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 3/9] scsi: stop using q->prepare_flush_fn FUJITA Tomonori
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori

This is preparation for removing q->prepare_flush_fn.

Temporarily, blk_queue_ordered() permits QUEUE_ORDERED_DO_PREFLUSH and
QUEUE_ORDERED_DO_POSTFLUSH without prepare_flush_fn.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index a348242..7ce0a32 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -25,12 +25,6 @@
 int blk_queue_ordered(struct request_queue *q, unsigned ordered,
 		      prepare_flush_fn *prepare_flush_fn)
 {
-	if (!prepare_flush_fn && (ordered & (QUEUE_ORDERED_DO_PREFLUSH |
-					     QUEUE_ORDERED_DO_POSTFLUSH))) {
-		printk(KERN_ERR "%s: prepare_flush_fn required\n", __func__);
-		return -EINVAL;
-	}
-
 	if (ordered != QUEUE_ORDERED_NONE &&
 	    ordered != QUEUE_ORDERED_DRAIN &&
 	    ordered != QUEUE_ORDERED_DRAIN_FLUSH &&
@@ -146,7 +140,8 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->bar_rq.rq_disk;
 	rq->end_io = end_io;
-	q->prepare_flush_fn(q, rq);
+	if (q->prepare_flush_fn)
+		q->prepare_flush_fn(q, rq);
 
 	elv_insert(q, rq, ELEVATOR_INSERT_FRONT);
 }
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] scsi: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 1/9] block: introduce REQ_FLUSH flag FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 4/9] osdblk: " FUJITA Tomonori
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori, James Bottomley

scsi-ml builds flush requests via q->prepare_flush_fn(), however,
builds discard requests via q->prep_rq_fn.

Using two different mechnisms for the similar requests (building
commands in SCSI ULD) doesn't make sense.

Handing both via q->prep_rq_fn makes the code design simpler.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: James Bottomley <James.Bottomley@suse.de>
---
 drivers/scsi/sd.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index aa6b48b..e8c295e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -471,6 +471,18 @@ static int scsi_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
 	return ret;
 }
 
+static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
+{
+	/* for now, we use REQ_TYPE_BLOCK_PC. */
+	rq->cmd_type = REQ_TYPE_BLOCK_PC;
+	rq->timeout = SD_TIMEOUT;
+	rq->retries = SD_MAX_RETRIES;
+	rq->cmd[0] = SYNCHRONIZE_CACHE;
+	rq->cmd_len = 10;
+
+	return scsi_setup_blk_pc_cmnd(sdp, rq);
+}
+
 static void sd_unprep_fn(struct request_queue *q, struct request *rq)
 {
 	if (rq->cmd_flags & REQ_DISCARD)
@@ -504,6 +516,9 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	if (rq->cmd_flags & REQ_DISCARD) {
 		ret = scsi_setup_discard_cmnd(sdp, rq);
 		goto out;
+	} else if (rq->cmd_flags & REQ_FLUSH) {
+		ret = scsi_setup_flush_cmnd(sdp, rq);
+		goto out;
 	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
 		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
 		goto out;
@@ -1053,15 +1068,6 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
 	return 0;
 }
 
-static void sd_prepare_flush(struct request_queue *q, struct request *rq)
-{
-	rq->cmd_type = REQ_TYPE_BLOCK_PC;
-	rq->timeout = SD_TIMEOUT;
-	rq->retries = SD_MAX_RETRIES;
-	rq->cmd[0] = SYNCHRONIZE_CACHE;
-	rq->cmd_len = 10;
-}
-
 static void sd_rescan(struct device *dev)
 {
 	struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
@@ -2129,7 +2135,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	else
 		ordered = QUEUE_ORDERED_DRAIN;
 
-	blk_queue_ordered(sdkp->disk->queue, ordered, sd_prepare_flush);
+	blk_queue_ordered(sdkp->disk->queue, ordered, NULL);
 
 	set_capacity(disk, sdkp->capacity);
 	kfree(buffer);
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] osdblk: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (2 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 3/9] scsi: stop using q->prepare_flush_fn FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 5/9] ps3disk: " FUJITA Tomonori
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori

use REQ_FLUSH flag instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/block/osdblk.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/block/osdblk.c b/drivers/block/osdblk.c
index 819002b..9639565 100644
--- a/drivers/block/osdblk.c
+++ b/drivers/block/osdblk.c
@@ -323,7 +323,7 @@ static void osdblk_rq_fn(struct request_queue *q)
 		 * driver-specific, etc.
 		 */
 
-		do_flush = (rq->special == (void *) 0xdeadbeefUL);
+		do_flush = rq->cmd_flags & REQ_FLUSH;
 		do_write = (rq_data_dir(rq) == WRITE);
 
 		if (!do_flush) { /* osd_flush does not use a bio */
@@ -380,14 +380,6 @@ static void osdblk_rq_fn(struct request_queue *q)
 	}
 }
 
-static void osdblk_prepare_flush(struct request_queue *q, struct request *rq)
-{
-	/* add driver-specific marker, to indicate that this request
-	 * is a flush command
-	 */
-	rq->special = (void *) 0xdeadbeefUL;
-}
-
 static void osdblk_free_disk(struct osdblk_device *osdev)
 {
 	struct gendisk *disk = osdev->disk;
@@ -447,7 +439,7 @@ static int osdblk_init_disk(struct osdblk_device *osdev)
 	blk_queue_stack_limits(q, osd_request_queue(osdev->osd));
 
 	blk_queue_prep_rq(q, blk_queue_start_tag);
-	blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, osdblk_prepare_flush);
+	blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
 
 	disk->queue = q;
 
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] ps3disk: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (3 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 4/9] osdblk: " FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 6/9] dm: " FUJITA Tomonori
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori

REQ_FLUSH flag enables us to kill ps3disk_prepare_flush().

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/block/ps3disk.c |   25 ++++++-------------------
 1 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 5f208c0..ab528a4 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -196,13 +196,12 @@ static void ps3disk_do_request(struct ps3_storage_device *dev,
 	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
 
 	while ((req = blk_fetch_request(q))) {
-		if (req->cmd_type == REQ_TYPE_FS) {
-			if (ps3disk_submit_request_sg(dev, req))
-				break;
-		} else if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
-			   req->cmd[0] == REQ_LB_OP_FLUSH) {
+		if (req->cmd_flags & REQ_FLUSH) {
 			if (ps3disk_submit_flush_request(dev, req))
 				break;
+		} else if (req->cmd_type == REQ_TYPE_FS) {
+			if (ps3disk_submit_request_sg(dev, req))
+				break;
 		} else {
 			blk_dump_rq_flags(req, DEVICE_NAME " bad request");
 			__blk_end_request_all(req, -EIO);
@@ -257,8 +256,7 @@ static irqreturn_t ps3disk_interrupt(int irq, void *data)
 		return IRQ_HANDLED;
 	}
 
-	if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
-	    req->cmd[0] == REQ_LB_OP_FLUSH) {
+	if (req->cmd_flags & REQ_FLUSH) {
 		read = 0;
 		op = "flush";
 	} else {
@@ -398,16 +396,6 @@ static int ps3disk_identify(struct ps3_storage_device *dev)
 	return 0;
 }
 
-static void ps3disk_prepare_flush(struct request_queue *q, struct request *req)
-{
-	struct ps3_storage_device *dev = q->queuedata;
-
-	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
-
-	req->cmd_type = REQ_TYPE_LINUX_BLOCK;
-	req->cmd[0] = REQ_LB_OP_FLUSH;
-}
-
 static unsigned long ps3disk_mask;
 
 static DEFINE_MUTEX(ps3disk_mask_mutex);
@@ -480,8 +468,7 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
 	blk_queue_dma_alignment(queue, dev->blk_size-1);
 	blk_queue_logical_block_size(queue, dev->blk_size);
 
-	blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH,
-			  ps3disk_prepare_flush);
+	blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
 
 	blk_queue_max_segments(queue, -1);
 	blk_queue_max_segment_size(queue, dev->bounce_size);
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] dm: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (4 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 5/9] ps3disk: " FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 7/9] virtio_blk: " FUJITA Tomonori
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori, Alasdair G Kergon

use REQ_FLUSH flag instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Alasdair G Kergon <agk@redhat.com>
---
 drivers/md/dm.c |   16 ++--------------
 1 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index d6f77ba..00c8105 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1455,20 +1455,9 @@ static int dm_request(struct request_queue *q, struct bio *bio)
 	return _dm_request(q, bio);
 }
 
-/*
- * Mark this request as flush request, so that dm_request_fn() can
- * recognize.
- */
-static void dm_rq_prepare_flush(struct request_queue *q, struct request *rq)
-{
-	rq->cmd_type = REQ_TYPE_LINUX_BLOCK;
-	rq->cmd[0] = REQ_LB_OP_FLUSH;
-}
-
 static bool dm_rq_is_flush_request(struct request *rq)
 {
-	if (rq->cmd_type == REQ_TYPE_LINUX_BLOCK &&
-	    rq->cmd[0] == REQ_LB_OP_FLUSH)
+	if (rq->cmd_flags & REQ_FLUSH)
 		return true;
 	else
 		return false;
@@ -1912,8 +1901,7 @@ static struct mapped_device *alloc_dev(int minor)
 	blk_queue_softirq_done(md->queue, dm_softirq_done);
 	blk_queue_prep_rq(md->queue, dm_prep_fn);
 	blk_queue_lld_busy(md->queue, dm_lld_busy);
-	blk_queue_ordered(md->queue, QUEUE_ORDERED_DRAIN_FLUSH,
-			  dm_rq_prepare_flush);
+	blk_queue_ordered(md->queue, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
 
 	md->disk = alloc_disk(1);
 	if (!md->disk)
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] virtio_blk: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (5 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 6/9] dm: " FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03  8:45 ` [PATCH 8/9] ide: " FUJITA Tomonori
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori, Rusty Russell

use REQ_FLUSH flag instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/block/virtio_blk.c |   50 ++++++++++++++++++-------------------------
 1 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b5ebcd3..b277f9e 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -99,33 +99,32 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
 		return false;
 
 	vbr->req = req;
-	switch (req->cmd_type) {
-	case REQ_TYPE_FS:
-		vbr->out_hdr.type = 0;
-		vbr->out_hdr.sector = blk_rq_pos(vbr->req);
-		vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
-		break;
-	case REQ_TYPE_BLOCK_PC:
-		vbr->out_hdr.type = VIRTIO_BLK_T_SCSI_CMD;
-		vbr->out_hdr.sector = 0;
-		vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
-		break;
-	case REQ_TYPE_SPECIAL:
-		vbr->out_hdr.type = VIRTIO_BLK_T_GET_ID;
+
+	if (req->cmd_flags & REQ_FLUSH) {
+		vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
 		vbr->out_hdr.sector = 0;
 		vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
-		break;
-	case REQ_TYPE_LINUX_BLOCK:
-		if (req->cmd[0] == REQ_LB_OP_FLUSH) {
-			vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
+	} else {
+		switch (req->cmd_type) {
+		case REQ_TYPE_FS:
+			vbr->out_hdr.type = 0;
+			vbr->out_hdr.sector = blk_rq_pos(vbr->req);
+			vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
+			break;
+		case REQ_TYPE_BLOCK_PC:
+			vbr->out_hdr.type = VIRTIO_BLK_T_SCSI_CMD;
 			vbr->out_hdr.sector = 0;
 			vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
 			break;
+		case REQ_TYPE_SPECIAL:
+			vbr->out_hdr.type = VIRTIO_BLK_T_GET_ID;
+			vbr->out_hdr.sector = 0;
+			vbr->out_hdr.ioprio = req_get_ioprio(vbr->req);
+			break;
+		default:
+			/* We don't put anything else in the queue. */
+			BUG();
 		}
-		/*FALLTHRU*/
-	default:
-		/* We don't put anything else in the queue. */
-		BUG();
 	}
 
 	if (vbr->req->cmd_flags & REQ_HARDBARRIER)
@@ -195,12 +194,6 @@ static void do_virtblk_request(struct request_queue *q)
 		virtqueue_kick(vblk->vq);
 }
 
-static void virtblk_prepare_flush(struct request_queue *q, struct request *req)
-{
-	req->cmd_type = REQ_TYPE_LINUX_BLOCK;
-	req->cmd[0] = REQ_LB_OP_FLUSH;
-}
-
 /* return id (s/n) string for *disk to *id_str
  */
 static int virtblk_get_id(struct gendisk *disk, char *id_str)
@@ -373,8 +366,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 
 	/* If barriers are supported, tell block layer that queue is ordered */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
-		blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH,
-				  virtblk_prepare_flush);
+		blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
 	else if (virtio_has_feature(vdev, VIRTIO_BLK_F_BARRIER))
 		blk_queue_ordered(q, QUEUE_ORDERED_TAG, NULL);
 
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] ide: stop using q->prepare_flush_fn
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (6 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 7/9] virtio_blk: " FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03 16:48   ` David Miller
  2010-07-03  8:45 ` [PATCH 9/9] block: remove q->prepare_flush_fn completely FUJITA Tomonori
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori, David S. Miller

use REQ_FLUSH flag instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: David S. Miller <davem@davemloft.net>
---
 drivers/ide/ide-disk.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index df3d91b..c22e622 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -427,10 +427,15 @@ static void ide_disk_unlock_native_capacity(ide_drive_t *drive)
 		drive->dev_flags |= IDE_DFLAG_NOHPA; /* disable HPA on resume */
 }
 
-static void idedisk_prepare_flush(struct request_queue *q, struct request *rq)
+static int idedisk_prep_fn(struct request_queue *q, struct request *rq)
 {
 	ide_drive_t *drive = q->queuedata;
-	struct ide_cmd *cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+	struct ide_cmd *cmd;
+
+	if (!(rq->cmd_flags & REQ_FLUSH))
+		return BLKPREP_OK;
+
+	cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
 
 	/* FIXME: map struct ide_taskfile on rq->cmd[] */
 	BUG_ON(cmd == NULL);
@@ -448,6 +453,8 @@ static void idedisk_prepare_flush(struct request_queue *q, struct request *rq)
 	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
 	rq->special = cmd;
 	cmd->rq = rq;
+
+	return BLKPREP_OK;
 }
 
 ide_devset_get(multcount, mult_count);
@@ -513,7 +520,6 @@ static void update_ordered(ide_drive_t *drive)
 {
 	u16 *id = drive->id;
 	unsigned ordered = QUEUE_ORDERED_NONE;
-	prepare_flush_fn *prep_fn = NULL;
 
 	if (drive->dev_flags & IDE_DFLAG_WCACHE) {
 		unsigned long long capacity;
@@ -538,12 +544,12 @@ static void update_ordered(ide_drive_t *drive)
 
 		if (barrier) {
 			ordered = QUEUE_ORDERED_DRAIN_FLUSH;
-			prep_fn = idedisk_prepare_flush;
+			blk_queue_prep_rq(drive->queue, idedisk_prep_fn);
 		}
 	} else
 		ordered = QUEUE_ORDERED_DRAIN;
 
-	blk_queue_ordered(drive->queue, ordered, prep_fn);
+	blk_queue_ordered(drive->queue, ordered, NULL);
 }
 
 ide_devset_get_flag(wcache, IDE_DFLAG_WCACHE);
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] block: remove q->prepare_flush_fn completely
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (7 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 8/9] ide: " FUJITA Tomonori
@ 2010-07-03  8:45 ` FUJITA Tomonori
  2010-07-03 10:13   ` Christoph Hellwig
  2010-07-03 10:02 ` [PATCH 0/9] remove q->prepare_flush_fn hook Christoph Hellwig
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-03  8:45 UTC (permalink / raw)
  To: axboe; +Cc: hch, linux-kernel, fujita.tomonori

This removes q->prepare_flush_fn completely (changes the
blk_queue_ordered API).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c          |    7 +------
 drivers/block/brd.c          |    2 +-
 drivers/block/loop.c         |    2 +-
 drivers/block/osdblk.c       |    2 +-
 drivers/block/ps3disk.c      |    2 +-
 drivers/block/virtio_blk.c   |    4 ++--
 drivers/block/xen-blkfront.c |    3 +--
 drivers/ide/ide-disk.c       |    2 +-
 drivers/md/dm.c              |    2 +-
 drivers/mmc/card/queue.c     |    2 +-
 drivers/s390/block/dasd.c    |    2 +-
 drivers/scsi/sd.c            |    2 +-
 include/linux/blkdev.h       |    4 +---
 13 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index 7ce0a32..eefbde8 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -13,7 +13,6 @@
  * blk_queue_ordered - does this queue support ordered writes
  * @q:        the request queue
  * @ordered:  one of QUEUE_ORDERED_*
- * @prepare_flush_fn: rq setup helper for cache flush ordered writes
  *
  * Description:
  *   For journalled file systems, doing ordered writes on a commit
@@ -22,8 +21,7 @@
  *   feature should call this function and indicate so.
  *
  **/
-int blk_queue_ordered(struct request_queue *q, unsigned ordered,
-		      prepare_flush_fn *prepare_flush_fn)
+int blk_queue_ordered(struct request_queue *q, unsigned ordered)
 {
 	if (ordered != QUEUE_ORDERED_NONE &&
 	    ordered != QUEUE_ORDERED_DRAIN &&
@@ -38,7 +36,6 @@ int blk_queue_ordered(struct request_queue *q, unsigned ordered,
 
 	q->ordered = ordered;
 	q->next_ordered = ordered;
-	q->prepare_flush_fn = prepare_flush_fn;
 
 	return 0;
 }
@@ -140,8 +137,6 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->bar_rq.rq_disk;
 	rq->end_io = end_io;
-	if (q->prepare_flush_fn)
-		q->prepare_flush_fn(q, rq);
 
 	elv_insert(q, rq, ELEVATOR_INSERT_FRONT);
 }
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 1b218c6..1d2c186 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -479,7 +479,7 @@ static struct brd_device *brd_alloc(int i)
 	if (!brd->brd_queue)
 		goto out_free_dev;
 	blk_queue_make_request(brd->brd_queue, brd_make_request);
-	blk_queue_ordered(brd->brd_queue, QUEUE_ORDERED_TAG, NULL);
+	blk_queue_ordered(brd->brd_queue, QUEUE_ORDERED_TAG);
 	blk_queue_max_hw_sectors(brd->brd_queue, 1024);
 	blk_queue_bounce_limit(brd->brd_queue, BLK_BOUNCE_ANY);
 
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fedfdb7..d285a54 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -831,7 +831,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	lo->lo_queue->unplug_fn = loop_unplug;
 
 	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
-		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN, NULL);
+		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN);
 
 	set_capacity(lo->lo_disk, size);
 	bd_set_size(bdev, size << 9);
diff --git a/drivers/block/osdblk.c b/drivers/block/osdblk.c
index 9639565..2284b4f 100644
--- a/drivers/block/osdblk.c
+++ b/drivers/block/osdblk.c
@@ -439,7 +439,7 @@ static int osdblk_init_disk(struct osdblk_device *osdev)
 	blk_queue_stack_limits(q, osd_request_queue(osdev->osd));
 
 	blk_queue_prep_rq(q, blk_queue_start_tag);
-	blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
+	blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH);
 
 	disk->queue = q;
 
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index ab528a4..e9da874 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -468,7 +468,7 @@ static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
 	blk_queue_dma_alignment(queue, dev->blk_size-1);
 	blk_queue_logical_block_size(queue, dev->blk_size);
 
-	blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
+	blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH);
 
 	blk_queue_max_segments(queue, -1);
 	blk_queue_max_segment_size(queue, dev->bounce_size);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b277f9e..0a3222f 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -366,9 +366,9 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
 
 	/* If barriers are supported, tell block layer that queue is ordered */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH))
-		blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
+		blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH);
 	else if (virtio_has_feature(vdev, VIRTIO_BLK_F_BARRIER))
-		blk_queue_ordered(q, QUEUE_ORDERED_TAG, NULL);
+		blk_queue_ordered(q, QUEUE_ORDERED_TAG);
 
 	/* If disk is read-only in the host, the guest should obey */
 	if (virtio_has_feature(vdev, VIRTIO_BLK_F_RO))
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 495533e..76af65b 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -373,8 +373,7 @@ static int xlvbd_barrier(struct blkfront_info *info)
 	int err;
 
 	err = blk_queue_ordered(info->rq,
-				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE,
-				NULL);
+				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE);
 
 	if (err)
 		return err;
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index c22e622..7433e07 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -549,7 +549,7 @@ static void update_ordered(ide_drive_t *drive)
 	} else
 		ordered = QUEUE_ORDERED_DRAIN;
 
-	blk_queue_ordered(drive->queue, ordered, NULL);
+	blk_queue_ordered(drive->queue, ordered);
 }
 
 ide_devset_get_flag(wcache, IDE_DFLAG_WCACHE);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 00c8105..d505a96 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1901,7 +1901,7 @@ static struct mapped_device *alloc_dev(int minor)
 	blk_queue_softirq_done(md->queue, dm_softirq_done);
 	blk_queue_prep_rq(md->queue, dm_prep_fn);
 	blk_queue_lld_busy(md->queue, dm_lld_busy);
-	blk_queue_ordered(md->queue, QUEUE_ORDERED_DRAIN_FLUSH, NULL);
+	blk_queue_ordered(md->queue, QUEUE_ORDERED_DRAIN_FLUSH);
 
 	md->disk = alloc_disk(1);
 	if (!md->disk)
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index ec92bcb..c77eb49 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -128,7 +128,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, spinlock_t *lock
 	mq->req = NULL;
 
 	blk_queue_prep_rq(mq->queue, mmc_prep_request);
-	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN, NULL);
+	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN);
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, mq->queue);
 
 #ifdef CONFIG_MMC_BLOCK_BOUNCE
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 33975e9..17b033d 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2196,7 +2196,7 @@ static void dasd_setup_queue(struct dasd_block *block)
 	 */
 	blk_queue_max_segment_size(block->request_queue, PAGE_SIZE);
 	blk_queue_segment_boundary(block->request_queue, PAGE_SIZE - 1);
-	blk_queue_ordered(block->request_queue, QUEUE_ORDERED_DRAIN, NULL);
+	blk_queue_ordered(block->request_queue, QUEUE_ORDERED_DRAIN);
 }
 
 /*
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e8c295e..d9a4314 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2135,7 +2135,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	else
 		ordered = QUEUE_ORDERED_DRAIN;
 
-	blk_queue_ordered(sdkp->disk->queue, ordered, NULL);
+	blk_queue_ordered(sdkp->disk->queue, ordered);
 
 	set_capacity(disk, sdkp->capacity);
 	kfree(buffer);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6bba04c..3a2c5d9 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -212,7 +212,6 @@ struct bvec_merge_data {
 };
 typedef int (merge_bvec_fn) (struct request_queue *, struct bvec_merge_data *,
 			     struct bio_vec *);
-typedef void (prepare_flush_fn) (struct request_queue *, struct request *);
 typedef void (softirq_done_fn)(struct request *);
 typedef int (dma_drain_needed_fn)(struct request *);
 typedef int (lld_busy_fn) (struct request_queue *q);
@@ -286,7 +285,6 @@ struct request_queue
 	unprep_rq_fn		*unprep_rq_fn;
 	unplug_fn		*unplug_fn;
 	merge_bvec_fn		*merge_bvec_fn;
-	prepare_flush_fn	*prepare_flush_fn;
 	softirq_done_fn		*softirq_done_fn;
 	rq_timed_out_fn		*rq_timed_out_fn;
 	dma_drain_needed_fn	*dma_drain_needed;
@@ -896,7 +894,7 @@ extern void blk_queue_softirq_done(struct request_queue *, softirq_done_fn *);
 extern void blk_queue_rq_timed_out(struct request_queue *, rq_timed_out_fn *);
 extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
 extern struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev);
-extern int blk_queue_ordered(struct request_queue *, unsigned, prepare_flush_fn *);
+extern int blk_queue_ordered(struct request_queue *, unsigned);
 extern bool blk_do_ordered(struct request_queue *, struct request **);
 extern unsigned blk_ordered_cur_seq(struct request_queue *);
 extern unsigned blk_ordered_req_seq(struct request *);
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (8 preceding siblings ...)
  2010-07-03  8:45 ` [PATCH 9/9] block: remove q->prepare_flush_fn completely FUJITA Tomonori
@ 2010-07-03 10:02 ` Christoph Hellwig
  2010-07-05  6:55 ` Jens Axboe
  2010-07-07 19:52 ` Christoph Hellwig
  11 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-07-03 10:02 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: axboe, hch, linux-kernel

On Sat, Jul 03, 2010 at 05:45:31PM +0900, FUJITA Tomonori wrote:
> This removes q->prepare_flush_fn. Except for ide and scsi, all the
> users just mark flush requests. They can use REQ_FLUSH flag
> instead.
> 
> SCSI and ide can use q->prep_rq_fn to build flush requests.
> 
> This can be applied to the block's for-2.6.36.

The whole patchset looks good to me,


Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 9/9] block: remove q->prepare_flush_fn completely
  2010-07-03  8:45 ` [PATCH 9/9] block: remove q->prepare_flush_fn completely FUJITA Tomonori
@ 2010-07-03 10:13   ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-07-03 10:13 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: axboe, linux-kernel, knikanth, jeremy, drzeus

Not a comment on the patch itself, but some interesting observations
on the drivers ordered flags:

> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index fedfdb7..d285a54 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -831,7 +831,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>  	lo->lo_queue->unplug_fn = loop_unplug;
>  
>  	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
> -		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN, NULL);
> +		blk_queue_ordered(lo->lo_queue, QUEUE_ORDERED_DRAIN);

loop actually does flushes.  But implements them by itself because
it's bio based.  Seems like something is not so nice about the API
for implementing barriers with bio based drivers.

Also do we actually get the draining semantics right this way?  Unless
loop only supports one in-flight bio this seems to not be correct.

> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 495533e..76af65b 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -373,8 +373,7 @@ static int xlvbd_barrier(struct blkfront_info *info)
>  	int err;
>  
>  	err = blk_queue_ordered(info->rq,
> -				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE,
> -				NULL);
> +				info->feature_barrier ? QUEUE_ORDERED_DRAIN : QUEUE_ORDERED_NONE);

Something is really broken with the way Xen implements barriers.
It claims to only require drains, but then actually marks barrier
requests as such.  The qemu backend at least then does the pre and post
drains by itself.  While this gets the correct results for writes marked
as barriers, it can't properly implement empty barriers (aka cache
flushes) which are just as important.

> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -128,7 +128,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, spinlock_t *lock
>  	mq->req = NULL;
>  
>  	blk_queue_prep_rq(mq->queue, mmc_prep_request);
> -	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN, NULL);
> +	blk_queue_ordered(mq->queue, QUEUE_ORDERED_DRAIN);

So MMC device never have volatile write caches?


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/9] ide: stop using q->prepare_flush_fn
  2010-07-03  8:45 ` [PATCH 8/9] ide: " FUJITA Tomonori
@ 2010-07-03 16:48   ` David Miller
  0 siblings, 0 replies; 19+ messages in thread
From: David Miller @ 2010-07-03 16:48 UTC (permalink / raw)
  To: fujita.tomonori; +Cc: axboe, hch, linux-kernel

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Sat,  3 Jul 2010 17:45:39 +0900

> use REQ_FLUSH flag instead.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (9 preceding siblings ...)
  2010-07-03 10:02 ` [PATCH 0/9] remove q->prepare_flush_fn hook Christoph Hellwig
@ 2010-07-05  6:55 ` Jens Axboe
  2010-07-05  8:01   ` FUJITA Tomonori
  2010-07-07 19:52 ` Christoph Hellwig
  11 siblings, 1 reply; 19+ messages in thread
From: Jens Axboe @ 2010-07-05  6:55 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: hch, linux-kernel

On 2010-07-03 10:45, FUJITA Tomonori wrote:
> This removes q->prepare_flush_fn. Except for ide and scsi, all the
> users just mark flush requests. They can use REQ_FLUSH flag
> instead.
> 
> SCSI and ide can use q->prep_rq_fn to build flush requests.
> 
> This can be applied to the block's for-2.6.36.

Thanks, applied.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-05  6:55 ` Jens Axboe
@ 2010-07-05  8:01   ` FUJITA Tomonori
  2010-07-05  8:03     ` Jens Axboe
  0 siblings, 1 reply; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-05  8:01 UTC (permalink / raw)
  To: axboe; +Cc: fujita.tomonori, hch, linux-kernel

On Mon, 05 Jul 2010 08:55:36 +0200
Jens Axboe <axboe@kernel.dk> wrote:

> On 2010-07-03 10:45, FUJITA Tomonori wrote:
> > This removes q->prepare_flush_fn. Except for ide and scsi, all the
> > users just mark flush requests. They can use REQ_FLUSH flag
> > instead.
> > 
> > SCSI and ide can use q->prep_rq_fn to build flush requests.
> > 
> > This can be applied to the block's for-2.6.36.
> 
> Thanks, applied.

Thanks,

looks like you applied the patches in a wrong order (breaks bisect).

At least, the first two patches need to be applied in order before the
rest:

[PATCH 1/9] block: introduce REQ_FLUSH flag
[PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-05  8:01   ` FUJITA Tomonori
@ 2010-07-05  8:03     ` Jens Axboe
  0 siblings, 0 replies; 19+ messages in thread
From: Jens Axboe @ 2010-07-05  8:03 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: hch, linux-kernel

On 2010-07-05 10:01, FUJITA Tomonori wrote:
> On Mon, 05 Jul 2010 08:55:36 +0200
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 2010-07-03 10:45, FUJITA Tomonori wrote:
>>> This removes q->prepare_flush_fn. Except for ide and scsi, all the
>>> users just mark flush requests. They can use REQ_FLUSH flag
>>> instead.
>>>
>>> SCSI and ide can use q->prep_rq_fn to build flush requests.
>>>
>>> This can be applied to the block's for-2.6.36.
>>
>> Thanks, applied.
> 
> Thanks,
> 
> looks like you applied the patches in a wrong order (breaks bisect).
> 
> At least, the first two patches need to be applied in order before the
> rest:
> 
> [PATCH 1/9] block: introduce REQ_FLUSH flag
> [PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn

Funky, my patch script must have shuffled them, that was definitely
not on purpose. Guess I'll have to rebase to fix that up...


-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
                   ` (10 preceding siblings ...)
  2010-07-05  6:55 ` Jens Axboe
@ 2010-07-07 19:52 ` Christoph Hellwig
  2010-07-07 23:54   ` FUJITA Tomonori
  11 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2010-07-07 19:52 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: axboe, hch, linux-kernel

One weird thing after this set is that the flush commands don't have any
cmd_type.  I think it should be set to REQ_TYPE_FS, even if we might be
resetting it inside the driver for now.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-07 19:52 ` Christoph Hellwig
@ 2010-07-07 23:54   ` FUJITA Tomonori
  2010-07-07 23:58     ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: FUJITA Tomonori @ 2010-07-07 23:54 UTC (permalink / raw)
  To: hch; +Cc: fujita.tomonori, axboe, linux-kernel

On Wed, 7 Jul 2010 15:52:09 -0400
Christoph Hellwig <hch@infradead.org> wrote:

> One weird thing after this set is that the flush commands don't have any
> cmd_type.  I think it should be set to REQ_TYPE_FS, even if we might be
> resetting it inside the driver for now.

I think even before this patchset, the block layer doesn't set
rq->cmd_type on flush requests (scsi and Ode sets it in
prepare_flush_fn).

I agree that we should set rq->cmd_type. And it should be REQ_TYPE_FS
by definition.

>From a quick look, setting REQ_TYPE_FS in the block layer doesn't
cause problems to the users of flush requests.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] block: set REQ_TYPE_FS on flush requests

the block layer doesn't set rq->cmd_type on flush requests. By
definition, it should be REQ_TYPE_FS (the lower layers build a command
and interpret the result of it, that is, the block layer doesn't know
the details).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 block/blk-barrier.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index eefbde8..e7fed5e 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -134,6 +134,7 @@ static void queue_flush(struct request_queue *q, unsigned which)
 	}
 
 	blk_rq_init(q, rq);
+	rq->cmd_type = REQ_TYPE_FS;
 	rq->cmd_flags = REQ_HARDBARRIER | REQ_FLUSH;
 	rq->rq_disk = q->bar_rq.rq_disk;
 	rq->end_io = end_io;
-- 
1.6.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] remove q->prepare_flush_fn hook
  2010-07-07 23:54   ` FUJITA Tomonori
@ 2010-07-07 23:58     ` Christoph Hellwig
  0 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2010-07-07 23:58 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: hch, axboe, linux-kernel

On Thu, Jul 08, 2010 at 08:54:59AM +0900, FUJITA Tomonori wrote:
> On Wed, 7 Jul 2010 15:52:09 -0400
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > One weird thing after this set is that the flush commands don't have any
> > cmd_type.  I think it should be set to REQ_TYPE_FS, even if we might be
> > resetting it inside the driver for now.
> 
> I think even before this patchset, the block layer doesn't set
> rq->cmd_type on flush requests (scsi and Ode sets it in
> prepare_flush_fn).

Indeed it didn't.  But before that it matters less because
prepare_flush_fn set it before the command entered prep_fn.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-07-07 23:58 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-03  8:45 [PATCH 0/9] remove q->prepare_flush_fn hook FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 1/9] block: introduce REQ_FLUSH flag FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 2/9] block: permit PREFLUSH and POSTFLUSH without prepare_flush_fn FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 3/9] scsi: stop using q->prepare_flush_fn FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 4/9] osdblk: " FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 5/9] ps3disk: " FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 6/9] dm: " FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 7/9] virtio_blk: " FUJITA Tomonori
2010-07-03  8:45 ` [PATCH 8/9] ide: " FUJITA Tomonori
2010-07-03 16:48   ` David Miller
2010-07-03  8:45 ` [PATCH 9/9] block: remove q->prepare_flush_fn completely FUJITA Tomonori
2010-07-03 10:13   ` Christoph Hellwig
2010-07-03 10:02 ` [PATCH 0/9] remove q->prepare_flush_fn hook Christoph Hellwig
2010-07-05  6:55 ` Jens Axboe
2010-07-05  8:01   ` FUJITA Tomonori
2010-07-05  8:03     ` Jens Axboe
2010-07-07 19:52 ` Christoph Hellwig
2010-07-07 23:54   ` FUJITA Tomonori
2010-07-07 23:58     ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.