Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH - Kernel

This is a discussion on Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

  1. Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

    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
    > ---
    > 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

  2. Re: [PATCH 11/7] Kill REQ_TYPE_FLUSH

    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



    --
    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/

+ Reply to Thread