Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2964 bytes --]

On Wed, 13 Aug 2008, David Woodhouse wrote:
> It was only used by ps3disk, and it should probably have been
> REQ_TYPE_LINUX_BLOCK + REQ_LB_OP_FLUSH.

I used REQ_TYPE_FLUSH after discussing with the block experts.

Note that REQ_LB_OP_FLUSH is also never used. Actually its definition in
include/linux/blkdev.h has:

| enum {
| 	/*
| 	 * just examples for now
           ^^^^^^^^^^^^^^^^^^^^^
| 	 */
| 	REQ_LB_OP_EJECT	= 0x40,		/* eject request */
| 	REQ_LB_OP_FLUSH = 0x41,		/* flush device */
| };

which makes me a bit reluctant to make this change.

BTW, how to test all this flushing behavior, if there are no real users?

> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  drivers/block/ps3disk.c |    9 ++++++---
>  include/linux/blkdev.h  |    1 -
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index d797e20..4b0d6c7 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -199,7 +199,8 @@ static void ps3disk_do_request(struct ps3_storage_device *dev,
>  		if (blk_fs_request(req)) {
>  			if (ps3disk_submit_request_sg(dev, req))
>  				break;
> -		} else if (req->cmd_type == REQ_TYPE_FLUSH) {
> +		} else if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
> +			   req->cmd[0] == REQ_LB_OP_FLUSH) {
>  			if (ps3disk_submit_flush_request(dev, req))
>  				break;
>  		} else {
> @@ -257,7 +258,8 @@ static irqreturn_t ps3disk_interrupt(int irq, void *data)
>  		return IRQ_HANDLED;
>  	}
>  
> -	if (req->cmd_type == REQ_TYPE_FLUSH) {
> +	if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
> +	    req->cmd[0] == REQ_LB_OP_FLUSH) {
>  		read = 0;
>  		num_sectors = req->hard_cur_sectors;
>  		op = "flush";
> @@ -405,7 +407,8 @@ static void ps3disk_prepare_flush(struct request_queue *q, struct request *req)
>  
>  	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
>  
> -	req->cmd_type = REQ_TYPE_FLUSH;
> +	req->cmd_type = REQ_TYPE_LINUX_BLOCK;
> +	req->cmd[0] = REQ_LB_OP_FLUSH;
>  }
>  
>  static unsigned long ps3disk_mask;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 293a71a..a0fa413 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -54,7 +54,6 @@ enum rq_cmd_type_bits {
>  	REQ_TYPE_PM_SUSPEND,		/* suspend request */
>  	REQ_TYPE_PM_RESUME,		/* resume request */
>  	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
> -	REQ_TYPE_FLUSH,			/* flush request */
>  	REQ_TYPE_SPECIAL,		/* driver defined type */
>  	REQ_TYPE_LINUX_BLOCK,		/* generic block layer message */
>  	/*

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis 293-0376800-10 GEBA-BE-BB

Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

[David Woodhouse]

On Wed, 2008-08-13 at 13:58 +0200, Geert Uytterhoeven wrote:
> Note that REQ_LB_OP_FLUSH is also never used.

That can be fixed...

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 681d5ac..8663f3f 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -32,6 +32,13 @@ struct mtd_blkcore_priv {
 	spinlock_t queue_lock;
 };
 
+static void blktrans_prepare_flush(struct request_queue *q,
+				   struct request *req)
+{
+	req->cmd_type = REQ_TYPE_LINUX_BLOCK;
+	req->cmd[0] = REQ_LB_OP_FLUSH;
+}
+
 static int blktrans_discard_request(struct request_queue *q,
 				    struct request *req)
 {
@@ -53,6 +60,14 @@ static int do_blktrans_request(struct mtd_blktrans_ops *tr,
 	buf = req->buffer;
 
 	if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
+	    req->cmd[0] == REQ_LB_OP_FLUSH) {
+		if (tr->flush)
+			return !tr->flush(dev);
+		else
+			return 1;
+	}
+
+	if (req->cmd_type == REQ_TYPE_LINUX_BLOCK &&
 	    req->cmd[0] == REQ_LB_OP_DISCARD)
 		return !tr->discard(dev, block, nsect);
 
@@ -383,6 +398,9 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 		blk_queue_set_discard(tr->blkcore_priv->rq,
 				      blktrans_discard_request);
 
+	blk_queue_ordered(tr->blkcore_priv->rq, QUEUE_ORDERED_DRAIN_FLUSH,
+			  blktrans_prepare_flush);
+
 	tr->blkshift = ffs(tr->blksize) - 1;
 
 	tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,


>  Actually its definition in include/linux/blkdev.h has:
> 
> | enum {
> |       /*
> |        * just examples for now
>            ^^^^^^^^^^^^^^^^^^^^^
> |        */
> |       REQ_LB_OP_EJECT = 0x40,         /* eject request */
> |       REQ_LB_OP_FLUSH = 0x41,         /* flush device */
> | };
> 
> which makes me a bit reluctant to make this change.

That can be fixed too...

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 293a71a..7ef582f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -76,11 +75,8 @@ enum rq_cmd_type_bits {
  *
  */
 enum {
-	/*
-	 * just examples for now
-	 */
 	REQ_LB_OP_EJECT	= 0x40,		/* eject request */
-	REQ_LB_OP_FLUSH = 0x41,		/* flush device */
+	REQ_LB_OP_FLUSH = 0x41,		/* flush request */
 	REQ_LB_OP_DISCARD = 0x42,	/* discard sectors */
 };
 

> BTW, how to test all this flushing behavior, if there are no real
> users?

btrfs uses it.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation