From: Christoph Hellwig <hch@infradead.org>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org, knikanth@suse.de,
jeremy@xensource.com, drzeus@drzeus.cx
Subject: Re: [PATCH 9/9] block: remove q->prepare_flush_fn completely
Date: Sat, 3 Jul 2010 06:13:20 -0400 [thread overview]
Message-ID: <20100703101320.GB22329@infradead.org> (raw)
In-Reply-To: <1278146740-8731-10-git-send-email-fujita.tomonori@lab.ntt.co.jp>
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?
next prev parent reply other threads:[~2010-07-03 10:13 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100703101320.GB22329@infradead.org \
--to=hch@infradead.org \
--cc=axboe@kernel.dk \
--cc=drzeus@drzeus.cx \
--cc=fujita.tomonori@lab.ntt.co.jp \
--cc=jeremy@xensource.com \
--cc=knikanth@suse.de \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome