Hello,
This patchset replaces the current barrier implementation with
sequenced flush which doesn't impose any restriction on ordering
around the flush requests. This patchst is result of the following
discussion thread.
http://thread.gmane.org/gmane.linux.file-systems/43877
In summary, filesystems can take over the ordering of requests around
commit writes and the block layer should just supply a mechanism to
perform the commit writes themselves. This would greatly lessen tha
stall caused by queue dumping and draining used by the current barrier
implementation for request ordering.
This patchset converts barrier mechanism to sequenced flush/fua
mechanism in the following steps.
1. Kill the mostly unused ORDERED_BY_TAG support.
2. Deprecate REQ_HARDBARRIER support. All hard barrier requests are
failed with -EOPNOTSUPP.
3. Drop barrier ordering by queue draining mechanism.
4. Rename barrier to flush and implement new interface based on
REQ_FLUSH and REQ_FUA as suggested by Christoph.
blkdev_issue_flush() is converted to use the new mechanism but all the
filesystems still use the deprecated REQ_HARDBARRIER which always
fails. Each filesystem needs to be updated to enforce request
ordering themselves and then to use REQ_FLUSH/FUA mechanism.
loop, md, dm, etc... haven't been converted yet and REQ_FLUSH/FUA
doesn't work with them yet. I'll convert most of them soonish if this
patchset is generally agreed upon.
This patchset contains the following patches.
0001-block-loop-queue-ordered-mode-should-be-DRAIN_FLUSH.patch
0002-block-kill-QUEUE_ORDERED_BY_TAG.patch
0003-block-deprecate-barrier-and-replace-blk_queue_ordere.patch
0004-block-remove-spurious-uses-of-REQ_HARDBARRIER.patch
0005-block-misc-cleanups-in-barrier-code.patch
0006-block-drop-barrier-ordering-by-queue-draining.patch
0007-block-rename-blk-barrier.c-to-blk-flush.c.patch
0008-block-rename-barrier-ordered-to-flush.patch
0009-block-implement-REQ_FLUSH-FUA-based-interface-for-FL.patch
0010-fs-block-propagate-REQ_FLUSH-FUA-interface-to-upper-.patch
0011-block-use-REQ_FLUSH-in-blkdev_issue_flush.patch
and is also available in the following git tree.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git flush-fua
and contains the following changes.
block/Makefile | 2
block/blk-barrier.c | 350 ------------------------------------
block/blk-core.c | 55 ++---
block/blk-flush.c | 248 +++++++++++++++++++++++++
block/blk-settings.c | 20 ++
block/blk.h | 8
block/elevator.c | 79 --------
drivers/block/brd.c | 1
drivers/block/loop.c | 2
drivers/block/osdblk.c | 5
drivers/block/pktcdvd.c | 1
drivers/block/ps3disk.c | 2
drivers/block/virtio_blk.c | 34 ---
drivers/block/xen-blkfront.c | 47 +---
drivers/ide/ide-disk.c | 13 -
drivers/md/dm.c | 2
drivers/mmc/card/queue.c | 1
drivers/s390/block/dasd.c | 1
drivers/scsi/aic7xxx_old.c | 21 --
drivers/scsi/libsas/sas_scsi_host.c | 13 -
drivers/scsi/sd.c | 18 -
fs/buffer.c | 27 +-
include/linux/blk_types.h | 4
include/linux/blkdev.h | 73 +------
include/linux/buffer_head.h | 8
include/linux/fs.h | 20 +-
include/scsi/scsi_tcq.h | 6
27 files changed, 402 insertions(+), 659 deletions(-)
Thanks.
--
tejun
loop implements FLUSH using fsync but was incorrectly setting its
ordered mode to DRAIN. Change it to DRAIN_FLUSH. In practice, this
doesn't change anything as loop doesn't make use of the block layer
ordered implementation.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/block/loop.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Nobody is making meaningful use of ORDERED_BY_TAG now and queue
draining for barrier requests will be removed soon which will render
the advantage of tag ordering moot. Kill ORDERED_BY_TAG. The
following users are affected.
* brd: converted to ORDERED_DRAIN.
* virtio_blk: ORDERED_TAG path was already marked deprecated. Removed.
* xen-blkfront: ORDERED_TAG case dropped.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Nick Piggin <redacted>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jeremy Fitzhardinge <redacted>
Cc: Chris Wright <redacted>
---
block/blk-barrier.c | 35 +++++++----------------------------
drivers/block/brd.c | 2 +-
drivers/block/virtio_blk.c | 9 ---------
drivers/block/xen-blkfront.c | 8 +++-----
drivers/scsi/sd.c | 4 +---
include/linux/blkdev.h | 17 +----------------
6 files changed, 13 insertions(+), 62 deletions(-)
@@ -26,10 +26,7 @@ int blk_queue_ordered(struct request_queue *q, unsigned ordered)if(ordered!=QUEUE_ORDERED_NONE&&ordered!=QUEUE_ORDERED_DRAIN&&ordered!=QUEUE_ORDERED_DRAIN_FLUSH&&-ordered!=QUEUE_ORDERED_DRAIN_FUA&&-ordered!=QUEUE_ORDERED_TAG&&-ordered!=QUEUE_ORDERED_TAG_FLUSH&&-ordered!=QUEUE_ORDERED_TAG_FUA){+ordered!=QUEUE_ORDERED_DRAIN_FUA){printk(KERN_ERR"blk_queue_ordered: bad value %d\n",ordered);return-EINVAL;}
@@ -155,21 +152,9 @@ static inline bool start_ordered(struct request_queue *q, struct request **rqp)*Foranemptybarrier,there'snoactualBARrequest,which*inturnmakesPOSTFLUSHunnecessary.Maskthemoff.*/-if(!blk_rq_sectors(rq)){+if(!blk_rq_sectors(rq))q->ordered&=~(QUEUE_ORDERED_DO_BAR|QUEUE_ORDERED_DO_POSTFLUSH);-/*-*Emptybarrieronawrite-throughdevicew/ordered-*taghasnocommandtoissueandwithoutanycommand-*toissue,orderingbytagcan'tbeused.Drain-*instead.-*/-if((q->ordered&QUEUE_ORDERED_BY_TAG)&&-!(q->ordered&QUEUE_ORDERED_DO_PREFLUSH)){-q->ordered&=~QUEUE_ORDERED_BY_TAG;-q->ordered|=QUEUE_ORDERED_BY_DRAIN;-}-}/* stash away the original request */blk_dequeue_request(rq);
@@ -257,16 +242,10 @@ bool blk_do_ordered(struct request_queue *q, struct request **rqp)rq!=&q->pre_flush_rq&&rq!=&q->post_flush_rq)returntrue;-if(q->ordered&QUEUE_ORDERED_BY_TAG){-/* Ordered by tag. Blocking the next barrier is enough. */-if(is_barrier&&rq!=&q->bar_rq)-*rqp=NULL;-}else{-/* Ordered by draining. Wait for turn. */-WARN_ON(blk_ordered_req_seq(rq)<blk_ordered_cur_seq(q));-if(blk_ordered_req_seq(rq)>blk_ordered_cur_seq(q))-*rqp=NULL;-}+/* Ordered by draining. Wait for turn. */+WARN_ON(blk_ordered_req_seq(rq)<blk_ordered_cur_seq(q));+if(blk_ordered_req_seq(rq)>blk_ordered_cur_seq(q))+*rqp=NULL;returntrue;}
Barrier is deemed too heavy and will soon be replaced by FLUSH/FUA
requests. Deprecate barrier. All REQ_HARDBARRIERs are failed with
-EOPNOTSUPP and blk_queue_ordered() is replaced with simpler
blk_queue_flush().
blk_queue_flush() takes combinations of REQ_FLUSH and FUA. If a
device has write cache and can flush it, it should set REQ_FLUSH. If
the device can handle FUA writes, it should also set REQ_FUA.
All blk_queue_ordered() users are converted.
* ORDERED_DRAIN is mapped to 0 which is the default value.
* ORDERED_DRAIN_FLUSH is mapped to REQ_FLUSH.
* ORDERED_DRAIN_FLUSH_FUA is mapped to REQ_FLUSH | REQ_FUA.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Nick Piggin <redacted>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jeremy Fitzhardinge <redacted>
Cc: Chris Wright <redacted>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Boaz Harrosh <redacted>
Cc: Geert Uytterhoeven <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Pierre Ossman <redacted>
Cc: Stefan Weinhuber <redacted>
---
block/blk-barrier.c | 29 ----------------------------
block/blk-core.c | 6 +++-
block/blk-settings.c | 20 +++++++++++++++++++
drivers/block/brd.c | 1 -
drivers/block/loop.c | 2 +-
drivers/block/osdblk.c | 2 +-
drivers/block/ps3disk.c | 2 +-
drivers/block/virtio_blk.c | 25 ++++++++---------------
drivers/block/xen-blkfront.c | 43 +++++++++++------------------------------
drivers/ide/ide-disk.c | 13 +++++------
drivers/md/dm.c | 2 +-
drivers/mmc/card/queue.c | 1 -
drivers/s390/block/dasd.c | 1 -
drivers/scsi/sd.c | 16 +++++++-------
include/linux/blkdev.h | 6 +++-
15 files changed, 67 insertions(+), 102 deletions(-)
@@ -9,35 +9,6 @@#include"blk.h"-/**-*blk_queue_ordered-doesthisqueuesupportorderedwrites-*@q:therequestqueue-*@ordered:oneofQUEUE_ORDERED_*-*-*Description:-*Forjournalledfilesystems,doingorderedwritesonacommit-*blockinsteadofexplicitlydoingwait_on_buffer(whichisbad-*forperformance)canbeabigwin.Blockdriverssupportingthis-*featureshouldcallthisfunctionandindicateso.-*-**/-intblk_queue_ordered(structrequest_queue*q,unsignedordered)-{-if(ordered!=QUEUE_ORDERED_NONE&&-ordered!=QUEUE_ORDERED_DRAIN&&-ordered!=QUEUE_ORDERED_DRAIN_FLUSH&&-ordered!=QUEUE_ORDERED_DRAIN_FUA){-printk(KERN_ERR"blk_queue_ordered: bad value %d\n",ordered);-return-EINVAL;-}--q->ordered=ordered;-q->next_ordered=ordered;--return0;-}-EXPORT_SYMBOL(blk_queue_ordered);-/**Cacheflushingfororderedwriteshandling*/
@@ -1203,11 +1203,13 @@ static int __make_request(struct request_queue *q, struct bio *bio)constunsignedintff=bio->bi_rw&REQ_FAILFAST_MASK;intrw_flags;-if((bio->bi_rw&REQ_HARDBARRIER)&&-(q->next_ordered==QUEUE_ORDERED_NONE)){+/* REQ_HARDBARRIER is no more */+if(WARN_ONCE(bio->bi_rw&REQ_HARDBARRIER,+"block: HARDBARRIER is deprecated, use FLUSH/FUA instead\n")){bio_endio(bio,-EOPNOTSUPP);return0;}+/**lowleveldrivercanindicatethatitwantspagesabovea*certainlimitbouncedtolowmemory(ieforhighmem,oreven
@@ -388,22 +388,15 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)vblk->disk->driverfs_dev=&vdev->dev;index++;-if(virtio_has_feature(vdev,VIRTIO_BLK_F_FLUSH)){-/*-*IftheFLUSHfeatureissupportedwedohavesupportfor-*flushingavolatilewritecacheonthehost.Usethat-*toimplementwritebarriersupport.-*/-blk_queue_ordered(q,QUEUE_ORDERED_DRAIN_FLUSH);-}else{-/*-*IftheFLUSHfeatureisnotsupportedwemustassumethat-*thehostdoesnotperformanykindofvolatilewrite-*caching.Westillneedtodrainthequeuetoprovider-*properbarriersemantics.-*/-blk_queue_ordered(q,QUEUE_ORDERED_DRAIN);-}+/*+*IftheFLUSHfeatureissupportedwedohavesupportfor+*flushingavolatilewritecacheonthehost.Usethatto+*implementwritebarriersupport;otherwise,wemustassume+*thatthehostdoesnotperformanykindofvolatilewrite+*caching.+*/+if(virtio_has_feature(vdev,VIRTIO_BLK_F_FLUSH))+blk_queue_flush(q,REQ_FLUSH);/* If disk is read-only in the host, the guest should obey */if(virtio_has_feature(vdev,VIRTIO_BLK_F_RO))
REQ_HARDBARRIER is deprecated. Remove spurious uses in the following
users. Please note that other than osdblk, all other uses were
already spurious before deprecation.
* osdblk: osdblk_rq_fn() won't receive any request with
REQ_HARDBARRIER set. Remove the test for it.
* pktcdvd: use of REQ_HARDBARRIER in pkt_generic_packet() doesn't mean
anything. Removed.
* aic7xxx_old: Setting MSG_ORDERED_Q_TAG on REQ_HARDBARRIER is
spurious. Removed.
* sas_scsi_host: Setting TASK_ATTR_ORDERED on REQ_HARDBARRIER is
spurious. Removed.
* scsi_tcq: The ordered tag path wasn't being used anyway. Removed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Boaz Harrosh <redacted>
Cc: James Bottomley <redacted>
Cc: Peter Osterlund <redacted>
---
drivers/block/osdblk.c | 3 +--
drivers/block/pktcdvd.c | 1 -
drivers/scsi/aic7xxx_old.c | 21 ++-------------------
drivers/scsi/libsas/sas_scsi_host.c | 13 +------------
include/scsi/scsi_tcq.h | 6 +-----
5 files changed, 5 insertions(+), 39 deletions(-)
Make the following cleanups in preparation of barrier/flush update.
* blk_do_ordered() declaration is moved from include/linux/blkdev.h to
block/blk.h.
* blk_do_ordered() now returns pointer to struct request, with %NULL
meaning "try the next request" and ERR_PTR(-EAGAIN) "try again
later". The third case will be dropped with further changes.
* In the initialization of proxy barrier request, data direction is
already set by init_request_from_bio(). Drop unnecessary explicit
REQ_WRITE setting and move init_request_from_bio() above REQ_FUA
flag setting.
* add_request() is collapsed into __make_request().
These changes don't make any functional difference.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/blk-barrier.c | 32 ++++++++++++++------------------
block/blk-core.c | 21 ++++-----------------
block/blk.h | 7 +++++--
include/linux/blkdev.h | 1 -
4 files changed, 23 insertions(+), 38 deletions(-)
@@ -211,14 +207,14 @@ bool blk_do_ordered(struct request_queue *q, struct request **rqp)/* Special requests are not subject to ordering rules. */if(rq->cmd_type!=REQ_TYPE_FS&&rq!=&q->pre_flush_rq&&rq!=&q->post_flush_rq)-returntrue;+returnrq;/* Ordered by draining. Wait for turn. */WARN_ON(blk_ordered_req_seq(rq)<blk_ordered_cur_seq(q));if(blk_ordered_req_seq(rq)>blk_ordered_cur_seq(q))-*rqp=NULL;+rq=ERR_PTR(-EAGAIN);-returntrue;+returnrq;}staticvoidbio_end_empty_barrier(structbio*bio,interr)
@@ -1316,7 +1300,10 @@ get_rq:req->cpu=blk_cpu_to_group(smp_processor_id());if(queue_should_plug(q)&&elv_queue_empty(q))blk_plug_device(q);-add_request(q,req);++/* insert the request into the elevator */+drive_stat_acct(req,1);+__elv_add_request(q,req,ELEVATOR_INSERT_SORT,0);out:if(unplug||!queue_should_plug(q))__generic_unplug_device(q);
Filesystems will take all the responsibilities for ordering requests
around commit writes and will only indicate how the commit writes
themselves should be handled by block layers. This patch drops
barrier ordering by queue draining from block layer. Ordering by
draining implementation was somewhat invasive to request handling.
List of notable changes follow.
* Each queue has 1 bit color which is flipped on each barrier issue.
This is used to track whether a given request is issued before the
current barrier or not. REQ_ORDERED_COLOR flag and coloring
implementation in __elv_add_request() are removed.
* Requests which shouldn't be processed yet for draining were stalled
by returning -EAGAIN from blk_do_ordered() according to the test
result between blk_ordered_req_seq() and blk_blk_ordered_cur_seq().
This logic is removed.
* Draining completion logic in elv_completed_request() removed.
* All barrier sequence requests were queued to request queue and then
trckled to lower layer according to progress and thus maintaining
request orders during requeue was necessary. This is replaced by
queueing the next request in the barrier sequence only after the
current one is complete from blk_ordered_complete_seq(), which
removes the need for multiple proxy requests in struct request_queue
and the request sorting logic in the ELEVATOR_INSERT_REQUEUE path of
elv_insert().
* As barriers no longer have ordering constraints, there's no need to
dump the whole elevator onto the dispatch queue on each barrier.
Insert barriers at the front instead.
* If other barrier requests come to the front of the dispatch queue
while one is already in progress, they are stored in
q->pending_barriers and restored to dispatch queue one-by-one after
each barrier completion from blk_ordered_complete_seq().
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
block/blk-barrier.c | 220 ++++++++++++++++++---------------------------
block/blk-core.c | 11 ++-
block/blk.h | 2 +-
block/elevator.c | 79 ++--------------
include/linux/blk_types.h | 2 -
include/linux/blkdev.h | 19 ++---
6 files changed, 113 insertions(+), 220 deletions(-)
@@ -58,16 +32,22 @@ bool blk_ordered_complete_seq(struct request_queue *q, unsigned seq, int error)BUG_ON(q->ordseq&seq);q->ordseq|=seq;-if(blk_ordered_cur_seq(q)!=QUEUE_ORDSEQ_DONE)-returnfalse;--/*-*Okay,sequencecomplete.-*/-q->ordseq=0;-rq=q->orig_bar_rq;-__blk_end_request_all(rq,q->orderr);-returntrue;+if(blk_ordered_cur_seq(q)!=QUEUE_ORDSEQ_DONE){+/* not complete yet, queue the next ordered sequence */+next_rq=queue_next_ordseq(q);+}else{+/* complete this barrier request */+__blk_end_request_all(q->orig_bar_rq,q->orderr);+q->orig_bar_rq=NULL;+q->ordseq=0;++/* dispatch the next barrier if there's one */+if(!list_empty(&q->pending_barriers)){+next_rq=list_entry_rq(q->pending_barriers.next);+list_move(&next_rq->queuelist,&q->queue_head);+}+}+returnnext_rq;}staticvoidpre_flush_end_io(structrequest*rq,interror)
@@ -88,133 +68,105 @@ static void post_flush_end_io(struct request *rq, int error)blk_ordered_complete_seq(rq->q,QUEUE_ORDSEQ_POSTFLUSH,error);}-staticvoidqueue_flush(structrequest_queue*q,unsignedwhich)+staticvoidqueue_flush(structrequest_queue*q,structrequest*rq,+rq_end_io_fn*end_io){-structrequest*rq;-rq_end_io_fn*end_io;--if(which==QUEUE_ORDERED_DO_PREFLUSH){-rq=&q->pre_flush_rq;-end_io=pre_flush_end_io;-}else{-rq=&q->post_flush_rq;-end_io=post_flush_end_io;-}-blk_rq_init(q,rq);rq->cmd_type=REQ_TYPE_FS;-rq->cmd_flags=REQ_HARDBARRIER|REQ_FLUSH;+rq->cmd_flags=REQ_FLUSH;rq->rq_disk=q->orig_bar_rq->rq_disk;rq->end_io=end_io;elv_insert(q,rq,ELEVATOR_INSERT_FRONT);}-staticinlinestructrequest*start_ordered(structrequest_queue*q,-structrequest*rq)+staticstructrequest*queue_next_ordseq(structrequest_queue*q){-unsignedskip=0;--q->orderr=0;-q->ordered=q->next_ordered;-q->ordseq|=QUEUE_ORDSEQ_STARTED;--/*-*Foranemptybarrier,there'snoactualBARrequest,which-*inturnmakesPOSTFLUSHunnecessary.Maskthemoff.-*/-if(!blk_rq_sectors(rq))-q->ordered&=~(QUEUE_ORDERED_DO_BAR|-QUEUE_ORDERED_DO_POSTFLUSH);--/* stash away the original request */-blk_dequeue_request(rq);-q->orig_bar_rq=rq;-rq=NULL;--/*-*Queueorderedsequence.Aswestackthematthehead,we-*needtoqueueinreverseorder.Notethatwerelyonthat-*nofsrequestusesELEVATOR_INSERT_FRONTandthusnofs-*requestgetsinbetweenorderedsequence.-*/-if(q->ordered&QUEUE_ORDERED_DO_POSTFLUSH){-queue_flush(q,QUEUE_ORDERED_DO_POSTFLUSH);-rq=&q->post_flush_rq;-}else-skip|=QUEUE_ORDSEQ_POSTFLUSH;+structrequest*rq=&q->bar_rq;-if(q->ordered&QUEUE_ORDERED_DO_BAR){-rq=&q->bar_rq;+switch(blk_ordered_cur_seq(q)){+caseQUEUE_ORDSEQ_PREFLUSH:+queue_flush(q,rq,pre_flush_end_io);+break;+caseQUEUE_ORDSEQ_BAR:/* initialize proxy request and queue it */blk_rq_init(q,rq);init_request_from_bio(rq,q->orig_bar_rq->bio);+rq->cmd_flags&=~REQ_HARDBARRIER;if(q->ordered&QUEUE_ORDERED_DO_FUA)rq->cmd_flags|=REQ_FUA;rq->end_io=bar_end_io;elv_insert(q,rq,ELEVATOR_INSERT_FRONT);-}else-skip|=QUEUE_ORDSEQ_BAR;+break;-if(q->ordered&QUEUE_ORDERED_DO_PREFLUSH){-queue_flush(q,QUEUE_ORDERED_DO_PREFLUSH);-rq=&q->pre_flush_rq;-}else-skip|=QUEUE_ORDSEQ_PREFLUSH;+caseQUEUE_ORDSEQ_POSTFLUSH:+queue_flush(q,rq,post_flush_end_io);+break;-if(queue_in_flight(q))-rq=NULL;-else-skip|=QUEUE_ORDSEQ_DRAIN;--/*-*Completeskippedsequences.Ifwholesequenceiscomplete,-*return%NULLtotellelevatorthatthisrequestisgone.-*/-if(blk_ordered_complete_seq(q,skip,0))-rq=NULL;+default:+BUG();+}returnrq;}structrequest*blk_do_ordered(structrequest_queue*q,structrequest*rq){-constintis_barrier=rq->cmd_type==REQ_TYPE_FS&&-(rq->cmd_flags&REQ_HARDBARRIER);--if(!q->ordseq){-if(!is_barrier)-returnrq;--if(q->next_ordered!=QUEUE_ORDERED_NONE)-returnstart_ordered(q,rq);-else{-/*-*Queueorderingnotsupported.Terminate-*withprejudice.-*/-blk_dequeue_request(rq);-__blk_end_request_all(rq,-EOPNOTSUPP);-returnNULL;-}+unsignedskip=0;++if(!(rq->cmd_flags&REQ_HARDBARRIER))+returnrq;++if(q->ordseq){+/*+*Barrierisalreadyinprogressandtheycan'tbe+*processedinparallel.Queueforlaterprocessing.+*/+list_move_tail(&rq->queuelist,&q->pending_barriers);+returnNULL;+}++if(unlikely(q->next_ordered==QUEUE_ORDERED_NONE)){+/*+*Queueorderingnotsupported.Terminate+*withprejudice.+*/+blk_dequeue_request(rq);+__blk_end_request_all(rq,-EOPNOTSUPP);+returnNULL;}/*-*Orderedsequenceinprogress+*Startaneworderedsequence*/+q->orderr=0;+q->ordered=q->next_ordered;+q->ordseq|=QUEUE_ORDSEQ_STARTED;-/* Special requests are not subject to ordering rules. */-if(rq->cmd_type!=REQ_TYPE_FS&&-rq!=&q->pre_flush_rq&&rq!=&q->post_flush_rq)-returnrq;+/*+*Foranemptybarrier,there'snoactualBARrequest,which+*inturnmakesPOSTFLUSHunnecessary.Maskthemoff.+*/+if(!blk_rq_sectors(rq))+q->ordered&=~(QUEUE_ORDERED_DO_BAR|+QUEUE_ORDERED_DO_POSTFLUSH);-/* Ordered by draining. Wait for turn. */-WARN_ON(blk_ordered_req_seq(rq)<blk_ordered_cur_seq(q));-if(blk_ordered_req_seq(rq)>blk_ordered_cur_seq(q))-rq=ERR_PTR(-EAGAIN);+/* stash away the original request */+blk_dequeue_request(rq);+q->orig_bar_rq=rq;-returnrq;+if(!(q->ordered&QUEUE_ORDERED_DO_PREFLUSH))+skip|=QUEUE_ORDSEQ_PREFLUSH;++if(!(q->ordered&QUEUE_ORDERED_DO_BAR))+skip|=QUEUE_ORDSEQ_BAR;++if(!(q->ordered&QUEUE_ORDERED_DO_POSTFLUSH))+skip|=QUEUE_ORDSEQ_POSTFLUSH;++/* complete skipped sequences and return the first sequence */+returnblk_ordered_complete_seq(q,skip,0);}staticvoidbio_end_empty_barrier(structbio*bio,interr)
@@ -520,6 +520,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id)init_timer(&q->unplug_timer);setup_timer(&q->timeout,blk_rq_timed_out_timer,(unsignedlong)q);INIT_LIST_HEAD(&q->timeout_list);+INIT_LIST_HEAD(&q->pending_barriers);INIT_WORK(&q->unplug_work,blk_unplug_work);kobject_init(&q->kobj,&blk_queue_ktype);
@@ -1185,6 +1186,7 @@ static int __make_request(struct request_queue *q, struct bio *bio)constboolsync=(bio->bi_rw&REQ_SYNC);constboolunplug=(bio->bi_rw&REQ_UNPLUG);constunsignedintff=bio->bi_rw&REQ_FAILFAST_MASK;+intwhere=ELEVATOR_INSERT_SORT;intrw_flags;/* REQ_HARDBARRIER is no more */
@@ -1203,7 +1205,12 @@ static int __make_request(struct request_queue *q, struct bio *bio)spin_lock_irq(q->queue_lock);-if(unlikely((bio->bi_rw&REQ_HARDBARRIER))||elv_queue_empty(q))+if(bio->bi_rw&REQ_HARDBARRIER){+where=ELEVATOR_INSERT_FRONT;+gotoget_rq;+}++if(elv_queue_empty(q))gotoget_rq;el_ret=elv_merge(q,&req,bio);
@@ -1303,7 +1310,7 @@ get_rq:/* insert the request into the elevator */drive_stat_acct(req,1);-__elv_add_request(q,req,ELEVATOR_INSERT_SORT,0);+__elv_add_request(q,req,where,0);out:if(unplug||!queue_should_plug(q))__generic_unplug_device(q);
@@ -141,7 +141,6 @@ enum rq_flag_bits {__REQ_FAILED,/* set if the request failed */__REQ_QUIET,/* don't worry about errors */__REQ_PREEMPT,/* set for "ide_preempt" requests */-__REQ_ORDERED_COLOR,/* is before or after barrier */__REQ_ALLOCED,/* request came from our alloc pool */__REQ_COPY_USER,/* contains copies of user pages */__REQ_INTEGRITY,/* integrity metadata has been remapped */
@@ -490,12 +491,11 @@ enum {/**Orderedoperationsequence*/-QUEUE_ORDSEQ_STARTED=0x01,/* flushing in progress */-QUEUE_ORDSEQ_DRAIN=0x02,/* waiting for the queue to be drained */-QUEUE_ORDSEQ_PREFLUSH=0x04,/* pre-flushing in progress */-QUEUE_ORDSEQ_BAR=0x08,/* original barrier req in progress */-QUEUE_ORDSEQ_POSTFLUSH=0x10,/* post-flushing in progress */-QUEUE_ORDSEQ_DONE=0x20,+QUEUE_ORDSEQ_STARTED=(1<<0),/* flushing in progress */+QUEUE_ORDSEQ_PREFLUSH=(1<<1),/* pre-flushing in progress */+QUEUE_ORDSEQ_BAR=(1<<2),/* barrier write in progress */+QUEUE_ORDSEQ_POSTFLUSH=(1<<3),/* post-flushing in progress */+QUEUE_ORDSEQ_DONE=(1<<4),};#define blk_queue_plugged(q) test_bit(QUEUE_FLAG_PLUGGED, &(q)->queue_flags)
@@ -1,248 +0,0 @@-/*- * Functions related to barrier IO handling- */-#include <linux/kernel.h>-#include <linux/module.h>-#include <linux/bio.h>-#include <linux/blkdev.h>-#include <linux/gfp.h>--#include "blk.h"--static struct request *queue_next_ordseq(struct request_queue *q);--/*- * Cache flushing for ordered writes handling- */-unsigned blk_ordered_cur_seq(struct request_queue *q)-{- if (!q->ordseq)- return 0;- return 1 << ffz(q->ordseq);-}--static struct request *blk_ordered_complete_seq(struct request_queue *q,- unsigned seq, int error)-{- struct request *next_rq = NULL;-- if (error && !q->orderr)- q->orderr = error;-- BUG_ON(q->ordseq & seq);- q->ordseq |= seq;-- if (blk_ordered_cur_seq(q) != QUEUE_ORDSEQ_DONE) {- /* not complete yet, queue the next ordered sequence */- next_rq = queue_next_ordseq(q);- } else {- /* complete this barrier request */- __blk_end_request_all(q->orig_bar_rq, q->orderr);- q->orig_bar_rq = NULL;- q->ordseq = 0;-- /* dispatch the next barrier if there's one */- if (!list_empty(&q->pending_barriers)) {- next_rq = list_entry_rq(q->pending_barriers.next);- list_move(&next_rq->queuelist, &q->queue_head);- }- }- return next_rq;-}--static void pre_flush_end_io(struct request *rq, int error)-{- elv_completed_request(rq->q, rq);- blk_ordered_complete_seq(rq->q, QUEUE_ORDSEQ_PREFLUSH, error);-}--static void bar_end_io(struct request *rq, int error)-{- elv_completed_request(rq->q, rq);- blk_ordered_complete_seq(rq->q, QUEUE_ORDSEQ_BAR, error);-}--static void post_flush_end_io(struct request *rq, int error)-{- elv_completed_request(rq->q, rq);- blk_ordered_complete_seq(rq->q, QUEUE_ORDSEQ_POSTFLUSH, error);-}--static void queue_flush(struct request_queue *q, struct request *rq,- rq_end_io_fn *end_io)-{- blk_rq_init(q, rq);- rq->cmd_type = REQ_TYPE_FS;- rq->cmd_flags = REQ_FLUSH;- rq->rq_disk = q->orig_bar_rq->rq_disk;- rq->end_io = end_io;-- elv_insert(q, rq, ELEVATOR_INSERT_FRONT);-}--static struct request *queue_next_ordseq(struct request_queue *q)-{- struct request *rq = &q->bar_rq;-- switch (blk_ordered_cur_seq(q)) {- case QUEUE_ORDSEQ_PREFLUSH:- queue_flush(q, rq, pre_flush_end_io);- break;-- case QUEUE_ORDSEQ_BAR:- /* initialize proxy request and queue it */- blk_rq_init(q, rq);- init_request_from_bio(rq, q->orig_bar_rq->bio);- rq->cmd_flags &= ~REQ_HARDBARRIER;- if (q->ordered & QUEUE_ORDERED_DO_FUA)- rq->cmd_flags |= REQ_FUA;- rq->end_io = bar_end_io;-- elv_insert(q, rq, ELEVATOR_INSERT_FRONT);- break;-- case QUEUE_ORDSEQ_POSTFLUSH:- queue_flush(q, rq, post_flush_end_io);- break;-- default:- BUG();- }- return rq;-}--struct request *blk_do_ordered(struct request_queue *q, struct request *rq)-{- unsigned skip = 0;-- if (!(rq->cmd_flags & REQ_HARDBARRIER))- return rq;-- if (q->ordseq) {- /*- * Barrier is already in progress and they can't be- * processed in parallel. Queue for later processing.- */- list_move_tail(&rq->queuelist, &q->pending_barriers);- return NULL;- }-- if (unlikely(q->next_ordered == QUEUE_ORDERED_NONE)) {- /*- * Queue ordering not supported. Terminate- * with prejudice.- */- blk_dequeue_request(rq);- __blk_end_request_all(rq, -EOPNOTSUPP);- return NULL;- }-- /*- * Start a new ordered sequence- */- q->orderr = 0;- q->ordered = q->next_ordered;- q->ordseq |= QUEUE_ORDSEQ_STARTED;-- /*- * For an empty barrier, there's no actual BAR request, which- * in turn makes POSTFLUSH unnecessary. Mask them off.- */- if (!blk_rq_sectors(rq))- q->ordered &= ~(QUEUE_ORDERED_DO_BAR |- QUEUE_ORDERED_DO_POSTFLUSH);-- /* stash away the original request */- blk_dequeue_request(rq);- q->orig_bar_rq = rq;-- if (!(q->ordered & QUEUE_ORDERED_DO_PREFLUSH))- skip |= QUEUE_ORDSEQ_PREFLUSH;-- if (!(q->ordered & QUEUE_ORDERED_DO_BAR))- skip |= QUEUE_ORDSEQ_BAR;-- if (!(q->ordered & QUEUE_ORDERED_DO_POSTFLUSH))- skip |= QUEUE_ORDSEQ_POSTFLUSH;-- /* complete skipped sequences and return the first sequence */- return blk_ordered_complete_seq(q, skip, 0);-}--static void bio_end_empty_barrier(struct bio *bio, int err)-{- if (err) {- if (err == -EOPNOTSUPP)- set_bit(BIO_EOPNOTSUPP, &bio->bi_flags);- clear_bit(BIO_UPTODATE, &bio->bi_flags);- }- if (bio->bi_private)- complete(bio->bi_private);- bio_put(bio);-}--/**- * blkdev_issue_flush - queue a flush- * @bdev: blockdev to issue flush for- * @gfp_mask: memory allocation flags (for bio_alloc)- * @error_sector: error sector- * @flags: BLKDEV_IFL_* flags to control behaviour- *- * Description:- * Issue a flush for the block device in question. Caller can supply- * room for storing the error offset in case of a flush error, if they- * wish to. If WAIT flag is not passed then caller may check only what- * request was pushed in some internal queue for later handling.- */-int blkdev_issue_flush(struct block_device *bdev, gfp_t gfp_mask,- sector_t *error_sector, unsigned long flags)-{- DECLARE_COMPLETION_ONSTACK(wait);- struct request_queue *q;- struct bio *bio;- int ret = 0;-- if (bdev->bd_disk == NULL)- return -ENXIO;-- q = bdev_get_queue(bdev);- if (!q)- return -ENXIO;-- /*- * some block devices may not have their queue correctly set up here- * (e.g. loop device without a backing file) and so issuing a flush- * here will panic. Ensure there is a request function before issuing- * the barrier.- */- if (!q->make_request_fn)- return -ENXIO;-- bio = bio_alloc(gfp_mask, 0);- bio->bi_end_io = bio_end_empty_barrier;- bio->bi_bdev = bdev;- if (test_bit(BLKDEV_WAIT, &flags))- bio->bi_private = &wait;-- bio_get(bio);- submit_bio(WRITE_BARRIER, bio);- if (test_bit(BLKDEV_WAIT, &flags)) {- wait_for_completion(&wait);- /*- * The driver must store the error location in ->bi_sector, if- * it supports it. For non-stacked drivers, this should be- * copied from blk_rq_pos(rq).- */- if (error_sector)- *error_sector = bio->bi_sector;- }-- if (bio_flagged(bio, BIO_EOPNOTSUPP))- ret = -EOPNOTSUPP;- else if (!bio_flagged(bio, BIO_UPTODATE))- ret = -EIO;-- bio_put(bio);- return ret;-}-EXPORT_SYMBOL(blkdev_issue_flush);
@@ -0,0 +1,248 @@+/*+*FunctionsrelatedtobarrierIOhandling+*/+#include<linux/kernel.h>+#include<linux/module.h>+#include<linux/bio.h>+#include<linux/blkdev.h>+#include<linux/gfp.h>++#include"blk.h"++staticstructrequest*queue_next_ordseq(structrequest_queue*q);++/*+*Cacheflushingfororderedwriteshandling+*/+unsignedblk_ordered_cur_seq(structrequest_queue*q)+{+if(!q->ordseq)+return0;+return1<<ffz(q->ordseq);+}++staticstructrequest*blk_ordered_complete_seq(structrequest_queue*q,+unsignedseq,interror)+{+structrequest*next_rq=NULL;++if(error&&!q->orderr)+q->orderr=error;++BUG_ON(q->ordseq&seq);+q->ordseq|=seq;++if(blk_ordered_cur_seq(q)!=QUEUE_ORDSEQ_DONE){+/* not complete yet, queue the next ordered sequence */+next_rq=queue_next_ordseq(q);+}else{+/* complete this barrier request */+__blk_end_request_all(q->orig_bar_rq,q->orderr);+q->orig_bar_rq=NULL;+q->ordseq=0;++/* dispatch the next barrier if there's one */+if(!list_empty(&q->pending_barriers)){+next_rq=list_entry_rq(q->pending_barriers.next);+list_move(&next_rq->queuelist,&q->queue_head);+}+}+returnnext_rq;+}++staticvoidpre_flush_end_io(structrequest*rq,interror)+{+elv_completed_request(rq->q,rq);+blk_ordered_complete_seq(rq->q,QUEUE_ORDSEQ_PREFLUSH,error);+}++staticvoidbar_end_io(structrequest*rq,interror)+{+elv_completed_request(rq->q,rq);+blk_ordered_complete_seq(rq->q,QUEUE_ORDSEQ_BAR,error);+}++staticvoidpost_flush_end_io(structrequest*rq,interror)+{+elv_completed_request(rq->q,rq);+blk_ordered_complete_seq(rq->q,QUEUE_ORDSEQ_POSTFLUSH,error);+}++staticvoidqueue_flush(structrequest_queue*q,structrequest*rq,+rq_end_io_fn*end_io)+{+blk_rq_init(q,rq);+rq->cmd_type=REQ_TYPE_FS;+rq->cmd_flags=REQ_FLUSH;+rq->rq_disk=q->orig_bar_rq->rq_disk;+rq->end_io=end_io;++elv_insert(q,rq,ELEVATOR_INSERT_FRONT);+}++staticstructrequest*queue_next_ordseq(structrequest_queue*q)+{+structrequest*rq=&q->bar_rq;++switch(blk_ordered_cur_seq(q)){+caseQUEUE_ORDSEQ_PREFLUSH:+queue_flush(q,rq,pre_flush_end_io);+break;++caseQUEUE_ORDSEQ_BAR:+/* initialize proxy request and queue it */+blk_rq_init(q,rq);+init_request_from_bio(rq,q->orig_bar_rq->bio);+rq->cmd_flags&=~REQ_HARDBARRIER;+if(q->ordered&QUEUE_ORDERED_DO_FUA)+rq->cmd_flags|=REQ_FUA;+rq->end_io=bar_end_io;++elv_insert(q,rq,ELEVATOR_INSERT_FRONT);+break;++caseQUEUE_ORDSEQ_POSTFLUSH:+queue_flush(q,rq,post_flush_end_io);+break;++default:+BUG();+}+returnrq;+}++structrequest*blk_do_ordered(structrequest_queue*q,structrequest*rq)+{+unsignedskip=0;++if(!(rq->cmd_flags&REQ_HARDBARRIER))+returnrq;++if(q->ordseq){+/*+*Barrierisalreadyinprogressandtheycan'tbe+*processedinparallel.Queueforlaterprocessing.+*/+list_move_tail(&rq->queuelist,&q->pending_barriers);+returnNULL;+}++if(unlikely(q->next_ordered==QUEUE_ORDERED_NONE)){+/*+*Queueorderingnotsupported.Terminate+*withprejudice.+*/+blk_dequeue_request(rq);+__blk_end_request_all(rq,-EOPNOTSUPP);+returnNULL;+}++/*+*Startaneworderedsequence+*/+q->orderr=0;+q->ordered=q->next_ordered;+q->ordseq|=QUEUE_ORDSEQ_STARTED;++/*+*Foranemptybarrier,there'snoactualBARrequest,which+*inturnmakesPOSTFLUSHunnecessary.Maskthemoff.+*/+if(!blk_rq_sectors(rq))+q->ordered&=~(QUEUE_ORDERED_DO_BAR|+QUEUE_ORDERED_DO_POSTFLUSH);++/* stash away the original request */+blk_dequeue_request(rq);+q->orig_bar_rq=rq;++if(!(q->ordered&QUEUE_ORDERED_DO_PREFLUSH))+skip|=QUEUE_ORDSEQ_PREFLUSH;++if(!(q->ordered&QUEUE_ORDERED_DO_BAR))+skip|=QUEUE_ORDSEQ_BAR;++if(!(q->ordered&QUEUE_ORDERED_DO_POSTFLUSH))+skip|=QUEUE_ORDSEQ_POSTFLUSH;++/* complete skipped sequences and return the first sequence */+returnblk_ordered_complete_seq(q,skip,0);+}++staticvoidbio_end_empty_barrier(structbio*bio,interr)+{+if(err){+if(err==-EOPNOTSUPP)+set_bit(BIO_EOPNOTSUPP,&bio->bi_flags);+clear_bit(BIO_UPTODATE,&bio->bi_flags);+}+if(bio->bi_private)+complete(bio->bi_private);+bio_put(bio);+}++/**+*blkdev_issue_flush-queueaflush+*@bdev:blockdevtoissueflushfor+*@gfp_mask:memoryallocationflags(forbio_alloc)+*@error_sector:errorsector+*@flags:BLKDEV_IFL_*flagstocontrolbehaviour+*+*Description:+*Issueaflushfortheblockdeviceinquestion.Callercansupply+*roomforstoringtheerroroffsetincaseofaflusherror,ifthey+*wishto.IfWAITflagisnotpassedthencallermaycheckonlywhat+*requestwaspushedinsomeinternalqueueforlaterhandling.+*/+intblkdev_issue_flush(structblock_device*bdev,gfp_tgfp_mask,+sector_t*error_sector,unsignedlongflags)+{+DECLARE_COMPLETION_ONSTACK(wait);+structrequest_queue*q;+structbio*bio;+intret=0;++if(bdev->bd_disk==NULL)+return-ENXIO;++q=bdev_get_queue(bdev);+if(!q)+return-ENXIO;++/*+*someblockdevicesmaynothavetheirqueuecorrectlysetuphere+*(e.g.loopdevicewithoutabackingfile)andsoissuingaflush+*herewillpanic.Ensurethereisarequestfunctionbeforeissuing+*thebarrier.+*/+if(!q->make_request_fn)+return-ENXIO;++bio=bio_alloc(gfp_mask,0);+bio->bi_end_io=bio_end_empty_barrier;+bio->bi_bdev=bdev;+if(test_bit(BLKDEV_WAIT,&flags))+bio->bi_private=&wait;++bio_get(bio);+submit_bio(WRITE_BARRIER,bio);+if(test_bit(BLKDEV_WAIT,&flags)){+wait_for_completion(&wait);+/*+*Thedrivermuststoretheerrorlocationin->bi_sector,if+*itsupportsit.Fornon-stackeddrivers,thisshouldbe+*copiedfromblk_rq_pos(rq).+*/+if(error_sector)+*error_sector=bio->bi_sector;+}++if(bio_flagged(bio,BIO_EOPNOTSUPP))+ret=-EOPNOTSUPP;+elseif(!bio_flagged(bio,BIO_UPTODATE))+ret=-EIO;++bio_put(bio);+returnret;+}+EXPORT_SYMBOL(blkdev_issue_flush);
With ordering requirements dropped, barrier and ordered are misnomers.
Now all block layer does is sequencing FLUSH and FUA. Rename them to
flush.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
block/blk-core.c | 21 +++++-----
block/blk-flush.c | 98 +++++++++++++++++++++++------------------------
block/blk.h | 4 +-
include/linux/blkdev.h | 26 ++++++------
4 files changed, 73 insertions(+), 76 deletions(-)
@@ -9,41 +9,38 @@#include"blk.h"-staticstructrequest*queue_next_ordseq(structrequest_queue*q);+staticstructrequest*queue_next_fseq(structrequest_queue*q);-/*-*Cacheflushingfororderedwriteshandling-*/-unsignedblk_ordered_cur_seq(structrequest_queue*q)+unsignedblk_flush_cur_seq(structrequest_queue*q){-if(!q->ordseq)+if(!q->flush_seq)return0;-return1<<ffz(q->ordseq);+return1<<ffz(q->flush_seq);}-staticstructrequest*blk_ordered_complete_seq(structrequest_queue*q,-unsignedseq,interror)+staticstructrequest*blk_flush_complete_seq(structrequest_queue*q,+unsignedseq,interror){structrequest*next_rq=NULL;-if(error&&!q->orderr)-q->orderr=error;+if(error&&!q->flush_err)+q->flush_err=error;-BUG_ON(q->ordseq&seq);-q->ordseq|=seq;+BUG_ON(q->flush_seq&seq);+q->flush_seq|=seq;-if(blk_ordered_cur_seq(q)!=QUEUE_ORDSEQ_DONE){-/* not complete yet, queue the next ordered sequence */-next_rq=queue_next_ordseq(q);+if(blk_flush_cur_seq(q)!=QUEUE_FSEQ_DONE){+/* not complete yet, queue the next flush sequence */+next_rq=queue_next_fseq(q);}else{-/* complete this barrier request */-__blk_end_request_all(q->orig_bar_rq,q->orderr);-q->orig_bar_rq=NULL;-q->ordseq=0;--/* dispatch the next barrier if there's one */-if(!list_empty(&q->pending_barriers)){-next_rq=list_entry_rq(q->pending_barriers.next);+/* complete this flush request */+__blk_end_request_all(q->orig_flush_rq,q->flush_err);+q->orig_flush_rq=NULL;+q->flush_seq=0;++/* dispatch the next flush if there's one */+if(!list_empty(&q->pending_flushes)){+next_rq=list_entry_rq(q->pending_flushes.next);list_move(&next_rq->queuelist,&q->queue_head);}}
@@ -489,13 +489,13 @@ enum {QUEUE_ORDERED_DO_FUA,/*-*Orderedoperationsequence+*FLUSH/FUAsequences.*/-QUEUE_ORDSEQ_STARTED=(1<<0),/* flushing in progress */-QUEUE_ORDSEQ_PREFLUSH=(1<<1),/* pre-flushing in progress */-QUEUE_ORDSEQ_BAR=(1<<2),/* barrier write in progress */-QUEUE_ORDSEQ_POSTFLUSH=(1<<3),/* post-flushing in progress */-QUEUE_ORDSEQ_DONE=(1<<4),+QUEUE_FSEQ_STARTED=(1<<0),/* flushing in progress */+QUEUE_FSEQ_PREFLUSH=(1<<1),/* pre-flushing in progress */+QUEUE_FSEQ_DATA=(1<<2),/* data write in progress */+QUEUE_FSEQ_POSTFLUSH=(1<<3),/* post-flushing in progress */+QUEUE_FSEQ_DONE=(1<<4),};#define blk_queue_plugged(q) test_bit(QUEUE_FLAG_PLUGGED, &(q)->queue_flags)
Now that the backend conversion is complete, export sequenced
FLUSH/FUA capability through REQ_FLUSH/FUA flags. REQ_FLUSH means the
device cache should be flushed before executing the request. REQ_FUA
means that the data in the request should be on non-volatile media on
completion.
Block layer will choose the correct way of implementing the semantics
and execute it. The request may be passed to the device directly if
the device can handle it; otherwise, it will be sequenced using one or
more proxy requests. Devices will never see REQ_FLUSH and/or FUA
which it doesn't support.
* QUEUE_ORDERED_* are removed and QUEUE_FSEQ_* are moved into
blk-flush.c.
* REQ_FLUSH w/o data can also be directly passed to drivers without
sequencing but some drivers assume that zero length requests don't
have rq->bio which isn't true for these requests requiring the use
of proxy requests.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
block/blk-core.c | 2 +-
block/blk-flush.c | 85 ++++++++++++++++++++++++++----------------------
block/blk.h | 3 ++
include/linux/blkdev.h | 38 +--------------------
4 files changed, 52 insertions(+), 76 deletions(-)
@@ -9,6 +9,15 @@#include"blk.h"+/* FLUSH/FUA sequences */+enum{+QUEUE_FSEQ_STARTED=(1<<0),/* flushing in progress */+QUEUE_FSEQ_PREFLUSH=(1<<1),/* pre-flushing in progress */+QUEUE_FSEQ_DATA=(1<<2),/* data write in progress */+QUEUE_FSEQ_POSTFLUSH=(1<<3),/* post-flushing in progress */+QUEUE_FSEQ_DONE=(1<<4),+};+staticstructrequest*queue_next_fseq(structrequest_queue*q);unsignedblk_flush_cur_seq(structrequest_queue*q)
@@ -87,12 +97,11 @@ static struct request *queue_next_fseq(struct request_queue *q)break;caseQUEUE_FSEQ_DATA:-/* initialize proxy request and queue it */+/* initialize proxy request, inherit FLUSH/FUA and queue it */blk_rq_init(q,rq);-init_request_from_bio(rq,q->orig_flush_rq->bio);-rq->cmd_flags&=~REQ_HARDBARRIER;-if(q->ordered&QUEUE_ORDERED_DO_FUA)-rq->cmd_flags|=REQ_FUA;+init_request_from_bio(rq,orig_rq->bio);+rq->cmd_flags&=~(REQ_FLUSH|REQ_FUA);+rq->cmd_flags|=orig_rq->cmd_flags&(REQ_FLUSH|REQ_FUA);rq->end_io=flush_data_end_io;elv_insert(q,rq,ELEVATOR_INSERT_FRONT);
@@ -110,60 +119,58 @@ static struct request *queue_next_fseq(struct request_queue *q)structrequest*blk_do_flush(structrequest_queue*q,structrequest*rq){+unsignedintfflags=q->flush_flags;/* may change, cache it */+boolhas_flush=fflags&REQ_FLUSH,has_fua=fflags&REQ_FUA;+booldo_preflush=has_flush&&(rq->cmd_flags&REQ_FLUSH);+booldo_postflush=has_flush&&!has_fua&&(rq->cmd_flags&REQ_FUA);unsignedskip=0;-if(!(rq->cmd_flags&REQ_HARDBARRIER))+/*+*Specialcase.Ifthere'sdatabutflushisnotnecessary,+*therequestcanbeissueddirectly.+*+*Flushw/odatashouldbeabletobeissueddirectlytoobut+*currentlysomedriversassumethatrq->biocontains+*non-zerodataifitisn'tNULLandemptyFLUSHrequests+*gettinghereusuallyhavebio'swithoutdata.+*/+if(blk_rq_sectors(rq)&&!do_preflush&&!do_postflush){+rq->cmd_flags&=~REQ_FLUSH;+if(!has_fua)+rq->cmd_flags&=~REQ_FUA;returnrq;+}+/*+*Sequencedflushescan'tbeprocessedinparallel.If+*anotheroneisalreadyinprogress,queueforlater+*processing.+*/if(q->flush_seq){-/*-*Sequencedflushisalreadyinprogressandthey-*can'tbeprocessedinparallel.Queueforlater-*processing.-*/list_move_tail(&rq->queuelist,&q->pending_flushes);returnNULL;}-if(unlikely(q->next_ordered==QUEUE_ORDERED_NONE)){-/*-*Queueorderingnotsupported.Terminate-*withprejudice.-*/-blk_dequeue_request(rq);-__blk_end_request_all(rq,-EOPNOTSUPP);-returnNULL;-}-/**Startanewflushsequence*/q->flush_err=0;-q->ordered=q->next_ordered;q->flush_seq|=QUEUE_FSEQ_STARTED;-/*-*Foranemptybarrier,there'snoactualBARrequest,which-*inturnmakesPOSTFLUSHunnecessary.Maskthemoff.-*/-if(!blk_rq_sectors(rq))-q->ordered&=~(QUEUE_ORDERED_DO_BAR|-QUEUE_ORDERED_DO_POSTFLUSH);--/* stash away the original request */+/* adjust FLUSH/FUA of the original request and stash it away */+rq->cmd_flags&=~REQ_FLUSH;+if(!has_fua)+rq->cmd_flags&=~REQ_FUA;blk_dequeue_request(rq);q->orig_flush_rq=rq;-if(!(q->ordered&QUEUE_ORDERED_DO_PREFLUSH))+/* skip unneded sequences and return the first one */+if(!do_preflush)skip|=QUEUE_FSEQ_PREFLUSH;--if(!(q->ordered&QUEUE_ORDERED_DO_BAR))+if(!blk_rq_sectors(rq))skip|=QUEUE_FSEQ_DATA;--if(!(q->ordered&QUEUE_ORDERED_DO_POSTFLUSH))+if(!do_postflush)skip|=QUEUE_FSEQ_POSTFLUSH;--/* complete skipped sequences and return the first sequence */returnblk_flush_complete_seq(q,skip,0);}
Propagate deprecation of REQ_HARDBARRIER and new REQ_FLUSH/FUA
interface to upper layers.
* WRITE_BARRIER is marked deprecated and WRITE_FLUSH, WRITE_FUA and
WRITE_FLUSH_FUA are added.
* REQ_COMMON_MASK now includes REQ_FLUSH | REQ_FUA so that they are
copied from bio to request.
* BH_Ordered is marked deprecated and BH_Flush and BH_FUA are added.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
fs/buffer.c | 27 ++++++++++++++++-----------
include/linux/blk_types.h | 2 +-
include/linux/buffer_head.h | 8 ++++++--
include/linux/fs.h | 20 +++++++++++++-------
4 files changed, 36 insertions(+), 21 deletions(-)
@@ -3019,18 +3019,23 @@ int submit_bh(int rw, struct buffer_head * bh)BUG_ON(buffer_delay(bh));BUG_ON(buffer_unwritten(bh));-/*-*Maskinbarrierbitforawrite(couldbeeitheraWRITEora-*WRITE_SYNC-*/-if(buffer_ordered(bh)&&(rw&WRITE))-rw|=WRITE_BARRIER;+if(rw&WRITE){+/* ordered is deprecated, will be removed */+if(buffer_ordered(bh))+rw|=WRITE_BARRIER;-/*-*Onlyclearoutawriteerrorwhenrewriting-*/-if(test_set_buffer_req(bh)&&(rw&WRITE))-clear_buffer_write_io_error(bh);+if(buffer_flush(bh))+rw|=WRITE_FLUSH;++if(buffer_fua(bh))+rw|=WRITE_FUA;++/*+*Onlyclearoutawriteerrorwhenrewriting+*/+if(test_set_buffer_req(bh))+clear_buffer_write_io_error(bh);+}/**fromhereondown,it'sallbio--dotheinitialmapping,
@@ -32,8 +32,10 @@ enum bh_state_bits {BH_Delay,/* Buffer is not yet allocated on disk */BH_Boundary,/* Block is followed by a discontiguity */BH_Write_EIO,/* I/O error on write */-BH_Ordered,/* ordered write */-BH_Eopnotsupp,/* operation not supported (barrier) */+BH_Ordered,/* DEPRECATED: ordered write */+BH_Eopnotsupp,/* DEPRECATED: operation not supported (barrier) */+BH_Flush,/* Flush device cache before executing IO */+BH_FUA,/* Data should be on non-volatile media on completion */BH_Unwritten,/* Buffer is allocated on disk but not written */BH_Quiet,/* Buffer Error Prinks to be quiet */
From: Jan Kara <jack@suse.cz> Date: 2010-08-12 21:24:04
On Thu 12-08-10 14:41:30, Tejun Heo wrote:
Propagate deprecation of REQ_HARDBARRIER and new REQ_FLUSH/FUA
interface to upper layers.
* WRITE_BARRIER is marked deprecated and WRITE_FLUSH, WRITE_FUA and
WRITE_FLUSH_FUA are added.
* REQ_COMMON_MASK now includes REQ_FLUSH | REQ_FUA so that they are
copied from bio to request.
* BH_Ordered is marked deprecated and BH_Flush and BH_FUA are added.
Deprecating BH_Ordered is fine but I wouldn't introduce new BH flags for
this. BH flags should be used for buffer state, not for encoding how the
buffer should be written (there were actually bugs in the past because of
this). Being able to set proper flags when calling submit_bh() in the rw
parameter is enough.
Honza
@@ -3019,18 +3019,23 @@ int submit_bh(int rw, struct buffer_head * bh)BUG_ON(buffer_delay(bh));BUG_ON(buffer_unwritten(bh));-/*-*Maskinbarrierbitforawrite(couldbeeitheraWRITEora-*WRITE_SYNC-*/-if(buffer_ordered(bh)&&(rw&WRITE))-rw|=WRITE_BARRIER;+if(rw&WRITE){+/* ordered is deprecated, will be removed */+if(buffer_ordered(bh))+rw|=WRITE_BARRIER;-/*-*Onlyclearoutawriteerrorwhenrewriting-*/-if(test_set_buffer_req(bh)&&(rw&WRITE))-clear_buffer_write_io_error(bh);+if(buffer_flush(bh))+rw|=WRITE_FLUSH;++if(buffer_fua(bh))+rw|=WRITE_FUA;++/*+*Onlyclearoutawriteerrorwhenrewriting+*/+if(test_set_buffer_req(bh))+clear_buffer_write_io_error(bh);+}/**fromhereondown,it'sallbio--dotheinitialmapping,
@@ -32,8 +32,10 @@ enum bh_state_bits {BH_Delay,/* Buffer is not yet allocated on disk */BH_Boundary,/* Block is followed by a discontiguity */BH_Write_EIO,/* I/O error on write */-BH_Ordered,/* ordered write */-BH_Eopnotsupp,/* operation not supported (barrier) */+BH_Ordered,/* DEPRECATED: ordered write */+BH_Eopnotsupp,/* DEPRECATED: operation not supported (barrier) */+BH_Flush,/* Flush device cache before executing IO */+BH_FUA,/* Data should be on non-volatile media on completion */BH_Unwritten,/* Buffer is allocated on disk but not written */BH_Quiet,/* Buffer Error Prinks to be quiet */
Hello, Jan.
On 08/12/2010 11:24 PM, Jan Kara wrote:
On Thu 12-08-10 14:41:30, Tejun Heo wrote:
quoted
Propagate deprecation of REQ_HARDBARRIER and new REQ_FLUSH/FUA
interface to upper layers.
* WRITE_BARRIER is marked deprecated and WRITE_FLUSH, WRITE_FUA and
WRITE_FLUSH_FUA are added.
* REQ_COMMON_MASK now includes REQ_FLUSH | REQ_FUA so that they are
copied from bio to request.
* BH_Ordered is marked deprecated and BH_Flush and BH_FUA are added.
Deprecating BH_Ordered is fine but I wouldn't introduce new BH flags for
this. BH flags should be used for buffer state, not for encoding how the
buffer should be written (there were actually bugs in the past because of
this). Being able to set proper flags when calling submit_bh() in the rw
parameter is enough.
Ah, okay, I was just trying to match the BH_Ordered usage but you're
saying just requiring submit_bh() users to specify appropriate REQ_*
(or WRITE_*) in @rw is okay, right? I'll drop the bh part then.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-13 11:48:58
The patchset looks functionally correct to me, and with a small patch
to make use of WRITE_FUA_FLUSH survives xfstests, and instrumenting the
underlying qemu shows that we actually get the flush requests where we should.
No performance or power fail testing done yet.
But I do not like the transition very much. The new WRITE_FUA_FLUSH
request is exactly what filesystems expect from a current barrier
request, so I'd rather move to that functionality without breaking stuff
inbetween.
So if it was to me I'd keep patches 1, 2, 4 and 5 from your series, than
a main one to relax barrier semantics, then have the renaming patches 7
and 8, and possible keep patch 11 separate from the main implementation
change, and if absolutely also a separate one to introduce REQ_FUA and
REQ_FLUSH in the bio interface, but keep things working while doing
this.
Then we can patches do disable the reiserfs barrier "optimization" as
the very first one, and DM/MD support which I'm currently working on
as the last one and we can start doing the heavy testing.
Each filesystem needs to be updated to enforce request
ordering themselves and then to use REQ_FLUSH/FUA mechanism.
I generally agree with the patchset, but I believe this particular move
is a really bad move.
I'm not mentioning the obvious that a common functionality (enforcing
requests ordering in this case) should be handled by a common library,
but not internally by a zillion file systems Linux has.
The worst in this move is that it would hide all the requests ordering
semantic inside file systems in, most likely, a very much unclear way.
That would lead that if I or someone else decide to implement the
"hardware offload" of requests ordering (ORDERED requests), I or he/she
would not be able to see any improvement until at least one file system
be changed to be able to use it. Worse, if the implementor can't
demonstrate the improvement, how can he encourage file systems
developers to update their file systems? Which, basically, would mean
that only a person with *BOTH* deep storage and file systems internals
knowledge can do the job. How many do you know such people? Both storage
and file systems topics are very wide and tricky, so nearly always
people specialize in one of them, not both.
Thus, this move would basically mean that the proper ordered queuing
would probably never be implemented in Linux.
I believe, much better would be to create a common interface, which file
systems would use to enforce requests order, when they need it.
Advantages of this approach:
1. The ordering requirements of file systems would be clear.
2. They would be handled in one place by a common code.
3. Any storage level expert can try to implement ordered queuing without
a deep dive into file systems design and implementation.
I already suggested such interface in
http://marc.info/?l=linux-scsi&m=128077574815881&w=2. Internally for the
moment it can be implemented using existing REQ_FLUSH/FUA/etc. and
waiting for all the requests in the group to finish. As a nice side
effect, if a device doesn't support FUA, it would be possible to issue
SYNC_CACHE command(s) only for required blocks, not for the whole device
as it is done now.
If requested, I can develop the interface further.
Vlad
Hello Tejun,
Tejun Heo, on 08/12/2010 04:41 PM wrote:
Nobody is making meaningful use of ORDERED_BY_TAG now and queue
draining for barrier requests will be removed soon which will render
the advantage of tag ordering moot.
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-13 13:06:57
On Fri, Aug 13, 2010 at 04:56:32PM +0400, Vladislav Bolkhovitin wrote:
Tejun Heo, on 08/12/2010 04:41 PM wrote:
quoted
Nobody is making meaningful use of ORDERED_BY_TAG now and queue
draining for barrier requests will be removed soon which will render
the advantage of tag ordering moot.
It's not tag odering that is a win but big queue depth. That's what you
measured and what I fully agree on. I haven't been able to get out of
Hannes what he actually measured.
And if you'd actually look at the patchset allowing deep queues is
exactly what it allows us, and while I haven't done testing on this
patchset but only on my previous version it does get us back to use
the full potential of large arrays exactly because of that.
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-13 13:17:22
On Fri, Aug 13, 2010 at 04:55:33PM +0400, Vladislav Bolkhovitin wrote:
I'm not mentioning the obvious that a common functionality (enforcing
requests ordering in this case) should be handled by a common library,
but not internally by a zillion file systems Linux has.
I/O ordering is still handled mostly by common code, that is the
pagecache and the buffercache, although a few filesystems like XFS and
btrfs have their own implementation of the second one.
The current ordered semantics of barriers have only successfull
implemented by a complete queue drain, and not effectively been used
by filesystems. This patchset removes the bogus global ordering
enforced by the block layer whenever a filesystems wants to be able
to use cache flushes, and because of that allows deeper outstanding
queue depth I/O with less latency.
Now I know you in particular are a fan of scsi ordered tags. And as I
told you before I'm open to review such an implementation if it shows
us any advantages. Adding it after this patch is in fact not any more
complicated than before, I'd almost be tempted it's easier as you don't
have to plug it into the complex state machine we used for barriers, and
more importantly we drop the requirement for the barrier sequence to
be atomic, which in fact made implementing barriers using tagged queues
impossible with the current scsi layer.
As far as playing with ordered tags it's just adding a new flag for
it on the bio that gets passed down to the driver. For a final version
you'd need a queue-level feature if it's supported, but you don't
even need that for the initial work. Then you can implement a
variant of blk_do_flush that does away with queueing additional requests
once finish but queues all two or three at the same time with your
new ordered flag set, at which point you are back to the level or
ordered tag usage that the old code allows. You're still left with
all the hard problems of actually implementing error handling for it
and using it higher up in the filesystem and generic page cache code.
I'd really love to see your results, up to the point of just trying
that once I get a little spare time. But my theory is that it won't
help us - the problem with ordered tags is that they enforce global
ordering while we currently have local ordering. While it will reduce
the latency for the process waiting for an fsync or similar it will
affect other I/O going on in the background and reduce the devices
ability to reorder that I/O.
So for now this patch set is a massive improvement of performance for
workloads we care about, while removing the interface we put in place
to allow a theoretical optimization that didn't show up for 8 years
before, and in fact made the interface just complicated enough to make
that optimization so hard.
Hello,
On 08/13/2010 02:55 PM, Vladislav Bolkhovitin wrote:
If requested, I can develop the interface further.
I still think the benefit of ordering by tag would be marginal at
best, and what have you guys measured there? Under the current
framework, there's no easy way to measure full ordered-by-tag
implementation. The mechanism for filesystems to communicate the
ordering information (which would be a partially ordered graph) just
isn't there and there is no way the current usage of ordering-by-tag
only for barrier sequence can achieve anything close to that level of
difference.
Ripping out the original ordering by tag mechanism doesn't amount to
much. The use of ordering-by-tag was pretty half-assed there anyway.
If you think exporting full ordering information from filesystem to
the lower layers is worthwhile, please go ahead. It would be very
interesting to see how much actual difference it can make compared to
ordering-by-filesystem and if it's actually better and the added
complexity is manageable, there's no reason not to do that.
Thank you.
--
tejun
Hello, Christoph.
On 08/13/2010 01:48 PM, Christoph Hellwig wrote:
The patchset looks functionally correct to me, and with a small patch
to make use of WRITE_FUA_FLUSH survives xfstests, and instrumenting the
underlying qemu shows that we actually get the flush requests where we should.
Great.
No performance or power fail testing done yet.
But I do not like the transition very much. The new WRITE_FUA_FLUSH
request is exactly what filesystems expect from a current barrier
request, so I'd rather move to that functionality without breaking stuff
inbetween.
So if it was to me I'd keep patches 1, 2, 4 and 5 from your series, than
a main one to relax barrier semantics, then have the renaming patches 7
and 8, and possible keep patch 11 separate from the main implementation
change, and if absolutely also a separate one to introduce REQ_FUA and
REQ_FLUSH in the bio interface, but keep things working while doing
this.
There are two reason to avoid changing the meaning of REQ_HARDBARRIER
and just deprecate it. One is to avoid breaking filesystems'
expectations underneath it. Please note that there are out-of-tree
filesystems too. I think it would be too dangerous to relax
REQ_HARDBARRIER.
Another is that pseudo block layer drivers (loop, virtio_blk,
md/dm...) have assumptions about REQ_HARDBARRIER behavior and things
would be broken in obscure ways between REQ_HARDBARRIER semantics
change and updates to each of those drivers, so I don't really think
changing the semantics while the mechanism is online is a good idea.
Then we can patches do disable the reiserfs barrier "optimization" as
the very first one, and DM/MD support which I'm currently working on
as the last one and we can start doing the heavy testing.
Oops, I've already converted loop, virtio_blk/lguest and am working on
md/dm right now too. I'm almost done with md and now doing dm. :-)
Maybe we should post them right now so that we don't waste too much
time trying to solve the same problems?
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-13 14:38:20
On Fri, Aug 13, 2010 at 03:48:59PM +0200, Tejun Heo wrote:
There are two reason to avoid changing the meaning of REQ_HARDBARRIER
and just deprecate it. One is to avoid breaking filesystems'
expectations underneath it. Please note that there are out-of-tree
filesystems too. I think it would be too dangerous to relax
REQ_HARDBARRIER.
Note that the renaming patch would include a move from REQ_HARDBARRIER
to REQ_FLUSH_FUA, so things just using REQ_HARDBARRIER will fail to
compile. And while out of tree filesystems do exist they it's their
problem to keep up with kernel changes. They decide not to be part
of the Linux kernel, so it'll be their job to keep up with it.
Another is that pseudo block layer drivers (loop, virtio_blk,
md/dm...) have assumptions about REQ_HARDBARRIER behavior and things
would be broken in obscure ways between REQ_HARDBARRIER semantics
change and updates to each of those drivers, so I don't really think
changing the semantics while the mechanism is online is a good idea.
I don't think doing those changes in a separate commit is a good idea.
quoted
Then we can patches do disable the reiserfs barrier "optimization" as
the very first one, and DM/MD support which I'm currently working on
as the last one and we can start doing the heavy testing.
Oops, I've already converted loop, virtio_blk/lguest and am working on
md/dm right now too. I'm almost done with md and now doing dm. :-)
Maybe we should post them right now so that we don't waste too much
time trying to solve the same problems?
Here's the dm patch. It only handles normal bio based dm yet, which
I understand and can test. request based dm (multipath) still needs
work.
Index: linux-2.6/drivers/md/dm-crypt.c
===================================================================
@@ -1581,7 +1581,7 @@ static int snapshot_map(struct dm_targetchunk_tchunk;structdm_snap_pending_exception*pe=NULL;-if(unlikely(bio_empty_barrier(bio))){+if(bio_empty_flush(bio)){bio->bi_bdev=s->cow->bdev;returnDM_MAPIO_REMAPPED;}
@@ -1685,7 +1685,7 @@ static int snapshot_merge_map(struct dm_intr=DM_MAPIO_REMAPPED;chunk_tchunk;-if(unlikely(bio_empty_barrier(bio))){+if(bio_empty_flush(bio)){if(!map_context->flush_request)bio->bi_bdev=s->origin->bdev;else
@@ -2123,7 +2123,7 @@ static int origin_map(struct dm_target *structdm_dev*dev=ti->private;bio->bi_bdev=dev->bdev;-if(unlikely(bio_empty_barrier(bio)))+if(bio_empty_flush(bio))returnDM_MAPIO_REMAPPED;/* Only tell snapshots if this is a write */
@@ -194,9 +183,6 @@ struct mapped_device {/* sysfs handle */structkobjectkobj;--/* zero-length barrier that will be cloned and submitted to targets */-structbiobarrier_bio;};/*
Hello,
On 08/13/2010 04:38 PM, Christoph Hellwig wrote:
On Fri, Aug 13, 2010 at 03:48:59PM +0200, Tejun Heo wrote:
quoted
There are two reason to avoid changing the meaning of REQ_HARDBARRIER
and just deprecate it. One is to avoid breaking filesystems'
expectations underneath it. Please note that there are out-of-tree
filesystems too. I think it would be too dangerous to relax
REQ_HARDBARRIER.
Note that the renaming patch would include a move from REQ_HARDBARRIER
to REQ_FLUSH_FUA, so things just using REQ_HARDBARRIER will fail to
compile. And while out of tree filesystems do exist they it's their
problem to keep up with kernel changes. They decide not to be part
of the Linux kernel, so it'll be their job to keep up with it.
Oh, right, we can simply remove REQ_HARDBARRIER completely.
quoted
Another is that pseudo block layer drivers (loop, virtio_blk,
md/dm...) have assumptions about REQ_HARDBARRIER behavior and things
would be broken in obscure ways between REQ_HARDBARRIER semantics
change and updates to each of those drivers, so I don't really think
changing the semantics while the mechanism is online is a good idea.
I don't think doing those changes in a separate commit is a good idea.
Do you want to change the whole thing in a single commit? That would
be a pretty big invasive patch touching multiple subsystems. Also, I
don't know what to do about drdb and would like to leave its
conversion to the maintainer (in separate patches).
Eh, well, this is mostly logistics. Jens, what do you think?
quoted
quoted
Then we can patches do disable the reiserfs barrier "optimization" as
the very first one, and DM/MD support which I'm currently working on
as the last one and we can start doing the heavy testing.
Oops, I've already converted loop, virtio_blk/lguest and am working on
md/dm right now too. I'm almost done with md and now doing dm. :-)
Maybe we should post them right now so that we don't waste too much
time trying to solve the same problems?
Here's the dm patch. It only handles normal bio based dm yet, which
I understand and can test. request based dm (multipath) still needs
work.
Here's the combined patch I've been working on. I've verified loop
and virtio_blk/loop. I just (like five mins ago) got dm/dm conversion
compiling, so I'm sure they're broken. The neat part is that thanks
to the separation between REQ_FLUSH and FUA handling, bio mangling
drivers only have to sequence the pre-flush and pass FUA directly to
lower layers which in many cases saves an array-wide cache flush
cycle.
After getting this patch working, the only remaining bits would be
blktrace and drdb.
Thanks.
Documentation/lguest/lguest.c | 36 +++-----
drivers/block/loop.c | 18 ++--
drivers/block/virtio_blk.c | 26 ++---
drivers/md/dm-io.c | 20 ----
drivers/md/dm-log.c | 2
drivers/md/dm-raid1.c | 8 -
drivers/md/dm-snap-persistent.c | 2
drivers/md/dm.c | 176 +++++++++++++++++++--------------------
drivers/md/linear.c | 4
drivers/md/md.c | 117 +++++---------------------
drivers/md/md.h | 23 +----
drivers/md/multipath.c | 4
drivers/md/raid0.c | 4
drivers/md/raid1.c | 178 +++++++++++++---------------------------
drivers/md/raid1.h | 2
drivers/md/raid10.c | 6 -
drivers/md/raid5.c | 18 +---
include/linux/virtio_blk.h | 6 +
18 files changed, 244 insertions(+), 406 deletions(-)
Index: block/drivers/block/loop.c
===================================================================
@@ -307,6 +306,7 @@ static int __devinit virtblk_probe(struc{structvirtio_blk*vblk;structrequest_queue*q;+unsignedintflush;interr;u64cap;u32v,blk_size,sg_elems,opt_io_size;
@@ -388,15 +388,13 @@ static int __devinit virtblk_probe(strucvblk->disk->driverfs_dev=&vdev->dev;index++;-/*-*IftheFLUSHfeatureissupportedwedohavesupportfor-*flushingavolatilewritecacheonthehost.Usethatto-*implementwritebarriersupport;otherwise,wemustassume-*thatthehostdoesnotperformanykindofvolatilewrite-*caching.-*/+/* configure queue flush support */+flush=0;if(virtio_has_feature(vdev,VIRTIO_BLK_F_FLUSH))-blk_queue_flush(q,REQ_FLUSH);+flush|=REQ_FLUSH;+if(virtio_has_feature(vdev,VIRTIO_BLK_F_FUA))+flush|=REQ_FUA;+blk_queue_flush(q,flush);/* If disk is read-only in the host, the guest should obey */if(virtio_has_feature(vdev,VIRTIO_BLK_F_RO))
@@ -1679,6 +1670,19 @@ static void blk_request(struct virtqueue/* Die, bad Guest, die. */errx(1,"Write past end %llu+%u",off,ret);}++/* Honor FUA by syncing everything. */+if(ret>=0&&(out->type&VIRTIO_BLK_T_FUA)){+ret=fdatasync(vblk->fd);+verbose("FUA fdatasync: %i\n",ret);+}++wlen=sizeof(*in);+*in=(ret>=0?VIRTIO_BLK_S_OK:VIRTIO_BLK_S_IOERR);+}elseif(out->type&VIRTIO_BLK_T_FLUSH){+/* Flush */+ret=fdatasync(vblk->fd);+verbose("FLUSH fdatasync: %i\n",ret);wlen=sizeof(*in);*in=(ret>=0?VIRTIO_BLK_S_OK:VIRTIO_BLK_S_IOERR);}else{
@@ -1702,15 +1706,6 @@ static void blk_request(struct virtqueue}}-/*-*OK,sowenotedthatitwasprettypoortouseanfdatasyncasa-*barrier.ButChristophHellwigpointsoutthatweneedasync-**afterwards*aswell:"Barriers specify no reordering to the front-*ortheback." And Jens Axboe confirmed it, so here we are:-*/-if(out->type&VIRTIO_BLK_T_BARRIER)-fdatasync(vblk->fd);-/* Finished that request. */add_used(vq,head,wlen);}
@@ -1735,8 +1730,9 @@ static void setup_block_file(const charvblk->fd=open_or_die(filename,O_RDWR|O_LARGEFILE);vblk->len=lseek64(vblk->fd,0,SEEK_END);-/* We support barriers. */-add_feature(dev,VIRTIO_BLK_F_BARRIER);+/* We support FLUSH and FUA. */+add_feature(dev,VIRTIO_BLK_F_FLUSH);+add_feature(dev,VIRTIO_BLK_F_FUA);/* Tell Guest how many sectors this device has. */conf.capacity=cpu_to_le64(vblk->len/512);
@@ -226,12 +226,12 @@ static int md_make_request(struct requesreturn0;}rcu_read_lock();-if(mddev->suspended||mddev->barrier){+if(mddev->suspended){DEFINE_WAIT(__wait);for(;;){prepare_to_wait(&mddev->sb_wait,&__wait,TASK_UNINTERRUPTIBLE);-if(!mddev->suspended&&!mddev->barrier)+if(!mddev->suspended)break;rcu_read_unlock();schedule();
@@ -280,40 +280,29 @@ static void mddev_resume(mddev_t *mddev)intmddev_congested(mddev_t*mddev,intbits){-if(mddev->barrier)-return1;returnmddev->suspended;}EXPORT_SYMBOL(mddev_congested);/*-*Genericbarrierhandlingformd+*Genericflushhandlingformd*/-#define POST_REQUEST_BARRIER ((void*)1)--staticvoidmd_end_barrier(structbio*bio,interr)+staticvoidmd_end_flush(structbio*bio,interr){mdk_rdev_t*rdev=bio->bi_private;mddev_t*mddev=rdev->mddev;-if(err==-EOPNOTSUPP&&mddev->barrier!=POST_REQUEST_BARRIER)-set_bit(BIO_EOPNOTSUPP,&mddev->barrier->bi_flags);rdev_dec_pending(rdev,mddev);if(atomic_dec_and_test(&mddev->flush_pending)){-if(mddev->barrier==POST_REQUEST_BARRIER){-/* This was a post-request barrier */-mddev->barrier=NULL;-wake_up(&mddev->sb_wait);-}else-/* The pre-request barrier has finished */-schedule_work(&mddev->barrier_work);+/* The pre-request flush has finished */+schedule_work(&mddev->flush_work);}bio_put(bio);}-staticvoidsubmit_barriers(mddev_t*mddev)+staticvoidsubmit_flushes(mddev_t*mddev){mdk_rdev_t*rdev;
@@ -330,60 +319,56 @@ static void submit_barriers(mddev_t *mddatomic_inc(&rdev->nr_pending);rcu_read_unlock();bi=bio_alloc(GFP_KERNEL,0);-bi->bi_end_io=md_end_barrier;+bi->bi_end_io=md_end_flush;bi->bi_private=rdev;bi->bi_bdev=rdev->bdev;atomic_inc(&mddev->flush_pending);-submit_bio(WRITE_BARRIER,bi);+submit_bio(WRITE_FLUSH,bi);rcu_read_lock();rdev_dec_pending(rdev,mddev);}rcu_read_unlock();}-staticvoidmd_submit_barrier(structwork_struct*ws)+staticvoidmd_submit_flush_data(structwork_struct*ws){-mddev_t*mddev=container_of(ws,mddev_t,barrier_work);-structbio*bio=mddev->barrier;+mddev_t*mddev=container_of(ws,mddev_t,flush_work);+structbio*bio=mddev->flush_bio;atomic_set(&mddev->flush_pending,1);-if(test_bit(BIO_EOPNOTSUPP,&bio->bi_flags))-bio_endio(bio,-EOPNOTSUPP);-elseif(bio->bi_size==0)+if(bio->bi_size==0)/* an empty barrier - all done */bio_endio(bio,0);else{-bio->bi_rw&=~REQ_HARDBARRIER;+bio->bi_rw&=~REQ_FLUSH;if(mddev->pers->make_request(mddev,bio))generic_make_request(bio);-mddev->barrier=POST_REQUEST_BARRIER;-submit_barriers(mddev);}if(atomic_dec_and_test(&mddev->flush_pending)){-mddev->barrier=NULL;+mddev->flush_bio=NULL;wake_up(&mddev->sb_wait);}}-voidmd_barrier_request(mddev_t*mddev,structbio*bio)+voidmd_flush_request(mddev_t*mddev,structbio*bio){spin_lock_irq(&mddev->write_lock);wait_event_lock_irq(mddev->sb_wait,-!mddev->barrier,+!mddev->flush_bio,mddev->write_lock,/*nothing*/);-mddev->barrier=bio;+mddev->flush_bio=bio;spin_unlock_irq(&mddev->write_lock);atomic_set(&mddev->flush_pending,1);-INIT_WORK(&mddev->barrier_work,md_submit_barrier);+INIT_WORK(&mddev->flush_work,md_submit_flush_data);-submit_barriers(mddev);+submit_flushes(mddev);if(atomic_dec_and_test(&mddev->flush_pending))-schedule_work(&mddev->barrier_work);+schedule_work(&mddev->flush_work);}-EXPORT_SYMBOL(md_barrier_request);+EXPORT_SYMBOL(md_flush_request);staticinlinemddev_t*mddev_get(mddev_t*mddev){
@@ -642,31 +627,6 @@ static void super_written(struct bio *bibio_put(bio);}-staticvoidsuper_written_barrier(structbio*bio,interror)-{-structbio*bio2=bio->bi_private;-mdk_rdev_t*rdev=bio2->bi_private;-mddev_t*mddev=rdev->mddev;--if(!test_bit(BIO_UPTODATE,&bio->bi_flags)&&-error==-EOPNOTSUPP){-unsignedlongflags;-/* barriers don't appear to be supported :-( */-set_bit(BarriersNotsupp,&rdev->flags);-mddev->barriers_work=0;-spin_lock_irqsave(&mddev->write_lock,flags);-bio2->bi_next=mddev->biolist;-mddev->biolist=bio2;-spin_unlock_irqrestore(&mddev->write_lock,flags);-wake_up(&mddev->sb_wait);-bio_put(bio);-}else{-bio_put(bio2);-bio->bi_private=rdev;-super_written(bio,error);-}-}-voidmd_super_write(mddev_t*mddev,mdk_rdev_t*rdev,sector_tsector,intsize,structpage*page){
@@ -675,51 +635,28 @@ void md_super_write(mddev_t *mddev, mdk_*anddecrementitoncompletion,wakingupsb_wait*ifzeroisreached.*Ifanerroroccurred,callmd_error-*-*AswemightneedtoresubmittherequestifREQ_HARDBARRIER-*causesENOTSUPP,weallocateasparebio...*/structbio*bio=bio_alloc(GFP_NOIO,1);-intrw=REQ_WRITE|REQ_SYNC|REQ_UNPLUG;bio->bi_bdev=rdev->bdev;bio->bi_sector=sector;bio_add_page(bio,page,size,0);bio->bi_private=rdev;bio->bi_end_io=super_written;-bio->bi_rw=rw;atomic_inc(&mddev->pending_writes);-if(!test_bit(BarriersNotsupp,&rdev->flags)){-structbio*rbio;-rw|=REQ_HARDBARRIER;-rbio=bio_clone(bio,GFP_NOIO);-rbio->bi_private=bio;-rbio->bi_end_io=super_written_barrier;-submit_bio(rw,rbio);-}else-submit_bio(rw,bio);+submit_bio(REQ_WRITE|REQ_SYNC|REQ_UNPLUG|REQ_FLUSH|REQ_FUA,+bio);}voidmd_super_wait(mddev_t*mddev){-/* wait for all superblock writes that were scheduled to complete.-*ifanyhadtoberetried(duetoBARRIERproblems),retrythem-*/+/* wait for all superblock writes that were scheduled to complete */DEFINE_WAIT(wq);for(;;){prepare_to_wait(&mddev->sb_wait,&wq,TASK_UNINTERRUPTIBLE);if(atomic_read(&mddev->pending_writes)==0)break;-while(mddev->biolist){-structbio*bio;-spin_lock_irq(&mddev->write_lock);-bio=mddev->biolist;-mddev->biolist=bio->bi_next;-bio->bi_next=NULL;-spin_unlock_irq(&mddev->write_lock);-submit_bio(bio->bi_rw,bio);-}schedule();}finish_wait(&mddev->sb_wait,&wq);
@@ -1016,7 +953,6 @@ static int super_90_validate(mddev_t *mdclear_bit(Faulty,&rdev->flags);clear_bit(In_sync,&rdev->flags);clear_bit(WriteMostly,&rdev->flags);-clear_bit(BarriersNotsupp,&rdev->flags);if(mddev->raid_disks==0){mddev->major_version=0;
@@ -1431,7 +1367,6 @@ static int super_1_validate(mddev_t *mddclear_bit(Faulty,&rdev->flags);clear_bit(In_sync,&rdev->flags);clear_bit(WriteMostly,&rdev->flags);-clear_bit(BarriersNotsupp,&rdev->flags);if(mddev->raid_disks==0){mddev->major_version=1;
@@ -4463,7 +4398,6 @@ static int md_run(mddev_t *mddev)/* may be over-ridden by personality */mddev->resync_max_sectors=mddev->dev_sectors;-mddev->barriers_work=1;mddev->ok_start_degraded=start_dirty_degraded;if(start_readonly&&mddev->ro==0)
@@ -67,7 +67,6 @@ struct mdk_rdev_s#define Faulty 1 /* device is known to have a fault */#define In_sync 2 /* device is in_sync with rest of array */#define WriteMostly 4 /* Avoid reading if at all possible */-#define BarriersNotsupp 5 /* REQ_HARDBARRIER is not supported */#define AllReserved 6 /* If whole device is reserved for*onearray*/#define AutoDetected 7 /* added by auto-detect */
@@ -249,13 +248,6 @@ struct mddev_sintdegraded;/* whether md should consider*addingaspare*/-intbarriers_work;/* initialised to true, cleared as soon-*asabarrierrequesttoslave-*fails.Onlysupported-*/-structbio*biolist;/* bios that need to be retried-*becauseREQ_HARDBARRIERisnotsupported-*/atomic_trecovery_active;/* blocks scheduled, but not written */wait_queue_head_trecovery_wait;
@@ -319,83 +319,74 @@ static void raid1_end_write_request(struif(r1_bio->bios[mirror]==bio)break;-if(error==-EOPNOTSUPP&&test_bit(R1BIO_Barrier,&r1_bio->state)){-set_bit(BarriersNotsupp,&conf->mirrors[mirror].rdev->flags);-set_bit(R1BIO_BarrierRetry,&r1_bio->state);-r1_bio->mddev->barriers_work=0;-/* Don't rdev_dec_pending in this branch - keep it for the retry */-}else{+/*+*'onemirrorIOhasfinished'eventhandler:+*/+r1_bio->bios[mirror]=NULL;+to_put=bio;+if(!uptodate){+md_error(r1_bio->mddev,conf->mirrors[mirror].rdev);+/* an I/O failed, we can't clear the bitmap */+set_bit(R1BIO_Degraded,&r1_bio->state);+}else/*-*thisbranchisour'onemirrorIOhasfinished'eventhandler:+*SetR1BIO_Uptodateinourmasterbio,sothatwe+*willreturnagooderrorcodefortothehigher+*levelsevenifIOonsomeothermirroredbuffer+*fails.+*+*The'master'representsthecompositeIOoperation+*touser-side.SoifsomethingwaitsforIO,thenit+*willwaitforthe'master'bio.*/-r1_bio->bios[mirror]=NULL;-to_put=bio;-if(!uptodate){-md_error(r1_bio->mddev,conf->mirrors[mirror].rdev);-/* an I/O failed, we can't clear the bitmap */-set_bit(R1BIO_Degraded,&r1_bio->state);-}else-/*-*SetR1BIO_Uptodateinourmasterbio,sothat-*wewillreturnagooderrorcodefortothehigher-*levelsevenifIOonsomeothermirroredbufferfails.-*-*The'master'representsthecompositeIOoperationto-*user-side.SoifsomethingwaitsforIO,thenitwill-*waitforthe'master'bio.-*/-set_bit(R1BIO_Uptodate,&r1_bio->state);+set_bit(R1BIO_Uptodate,&r1_bio->state);++update_head_pos(mirror,r1_bio);-update_head_pos(mirror,r1_bio);+if(behind){+if(test_bit(WriteMostly,&conf->mirrors[mirror].rdev->flags))+atomic_dec(&r1_bio->behind_remaining);-if(behind){-if(test_bit(WriteMostly,&conf->mirrors[mirror].rdev->flags))-atomic_dec(&r1_bio->behind_remaining);--/* In behind mode, we ACK the master bio once the I/O has safely-*reachedallnon-writemostlydisks.SettingtheReturnedbit-*ensuresthatthisgetsdoneonlyonce--wedon'teverwantto-*return-EIOhere,insteadwe'llwait*/--if(atomic_read(&r1_bio->behind_remaining)>=(atomic_read(&r1_bio->remaining)-1)&&-test_bit(R1BIO_Uptodate,&r1_bio->state)){-/* Maybe we can return now */-if(!test_and_set_bit(R1BIO_Returned,&r1_bio->state)){-structbio*mbio=r1_bio->master_bio;-PRINTK(KERN_DEBUG"raid1: behind end write sectors %llu-%llu\n",-(unsignedlonglong)mbio->bi_sector,-(unsignedlonglong)mbio->bi_sector+-(mbio->bi_size>>9)-1);-bio_endio(mbio,0);-}+/*+*Inbehindmode,weACKthemasterbiooncetheI/O+*hassafelyreachedallnon-writemostly+*disks.SettingtheReturnedbitensuresthatthis+*getsdoneonlyonce--wedon'teverwanttoreturn+*-EIOhere,insteadwe'llwait+*/+if(atomic_read(&r1_bio->behind_remaining)>=(atomic_read(&r1_bio->remaining)-1)&&+test_bit(R1BIO_Uptodate,&r1_bio->state)){+/* Maybe we can return now */+if(!test_and_set_bit(R1BIO_Returned,&r1_bio->state)){+structbio*mbio=r1_bio->master_bio;+PRINTK(KERN_DEBUG"raid1: behind end write sectors %llu-%llu\n",+(unsignedlonglong)mbio->bi_sector,+(unsignedlonglong)mbio->bi_sector++(mbio->bi_size>>9)-1);+bio_endio(mbio,0);}}-rdev_dec_pending(conf->mirrors[mirror].rdev,conf->mddev);}+rdev_dec_pending(conf->mirrors[mirror].rdev,conf->mddev);+/*-**Let'sseeifallmirroredwriteoperationshavefinished*already.*/if(atomic_dec_and_test(&r1_bio->remaining)){-if(test_bit(R1BIO_BarrierRetry,&r1_bio->state))-reschedule_retry(r1_bio);-else{-/* it really is the end of this request */-if(test_bit(R1BIO_BehindIO,&r1_bio->state)){-/* free extra copy of the data pages */-inti=bio->bi_vcnt;-while(i--)-safe_put_page(bio->bi_io_vec[i].bv_page);-}-/* clear the bitmap if all writes complete successfully */-bitmap_endwrite(r1_bio->mddev->bitmap,r1_bio->sector,-r1_bio->sectors,-!test_bit(R1BIO_Degraded,&r1_bio->state),-behind);-md_write_end(r1_bio->mddev);-raid_end_bio_io(r1_bio);-}+if(test_bit(R1BIO_BehindIO,&r1_bio->state)){+/* free extra copy of the data pages */+inti=bio->bi_vcnt;+while(i--)+safe_put_page(bio->bi_io_vec[i].bv_page);+}+/* clear the bitmap if all writes complete successfully */+bitmap_endwrite(r1_bio->mddev->bitmap,r1_bio->sector,+r1_bio->sectors,+!test_bit(R1BIO_Degraded,&r1_bio->state),+behind);+md_write_end(r1_bio->mddev);+raid_end_bio_io(r1_bio);}if(to_put)
@@ -787,17 +778,14 @@ static int make_request(mddev_t *mddev,structbio_listbl;structpage**behind_pages=NULL;constintrw=bio_data_dir(bio);-constbooldo_sync=(bio->bi_rw&REQ_SYNC);-booldo_barriers;+constunsignedintdo_sync=(bio->bi_rw&REQ_SYNC);+constunsignedintdo_flush_fua=(bio->bi_rw&(REQ_FLUSH|REQ_FUA));mdk_rdev_t*blocked_rdev;/**Registerthenewrequestandwaitifthereconstruction*threadhasputupabarfornewrequests.*Continueimmediatelyifnoresyncisactivecurrently.-*Wetestbarriers_work*after*md_write_startasmd_write_start-*maycausethefirstsuperblockwrite,andthatwillcheckout-*ifbarrierswork.*/md_write_start(mddev,bio);/* wait on superblock update early */
@@ -821,13 +809,6 @@ static int make_request(mddev_t *mddev,}finish_wait(&conf->wait_barrier,&w);}-if(unlikely(!mddev->barriers_work&&-(bio->bi_rw&REQ_HARDBARRIER))){-if(rw==WRITE)-md_write_end(mddev);-bio_endio(bio,-EOPNOTSUPP);-return0;-}wait_barrier(conf);
@@ -877,7 +858,7 @@ static int make_request(mddev_t *mddev,read_bio->bi_sector=r1_bio->sector+mirror->rdev->data_offset;read_bio->bi_bdev=mirror->rdev->bdev;read_bio->bi_end_io=raid1_end_read_request;-read_bio->bi_rw=READ|do_sync;+read_bio->bi_rw=READ|do_sync|do_flush_fua;read_bio->bi_private=r1_bio;generic_make_request(read_bio);
@@ -959,10 +940,6 @@ static int make_request(mddev_t *mddev,atomic_set(&r1_bio->remaining,0);atomic_set(&r1_bio->behind_remaining,0);-do_barriers=bio->bi_rw&REQ_HARDBARRIER;-if(do_barriers)-set_bit(R1BIO_Barrier,&r1_bio->state);-bio_list_init(&bl);for(i=0;i<disks;i++){structbio*mbio;
@@ -975,7 +952,7 @@ static int make_request(mddev_t *mddev,mbio->bi_sector=r1_bio->sector+conf->mirrors[i].rdev->data_offset;mbio->bi_bdev=conf->mirrors[i].rdev->bdev;mbio->bi_end_io=raid1_end_write_request;-mbio->bi_rw=WRITE|do_barriers|do_sync;+mbio->bi_rw=WRITE|do_sync;mbio->bi_private=r1_bio;if(behind_pages){
@@ -1631,41 +1608,6 @@ static void raid1d(mddev_t *mddev)if(test_bit(R1BIO_IsSync,&r1_bio->state)){sync_request_write(mddev,r1_bio);unplug=1;-}elseif(test_bit(R1BIO_BarrierRetry,&r1_bio->state)){-/* some requests in the r1bio were REQ_HARDBARRIER-*requestswhichfailedwith-EOPNOTSUPP.Hohumm..-*Betterresubmitwithoutthebarrier.-*Weknowwhichdevicestoresubmitfor,because-*allothershavehadtheirbios[]entrycleared.-*Wealreadyhaveanr_pendingreferenceontheserdevs.-*/-inti;-constbooldo_sync=(r1_bio->master_bio->bi_rw&REQ_SYNC);-clear_bit(R1BIO_BarrierRetry,&r1_bio->state);-clear_bit(R1BIO_Barrier,&r1_bio->state);-for(i=0;i<conf->raid_disks;i++)-if(r1_bio->bios[i])-atomic_inc(&r1_bio->remaining);-for(i=0;i<conf->raid_disks;i++)-if(r1_bio->bios[i]){-structbio_vec*bvec;-intj;--bio=bio_clone(r1_bio->master_bio,GFP_NOIO);-/* copy pages from the failed bio, as-*thismightbeawrite-behinddevice*/-__bio_for_each_segment(bvec,bio,j,0)-bvec->bv_page=bio_iovec_idx(r1_bio->bios[i],j)->bv_page;-bio_put(r1_bio->bios[i]);-bio->bi_sector=r1_bio->sector+-conf->mirrors[i].rdev->data_offset;-bio->bi_bdev=conf->mirrors[i].rdev->bdev;-bio->bi_end_io=raid1_end_write_request;-bio->bi_rw=WRITE|do_sync;-bio->bi_private=r1_bio;-r1_bio->bios[i]=bio;-generic_make_request(bio);-}}else{intdisk;
@@ -3278,7 +3278,7 @@ static void handle_stripe5(struct stripeif(dec_preread_active){/* We delay this until after ops_run_io so that if make_request-*iswaitingonabarrier,itwon'tcontinueuntilthewrites+*iswaitingonaflush,itwon'tcontinueuntilthewrites*haveactuallybeensubmitted.*/atomic_dec(&conf->preread_active_stripes);
@@ -3580,7 +3580,7 @@ static void handle_stripe6(struct stripeif(dec_preread_active){/* We delay this until after ops_run_io so that if make_request-*iswaitingonabarrier,itwon'tcontinueuntilthewrites+*iswaitingonaflush,itwon'tcontinueuntilthewrites*haveactuallybeensubmitted.*/atomic_dec(&conf->preread_active_stripes);
@@ -3958,14 +3958,8 @@ static int make_request(mddev_t *mddev,constintrw=bio_data_dir(bi);intremaining;-if(unlikely(bi->bi_rw&REQ_HARDBARRIER)){-/* Drain all pending writes. We only really need-*toensuretheyhavebeensubmitted,butthisis-*easier.-*/-mddev->pers->quiesce(mddev,1);-mddev->pers->quiesce(mddev,0);-md_barrier_request(mddev,bi);+if(unlikely(bi->bi_rw&REQ_FLUSH)){+md_flush_request(mddev,bi);return0;}
@@ -4083,7 +4077,7 @@ static int make_request(mddev_t *mddev,finish_wait(&conf->wait_for_overlap,&w);set_bit(STRIPE_HANDLE,&sh->state);clear_bit(STRIPE_DELAYED,&sh->state);-if(mddev->barrier&&+if(mddev->flush_bio&&!test_and_set_bit(STRIPE_PREREAD_ACTIVE,&sh->state))atomic_inc(&conf->preread_active_stripes);release_stripe(sh);
@@ -4106,7 +4100,7 @@ static int make_request(mddev_t *mddev,bio_endio(bi,0);}-if(mddev->barrier){+if(mddev->flush_bio){/* We need to wait for the stripes to all be handled.*So:waitforpreread_active_stripestodropto0.*/
@@ -139,21 +139,21 @@ struct mapped_device {spinlock_tdeferred_lock;/*-*Anerrorfromthebarrierrequestcurrentlybeingprocessed.+*Anerrorfromtheflushrequestcurrentlybeingprocessed.*/-intbarrier_error;+intflush_error;/*-*Protectbarrier_errorfromconcurrentendioprocessing+*Protectflush_errorfromconcurrentendioprocessing*inrequest-baseddm.*/-spinlock_tbarrier_error_lock;+spinlock_tflush_error_lock;/*-*Processingqueue(flush/barriers)+*Processingqueue(flush)*/structworkqueue_struct*wq;-structwork_structbarrier_work;+structwork_structflush_work;/* A pointer to the currently processing pre/post flush request */structrequest*flush_request;
@@ -195,8 +195,8 @@ struct mapped_device {/* sysfs handle */structkobjectkobj;-/* zero-length barrier that will be cloned and submitted to targets */-structbiobarrier_bio;+/* zero-length flush that will be cloned and submitted to targets */+structbioflush_bio;};/*
@@ -1325,14 +1319,22 @@ static void __split_and_process_bio(struci.io->md=md;spin_lock_init(&ci.io->endio_lock);ci.sector=bio->bi_sector;-ci.sector_count=bio_sectors(bio);-if(unlikely(bio_empty_barrier(bio)))+if(!(bio->bi_rw&REQ_FLUSH))+ci.sector_count=bio_sectors(bio);+else{+/* FLUSH bio reaching here should all be empty */+WARN_ON_ONCE(bio_has_data(bio));ci.sector_count=1;+}ci.idx=bio->bi_idx;start_io_acct(ci.io);-while(ci.sector_count&&!error)-error=__clone_and_map(&ci);+while(ci.sector_count&&!error){+if(!(bio->bi_rw&REQ_FLUSH))+error=__clone_and_map(&ci);+else+error=__clone_and_map_flush(&ci);+}/* drop the extra reference count */dec_pending(ci.io,error);
@@ -1417,11 +1419,11 @@ static int _dm_request(struct request_qupart_stat_unlock();/*-*Ifwe'resuspendedorthethreadisprocessingbarriers+*Ifwe'resuspendedorthethreadisprocessingflushes*wehavetoqueuethisioforlater.*/if(unlikely(test_bit(DMF_QUEUE_IO_TO_THREAD,&md->flags))||-unlikely(bio->bi_rw&REQ_HARDBARRIER)){+(bio->bi_rw&REQ_FLUSH)){up_read(&md->io_lock);if(unlikely(test_bit(DMF_BLOCK_IO_FOR_SUSPEND,&md->flags))&&
@@ -1464,10 +1466,7 @@ static int dm_request(struct request_questaticbooldm_rq_is_flush_request(structrequest*rq){-if(rq->cmd_flags&REQ_FLUSH)-returntrue;-else-returnfalse;+returnrq->cmd_flags&REQ_FLUSH;}voiddm_dispatch_request(structrequest*rq)
@@ -1520,7 +1519,7 @@ static int setup_clone(struct request *cif(dm_rq_is_flush_request(rq)){blk_rq_init(NULL,clone);clone->cmd_type=REQ_TYPE_FS;-clone->cmd_flags|=(REQ_HARDBARRIER|WRITE);+clone->cmd_flags|=(REQ_FLUSH|WRITE);}else{r=blk_rq_prep_clone(clone,rq,tio->md->bs,GFP_ATOMIC,dm_rq_bio_constructor,tio);
@@ -2317,8 +2313,8 @@ static void dm_rq_set_flush_nr(struct retio->info.flush_request=flush_nr;}-/* Issue barrier requests to targets and wait for their completion. */-staticintdm_rq_barrier(structmapped_device*md)+/* Issue flush requests to targets and wait for their completion. */+staticintdm_rq_flush(structmapped_device*md){inti,j;structdm_table*map=dm_get_live_table(md);
@@ -2326,7 +2322,7 @@ static int dm_rq_barrier(struct mapped_dstructdm_target*ti;structrequest*clone;-md->barrier_error=0;+md->flush_error=0;for(i=0;i<num_targets;i++){ti=dm_table_get_target(map,i);
@@ -2341,26 +2337,26 @@ static int dm_rq_barrier(struct mapped_ddm_wait_for_completion(md,TASK_UNINTERRUPTIBLE);dm_table_put(map);-returnmd->barrier_error;+returnmd->flush_error;}-staticvoiddm_rq_barrier_work(structwork_struct*work)+staticvoiddm_rq_flush_work(structwork_struct*work){interror;structmapped_device*md=container_of(work,structmapped_device,-barrier_work);+flush_work);structrequest_queue*q=md->queue;structrequest*rq;unsignedlongflags;/**Holdthemdreferencehereandleaveitatthelastpartsothat-*themdcan'tbedeletedbydeviceopenerwhenthebarrierrequest+*themdcan'tbedeletedbydeviceopenerwhentheflushrequest*completes.*/dm_get(md);-error=dm_rq_barrier(md);+error=dm_rq_flush(md);rq=md->flush_request;md->flush_request=NULL;
@@ -2520,7 +2516,7 @@ int dm_suspend(struct mapped_device *md,up_write(&md->io_lock);/*-*Request-baseddmusesmd->wqforbarrier(dm_rq_barrier_work)which+*Request-baseddmusesmd->wqforflush(dm_rq_flush_work)which*canbekickeduntilmd->queueisstopped.Sostopmd->queuebefore*flushingmd->wq.*/
From: Jeremy Fitzhardinge <hidden> Date: 2010-08-14 01:07:13
On 08/12/2010 05:41 AM, Tejun Heo wrote:
Barrier is deemed too heavy and will soon be replaced by FLUSH/FUA
requests. Deprecate barrier. All REQ_HARDBARRIERs are failed with
-EOPNOTSUPP and blk_queue_ordered() is replaced with simpler
blk_queue_flush().
blk_queue_flush() takes combinations of REQ_FLUSH and FUA. If a
device has write cache and can flush it, it should set REQ_FLUSH. If
the device can handle FUA writes, it should also set REQ_FUA.
Christoph, do these two patches (parts 2 and 3) make xen-blkfront
correct WRT barriers/flushing as far as your concerned?
Thanks,
J
quoted hunk
All blk_queue_ordered() users are converted.
* ORDERED_DRAIN is mapped to 0 which is the default value.
* ORDERED_DRAIN_FLUSH is mapped to REQ_FLUSH.
* ORDERED_DRAIN_FLUSH_FUA is mapped to REQ_FLUSH | REQ_FUA.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Nick Piggin <redacted>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jeremy Fitzhardinge <redacted>
Cc: Chris Wright <redacted>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Boaz Harrosh <redacted>
Cc: Geert Uytterhoeven <redacted>
Cc: David S. Miller <davem@davemloft.net>
Cc: Alasdair G Kergon <agk@redhat.com>
Cc: Pierre Ossman <redacted>
Cc: Stefan Weinhuber <redacted>
---
block/blk-barrier.c | 29 ----------------------------
block/blk-core.c | 6 +++-
block/blk-settings.c | 20 +++++++++++++++++++
drivers/block/brd.c | 1 -
drivers/block/loop.c | 2 +-
drivers/block/osdblk.c | 2 +-
drivers/block/ps3disk.c | 2 +-
drivers/block/virtio_blk.c | 25 ++++++++---------------
drivers/block/xen-blkfront.c | 43 +++++++++++------------------------------
drivers/ide/ide-disk.c | 13 +++++------
drivers/md/dm.c | 2 +-
drivers/mmc/card/queue.c | 1 -
drivers/s390/block/dasd.c | 1 -
drivers/scsi/sd.c | 16 +++++++-------
include/linux/blkdev.h | 6 +++-
15 files changed, 67 insertions(+), 102 deletions(-)
@@ -9,35 +9,6 @@#include"blk.h"-/**-*blk_queue_ordered-doesthisqueuesupportorderedwrites-*@q:therequestqueue-*@ordered:oneofQUEUE_ORDERED_*-*-*Description:-*Forjournalledfilesystems,doingorderedwritesonacommit-*blockinsteadofexplicitlydoingwait_on_buffer(whichisbad-*forperformance)canbeabigwin.Blockdriverssupportingthis-*featureshouldcallthisfunctionandindicateso.-*-**/-intblk_queue_ordered(structrequest_queue*q,unsignedordered)-{-if(ordered!=QUEUE_ORDERED_NONE&&-ordered!=QUEUE_ORDERED_DRAIN&&-ordered!=QUEUE_ORDERED_DRAIN_FLUSH&&-ordered!=QUEUE_ORDERED_DRAIN_FUA){-printk(KERN_ERR"blk_queue_ordered: bad value %d\n",ordered);-return-EINVAL;-}--q->ordered=ordered;-q->next_ordered=ordered;--return0;-}-EXPORT_SYMBOL(blk_queue_ordered);-/**Cacheflushingfororderedwriteshandling*/
@@ -1203,11 +1203,13 @@ static int __make_request(struct request_queue *q, struct bio *bio)constunsignedintff=bio->bi_rw&REQ_FAILFAST_MASK;intrw_flags;-if((bio->bi_rw&REQ_HARDBARRIER)&&-(q->next_ordered==QUEUE_ORDERED_NONE)){+/* REQ_HARDBARRIER is no more */+if(WARN_ONCE(bio->bi_rw&REQ_HARDBARRIER,+"block: HARDBARRIER is deprecated, use FLUSH/FUA instead\n")){bio_endio(bio,-EOPNOTSUPP);return0;}+/**lowleveldrivercanindicatethatitwantspagesabovea*certainlimitbouncedtolowmemory(ieforhighmem,oreven
@@ -388,22 +388,15 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)vblk->disk->driverfs_dev=&vdev->dev;index++;-if(virtio_has_feature(vdev,VIRTIO_BLK_F_FLUSH)){-/*-*IftheFLUSHfeatureissupportedwedohavesupportfor-*flushingavolatilewritecacheonthehost.Usethat-*toimplementwritebarriersupport.-*/-blk_queue_ordered(q,QUEUE_ORDERED_DRAIN_FLUSH);-}else{-/*-*IftheFLUSHfeatureisnotsupportedwemustassumethat-*thehostdoesnotperformanykindofvolatilewrite-*caching.Westillneedtodrainthequeuetoprovider-*properbarriersemantics.-*/-blk_queue_ordered(q,QUEUE_ORDERED_DRAIN);-}+/*+*IftheFLUSHfeatureissupportedwedohavesupportfor+*flushingavolatilewritecacheonthehost.Usethatto+*implementwritebarriersupport;otherwise,wemustassume+*thatthehostdoesnotperformanykindofvolatilewrite+*caching.+*/+if(virtio_has_feature(vdev,VIRTIO_BLK_F_FLUSH))+blk_queue_flush(q,REQ_FLUSH);/* If disk is read-only in the host, the guest should obey */if(virtio_has_feature(vdev,VIRTIO_BLK_F_RO))
On Fri, Aug 13, 2010 at 06:07:13PM -0700, Jeremy Fitzhardinge wrote:
On 08/12/2010 05:41 AM, Tejun Heo wrote:
quoted
Barrier is deemed too heavy and will soon be replaced by FLUSH/FUA
requests. Deprecate barrier. All REQ_HARDBARRIERs are failed with
-EOPNOTSUPP and blk_queue_ordered() is replaced with simpler
blk_queue_flush().
blk_queue_flush() takes combinations of REQ_FLUSH and FUA. If a
device has write cache and can flush it, it should set REQ_FLUSH. If
the device can handle FUA writes, it should also set REQ_FUA.
Christoph, do these two patches (parts 2 and 3) make xen-blkfront
correct WRT barriers/flushing as far as your concerned?
If all your backends handle a zero-length BLKIF_OP_WRITE_BARRIER request
it is a fully correct, but rather suboptimal implementation. To get
all the benefit of the new non-draining barriers you'll need a new
If all your backends handle a zero-length BLKIF_OP_FLUSH request that
only flushes the cache, but has no ordering side effects. Note that
the quite suboptimal here means not as good as the new barrier
implementation, but it shouldn't be notiably worse than the old one
for Xen.
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-14 10:36:54
On Fri, Aug 13, 2010 at 04:51:17PM +0200, Tejun Heo wrote:
Do you want to change the whole thing in a single commit? That would
be a pretty big invasive patch touching multiple subsystems.
We can just stop draining in the block layer in the first patch, then
stop doing the stuff in md/dm/etc in the following and then do the
final renaming patches. It would still be less patches then now, but
keep things working through the whole transition, which would really
help biseting any problems.
+ if (req->cmd_flags & REQ_FUA)
+ vbr->out_hdr.type |= VIRTIO_BLK_T_FUA;
I'd suggest not adding FUA support to virtio yet. Just using the flush
feature gives you a fully working barrier implementation.
Eventually we might want to add a flag in the block queue to send
REQ_FLUSH|REQ_FUA request through to virtio directly so that we can
avoid separate pre- and post flushes, but I really want to benchmark if
it makes an impact on real life setups first.
@@ -294,8 +294,8 @@ static int linear_make_request (mddev_tdev_info_t*tmp_dev;sector_tstart_sector;-if(unlikely(bio->bi_rw&REQ_HARDBARRIER)){-md_barrier_request(mddev,bio);+if(unlikely(bio->bi_rw&REQ_FLUSH)){+md_flush_request(mddev,bio);
We only need the special md_flush_request handling for
empty REQ_FLUSH requests. REQ_WRITE | REQ_FLUSH just need the
flag propagated to the underlying devices.
+static void md_end_flush(struct bio *bio, int err)
{
mdk_rdev_t *rdev = bio->bi_private;
mddev_t *mddev = rdev->mddev;
rdev_dec_pending(rdev, mddev);
if (atomic_dec_and_test(&mddev->flush_pending)) {
+ /* The pre-request flush has finished */
+ schedule_work(&mddev->flush_work);
Once we only handle empty barriers here we can directly call bio_endio
instead of first scheduling a work queue.Once we only handle empty
barriers here we can directly call bio_endio and the super wakeup
instead of first scheduling a work queue.
while ((bio = bio_list_pop(writes))) {
- if (unlikely(bio_empty_barrier(bio))) {
+ if ((bio->bi_rw & REQ_FLUSH) && !bio_has_data(bio)) {
I kept bio_empty_barrier as bio_empty_flush, which actually is a quite
useful macro for the bio based drivers.
quoted hunk
@@ -621,7 +621,7 @@ static void dec_pending(struct dm_io *io */ spin_lock_irqsave(&md->deferred_lock, flags); if (__noflush_suspending(md)) {- if (!(io->bio->bi_rw & REQ_HARDBARRIER))+ if (!(io->bio->bi_rw & REQ_FLUSH))
I suspect we don't actually need to special case flushes here anymore.
quoted hunk
@@ -633,14 +633,14 @@ static void dec_pending(struct dm_io *io io_error = io->error; bio = io->bio;- if (bio->bi_rw & REQ_HARDBARRIER) {+ if (bio->bi_rw & REQ_FLUSH) { /*- * There can be just one barrier request so we use+ * There can be just one flush request so we use * a per-device variable for error reporting. * Note that you can't touch the bio after end_io_acct */- if (!md->barrier_error && io_error != -EOPNOTSUPP)- md->barrier_error = io_error;+ if (!md->flush_error)+ md->flush_error = io_error;
And we certainly do not need any special casing here. See my patch.
@@ -1308,11 +1302,11 @@ static void __split_and_process_bio(stru ci.map = dm_get_live_table(md); if (unlikely(!ci.map)) {- if (!(bio->bi_rw & REQ_HARDBARRIER))+ if (!(bio->bi_rw & REQ_FLUSH)) bio_io_error(bio); else- if (!md->barrier_error)- md->barrier_error = -EIO;+ if (!md->flush_error)+ md->flush_error = -EIO;
No need for the special error handling here, flush requests can now
be completed normally.
quoted hunk
@@ -1417,11 +1419,11 @@ static int _dm_request(struct request_qu part_stat_unlock(); /*- * If we're suspended or the thread is processing barriers+ * If we're suspended or the thread is processing flushes * we have to queue this io for later. */ if (unlikely(test_bit(DMF_QUEUE_IO_TO_THREAD, &md->flags)) ||- unlikely(bio->bi_rw & REQ_HARDBARRIER)) {+ (bio->bi_rw & REQ_FLUSH)) { up_read(&md->io_lock);
AFAICS this is only needed for the old barrier code, no need for this
for pure flushes.
My suspicion is that we can get rif of all that special casing here
and just use blk_rq_prep_clone once it's been updated to propagate
REQ_FLUSH, similar to the DISCARD flag.
I also suspect that there is absolutely no need to the barrier work
queue once we stop waiting for outstanding request. But then again
the request based dm code still somewhat confuses me.
There's not need to use a separate flush_bio here.
__split_and_process_bio does the right thing for empty REQ_FLUSH
requests. See my patch for how to do this differenty. And yeah,
my version has been tested.
Propagate deprecation of REQ_HARDBARRIER and new REQ_FLUSH/FUA
interface to upper layers.
* WRITE_BARRIER is marked deprecated and WRITE_FLUSH, WRITE_FUA and
WRITE_FLUSH_FUA are added.
* REQ_COMMON_MASK now includes REQ_FLUSH | REQ_FUA so that they are
copied from bio to request.
* BH_Ordered and BH_Eopnotsupp are marked deprecated. BH_Flush/FUA
are _NOT_ added as they can and should be specified when calling
submit_bh() as @rw parameter as suggested by Jan Kara.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Jan Kara <jack@suse.cz>
---
Dropped BH_Flush/FUA as suggested.
Thanks.
include/linux/blk_types.h | 2 +-
include/linux/buffer_head.h | 4 ++--
include/linux/fs.h | 20 +++++++++++++-------
3 files changed, 16 insertions(+), 10 deletions(-)
Index: block/include/linux/fs.h
===================================================================
@@ -32,8 +32,8 @@ enum bh_state_bits {BH_Delay,/* Buffer is not yet allocated on disk */BH_Boundary,/* Block is followed by a discontiguity */BH_Write_EIO,/* I/O error on write */-BH_Ordered,/* ordered write */-BH_Eopnotsupp,/* operation not supported (barrier) */+BH_Ordered,/* DEPRECATED: ordered write */+BH_Eopnotsupp,/* DEPRECATED: operation not supported (barrier) */BH_Unwritten,/* Buffer is allocated on disk but not written */BH_Quiet,/* Buffer Error Prinks to be quiet */
From: Jeremy Fitzhardinge <hidden> Date: 2010-08-16 20:38:45
On 08/14/2010 02:42 AM, hch@lst.de wrote:
On Fri, Aug 13, 2010 at 06:07:13PM -0700, Jeremy Fitzhardinge wrote:
quoted
On 08/12/2010 05:41 AM, Tejun Heo wrote:
quoted
Barrier is deemed too heavy and will soon be replaced by FLUSH/FUA
requests. Deprecate barrier. All REQ_HARDBARRIERs are failed with
-EOPNOTSUPP and blk_queue_ordered() is replaced with simpler
blk_queue_flush().
blk_queue_flush() takes combinations of REQ_FLUSH and FUA. If a
device has write cache and can flush it, it should set REQ_FLUSH. If
the device can handle FUA writes, it should also set REQ_FUA.
Christoph, do these two patches (parts 2 and 3) make xen-blkfront
correct WRT barriers/flushing as far as your concerned?
If all your backends handle a zero-length BLKIF_OP_WRITE_BARRIER request
it is a fully correct, but rather suboptimal implementation. To get
all the benefit of the new non-draining barriers you'll need a new
If all your backends handle a zero-length BLKIF_OP_FLUSH request that
only flushes the cache, but has no ordering side effects.
Is the effect of the flush that, once complete, any previously completed
write is guaranteed to be on durable storage, but it is not guaranteed
to have any effect on pending writes? If so, does it flush writes that
were completed before the flush is issued, or writes that complete
before the flush completes?
Note that
the quite suboptimal here means not as good as the new barrier
implementation, but it shouldn't be notiably worse than the old one
for Xen.
OK, thanks. We can do some testing on that and see if there's a benefit
to adding a flush operation with the appropriate semantics.
J
Hello, Christoph.
On 08/14/2010 12:36 PM, Christoph Hellwig wrote:
On Fri, Aug 13, 2010 at 04:51:17PM +0200, Tejun Heo wrote:
quoted
Do you want to change the whole thing in a single commit? That would
be a pretty big invasive patch touching multiple subsystems.
We can just stop draining in the block layer in the first patch, then
stop doing the stuff in md/dm/etc in the following and then do the
final renaming patches. It would still be less patches then now, but
keep things working through the whole transition, which would really
help biseting any problems.
I'm not really convinced that would help much. If bisecting can point
to the conversion as the culprit for whatever kind of failure,
wouldn't that be enough? No matter what we do the conversion will be
a single step thing. If we make the filesystems enforce the ordering
first and then relax ordering in the block layer, bisection would
still just point at the later patch. The same goes for md/dm, the
best we can find out would be whether the conversion is correct or not
anyway.
I'm not against restructuring the patchset if it makes more sense but
it just feels like it would be a bit pointless effort (and one which
would require much tighter coordination among different trees) at this
point. Am I missing something?
quoted
+ if (req->cmd_flags & REQ_FUA)
+ vbr->out_hdr.type |= VIRTIO_BLK_T_FUA;
I'd suggest not adding FUA support to virtio yet. Just using the flush
feature gives you a fully working barrier implementation.
Eventually we might want to add a flag in the block queue to send
REQ_FLUSH|REQ_FUA request through to virtio directly so that we can
avoid separate pre- and post flushes, but I really want to benchmark if
it makes an impact on real life setups first.
I wrote this in the other mail but I think it would make difference if
the backend storag is md/dm especially if it's shared by multiple VMs.
It cuts down on one array wide cache flush.
@@ -294,8 +294,8 @@ static int linear_make_request (mddev_tdev_info_t*tmp_dev;sector_tstart_sector;-if(unlikely(bio->bi_rw&REQ_HARDBARRIER)){-md_barrier_request(mddev,bio);+if(unlikely(bio->bi_rw&REQ_FLUSH)){+md_flush_request(mddev,bio);
We only need the special md_flush_request handling for
empty REQ_FLUSH requests. REQ_WRITE | REQ_FLUSH just need the
flag propagated to the underlying devices.
Hmm, not really, the WRITE should happen after all the data in cache
are committed to NV media, meaning that empty FLUSH should already
have finished by the time the WRITE starts.
quoted
+static void md_end_flush(struct bio *bio, int err)
{
mdk_rdev_t *rdev = bio->bi_private;
mddev_t *mddev = rdev->mddev;
rdev_dec_pending(rdev, mddev);
if (atomic_dec_and_test(&mddev->flush_pending)) {
+ /* The pre-request flush has finished */
+ schedule_work(&mddev->flush_work);
Once we only handle empty barriers here we can directly call bio_endio
instead of first scheduling a work queue.Once we only handle empty
barriers here we can directly call bio_endio and the super wakeup
instead of first scheduling a work queue.
Yeap, right. That would be a nice optimization.
quoted
while ((bio = bio_list_pop(writes))) {
- if (unlikely(bio_empty_barrier(bio))) {
+ if ((bio->bi_rw & REQ_FLUSH) && !bio_has_data(bio)) {
I kept bio_empty_barrier as bio_empty_flush, which actually is a quite
useful macro for the bio based drivers.
Hmm... maybe. The reason why I removed bio_empty_flush() was that
except for the front-most sequencer (block layer for all the request
based ones and the front-most make_request for bio based ones), it
doesn't make sense to see REQ_FLUSH + data bios. They should be
sequenced at the front-most stage anyway, so I didn't have much use
for them. Those code paths couldn't deal with REQ_FLUSH + data bios
anyway.
quoted
@@ -621,7 +621,7 @@ static void dec_pending(struct dm_io *io */ spin_lock_irqsave(&md->deferred_lock, flags); if (__noflush_suspending(md)) {- if (!(io->bio->bi_rw & REQ_HARDBARRIER))+ if (!(io->bio->bi_rw & REQ_FLUSH))
I suspect we don't actually need to special case flushes here anymore.
Oh, I'm not sure about this part at all. I'll ask Mike.
quoted
@@ -633,14 +633,14 @@ static void dec_pending(struct dm_io *io io_error = io->error; bio = io->bio;- if (bio->bi_rw & REQ_HARDBARRIER) {+ if (bio->bi_rw & REQ_FLUSH) { /*- * There can be just one barrier request so we use+ * There can be just one flush request so we use * a per-device variable for error reporting. * Note that you can't touch the bio after end_io_acct */- if (!md->barrier_error && io_error != -EOPNOTSUPP)- md->barrier_error = io_error;+ if (!md->flush_error)+ md->flush_error = io_error;
And we certainly do not need any special casing here. See my patch.
I wasn't sure about that part. You removed store_flush_error(), but
DM_ENDIO_REQUEUE should still have higher priority than other
failures, no?
The same question as before. I think we still need to prioritize
DM_ENDIO_REQUEUE failures.
quoted
@@ -1417,11 +1419,11 @@ static int _dm_request(struct request_qu part_stat_unlock(); /*- * If we're suspended or the thread is processing barriers+ * If we're suspended or the thread is processing flushes * we have to queue this io for later. */ if (unlikely(test_bit(DMF_QUEUE_IO_TO_THREAD, &md->flags)) ||- unlikely(bio->bi_rw & REQ_HARDBARRIER)) {+ (bio->bi_rw & REQ_FLUSH)) { up_read(&md->io_lock);
AFAICS this is only needed for the old barrier code, no need for this
for pure flushes.
There's not need to use a separate flush_bio here.
__split_and_process_bio does the right thing for empty REQ_FLUSH
requests. See my patch for how to do this differenty. And yeah,
my version has been tested.
But how do you make sure REQ_FLUSHes for preflush finish before
starting the write?
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-17 13:19:15
On Tue, Aug 17, 2010 at 11:59:38AM +0200, Tejun Heo wrote:
I'm not really convinced that would help much. If bisecting can point
to the conversion as the culprit for whatever kind of failure,
wouldn't that be enough? No matter what we do the conversion will be
a single step thing. If we make the filesystems enforce the ordering
first and then relax ordering in the block layer, bisection would
still just point at the later patch. The same goes for md/dm, the
best we can find out would be whether the conversion is correct or not
anyway.
The filesystems already enforce the ordering, except reiserfs which
opts out if the barrier options is set.
I'm not against restructuring the patchset if it makes more sense but
it just feels like it would be a bit pointless effort (and one which
would require much tighter coordination among different trees) at this
point. Am I missing something?
What other trees do you mean? The conversions of the 8 filesystems
that actually support barriers need to go through this tree anyway
if we want to be able to test it. Also the changes in the filesystem
are absolutely minimal - it's basically just
s/WRITE_BARRIER/WRITE_FUA_FLUSH/ after my initial patch kill BH_Orderd,
and removing about 10 lines of code in reiserfs.
quoted
We only need the special md_flush_request handling for
empty REQ_FLUSH requests. REQ_WRITE | REQ_FLUSH just need the
flag propagated to the underlying devices.
Hmm, not really, the WRITE should happen after all the data in cache
are committed to NV media, meaning that empty FLUSH should already
have finished by the time the WRITE starts.
You're right.
quoted
quoted
while ((bio = bio_list_pop(writes))) {
- if (unlikely(bio_empty_barrier(bio))) {
+ if ((bio->bi_rw & REQ_FLUSH) && !bio_has_data(bio)) {
I kept bio_empty_barrier as bio_empty_flush, which actually is a quite
useful macro for the bio based drivers.
Hmm... maybe. The reason why I removed bio_empty_flush() was that
except for the front-most sequencer (block layer for all the request
based ones and the front-most make_request for bio based ones), it
doesn't make sense to see REQ_FLUSH + data bios. They should be
sequenced at the front-most stage anyway, so I didn't have much use
for them. Those code paths couldn't deal with REQ_FLUSH + data bios
anyway.
The current bio_empty_barrier is only used in dm, and indeed only makes
sense for make_request-based drivers. But I think it's a rather useful
helper for them. Either way, it's not a big issue and either way is
fine with me.
quoted
quoted
+ if (bio->bi_rw & REQ_FLUSH) {
/*
- * There can be just one barrier request so we use
+ * There can be just one flush request so we use
* a per-device variable for error reporting.
* Note that you can't touch the bio after end_io_acct
*/
- if (!md->barrier_error && io_error != -EOPNOTSUPP)
- md->barrier_error = io_error;
+ if (!md->flush_error)
+ md->flush_error = io_error;
And we certainly do not need any special casing here. See my patch.
I wasn't sure about that part. You removed store_flush_error(), but
DM_ENDIO_REQUEUE should still have higher priority than other
failures, no?
There's not need to use a separate flush_bio here.
__split_and_process_bio does the right thing for empty REQ_FLUSH
requests. See my patch for how to do this differenty. And yeah,
my version has been tested.
But how do you make sure REQ_FLUSHes for preflush finish before
starting the write?
Hmm, okay. I see how the special flush_bio makes the waiting easier,
let's see if Mike or other in the DM team have a better idea.
Btw, I think this one should just go away. It's only used by
ide in an attempt to make ordered sequences atomic, which isn't
needed for the new design.
Btw, I think this one should just go away. It's only used by
ide in an attempt to make ordered sequences atomic, which isn't
needed for the new design.
Yeap, agreed. I couldn't really understand why the the sequence
needed to be atomic for ide in the first place so just left it alone.
Do you understand why it tried to be atomic?
Thanks.
--
tejun
Hi,
On 08/17/2010 03:19 PM, Christoph Hellwig wrote:
On Tue, Aug 17, 2010 at 11:59:38AM +0200, Tejun Heo wrote:
quoted
I'm not against restructuring the patchset if it makes more sense but
it just feels like it would be a bit pointless effort (and one which
would require much tighter coordination among different trees) at this
point. Am I missing something?
What other trees do you mean?
I was mostly thinking about dm/md, drdb and stuff, but you're talking
about filesystem conversion patches being routed through block tree,
right?
The conversions of the 8 filesystems that actually support barriers
need to go through this tree anyway if we want to be able to test
it. Also the changes in the filesystem are absolutely minimal -
it's basically just s/WRITE_BARRIER/WRITE_FUA_FLUSH/ after my
initial patch kill BH_Orderd, and removing about 10 lines of code in
reiserfs.
I might just resequence it to finish this part of discussion but what
does that really buy us? It's not really gonna help bisection.
Bisection won't be able to tell anything in higher resolution than
"the new implementation doesn't work". If you show me how it would
actually help, I'll happily reshuffle the patches.
quoted
I wasn't sure about that part. You removed store_flush_error(), but
DM_ENDIO_REQUEUE should still have higher priority than other
failures, no?
Which priority?
IIUC, when any of flushes get DM_ENDIO_REQUEUE (which tells the dm
core layer to retry the whole bio later), it trumps all other failures
and the bio is retried later. That was why DM_ENDIO_REQUEUE was
prioritized over other error codes, which actually is sort of
incorrect in that once a FLUSH fails, it _MUST_ be reported to upper
layers as FLUSH failure implies data already lost. So,
DM_ENDIO_REQUEUE actually should have lower priority than other
failures. But, then again, the error codes still need to be
prioritized.
quoted
But how do you make sure REQ_FLUSHes for preflush finish before
starting the write?
Hmm, okay. I see how the special flush_bio makes the waiting easier,
let's see if Mike or other in the DM team have a better idea.
Yeah, it would be better if it can be sequenced w/o using a work but
let's leave it for later.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-17 16:59:29
On Tue, Aug 17, 2010 at 06:41:47PM +0200, Tejun Heo wrote:
quoted
What other trees do you mean?
I was mostly thinking about dm/md, drdb and stuff, but you're talking
about filesystem conversion patches being routed through block tree,
right?
I think we really need all the conversions in one tree, block layer,
remapping drivers and filesystems.
Btw, I've done the conversion for all filesystems and I'm running tests
over them now. Expect the series late today or tomorrow.
I might just resequence it to finish this part of discussion but what
does that really buy us? It's not really gonna help bisection.
Bisection won't be able to tell anything in higher resolution than
"the new implementation doesn't work". If you show me how it would
actually help, I'll happily reshuffle the patches.
It's not bisecting to find bugs in the barrier conversion. We can't
easily bisect it down anyway. The problem is when we try to bisect
other problems and get into the middle of the series barriers suddenly
are gone. Which is not very helpful for things like data integrity
problems in filesystems.
quoted
quoted
I wasn't sure about that part. You removed store_flush_error(), but
DM_ENDIO_REQUEUE should still have higher priority than other
failures, no?
Which priority?
IIUC, when any of flushes get DM_ENDIO_REQUEUE (which tells the dm
core layer to retry the whole bio later), it trumps all other failures
and the bio is retried later. That was why DM_ENDIO_REQUEUE was
prioritized over other error codes, which actually is sort of
incorrect in that once a FLUSH fails, it _MUST_ be reported to upper
layers as FLUSH failure implies data already lost. So,
DM_ENDIO_REQUEUE actually should have lower priority than other
failures. But, then again, the error codes still need to be
prioritized.
I think that's something we better leave to the DM team.
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-17 17:08:57
On Tue, Aug 17, 2010 at 06:23:55PM +0200, Tejun Heo wrote:
Yeap, agreed. I couldn't really understand why the the sequence
needed to be atomic for ide in the first place so just left it alone.
Do you understand why it tried to be atomic?
I think initial drafs of the barrier specification talked about atomic
sequences. Except for that I can't think of any reason.
Hello,
On 08/17/2010 07:08 PM, Christoph Hellwig wrote:
On Tue, Aug 17, 2010 at 06:23:55PM +0200, Tejun Heo wrote:
quoted
Yeap, agreed. I couldn't really understand why the the sequence
needed to be atomic for ide in the first place so just left it alone.
Do you understand why it tried to be atomic?
I think initial drafs of the barrier specification talked about atomic
sequences. Except for that I can't think of any reason.
Hello,
On 08/17/2010 06:59 PM, Christoph Hellwig wrote:
I think we really need all the conversions in one tree, block layer,
remapping drivers and filesystems.
I don't know. If filesystem changes are really trivial maybe, but
md/dm changes seem a bit too invasive to go through the block tree.
Btw, I've done the conversion for all filesystems and I'm running tests
over them now. Expect the series late today or tomorrow.
Cool. :-)
quoted
I might just resequence it to finish this part of discussion but what
does that really buy us? It's not really gonna help bisection.
Bisection won't be able to tell anything in higher resolution than
"the new implementation doesn't work". If you show me how it would
actually help, I'll happily reshuffle the patches.
It's not bisecting to find bugs in the barrier conversion. We can't
easily bisect it down anyway. The problem is when we try to bisect
other problems and get into the middle of the series barriers suddenly
are gone. Which is not very helpful for things like data integrity
problems in filesystems.
Ah, okay, hmmm.... alright, I'll resequence the patches. If the
filesystem changes can be put into a single tree somehow, we can keep
things mostly working at least for direct devices.
quoted
IIUC, when any of flushes get DM_ENDIO_REQUEUE (which tells the dm
core layer to retry the whole bio later), it trumps all other failures
and the bio is retried later. That was why DM_ENDIO_REQUEUE was
prioritized over other error codes, which actually is sort of
incorrect in that once a FLUSH fails, it _MUST_ be reported to upper
layers as FLUSH failure implies data already lost. So,
DM_ENDIO_REQUEUE actually should have lower priority than other
failures. But, then again, the error codes still need to be
prioritized.
I think that's something we better leave to the DM team.
Sure, but we shouldn't be ripping out the code to do that.
Thanks.
--
tejun
It's not bisecting to find bugs in the barrier conversion. We can't
easily bisect it down anyway. The problem is when we try to bisect
other problems and get into the middle of the series barriers suddenly
are gone. Which is not very helpful for things like data integrity
problems in filesystems.
Ah, okay, hmmm.... alright, I'll resequence the patches. If the
filesystem changes can be put into a single tree somehow, we can keep
things mostly working at least for direct devices.
Sorry but I'm doing it. It just doesn't make much sense. I can't
relax the ordering for REQ_HARDBARRIER without breaking the remapping
drivers. So, to keep things working, I'll have to 1. relax the
ordering 2. implement new REQ_FLUSH/FUA based interface and 3. use
them in the filesystems in the same patch. That's just wrong. And I
don't think md/dm changes can or should go through the block tree.
They're way too invasive for that. It's a new implementation and
barrier won't work (fail gracefully) for several commits during the
transition. I don't think there's a better way around it.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-18 09:46:20
FYI: One issue with this series is that make_request based drivers
not have to access all REQ_FLUSH and REQ_FUA requests. We'll either
need to add handling to empty REQ_FLUSH requests to all of them or
figure out a way to prevent them getting sent. That is assuming they'll
simply ignore REQ_FLUSH/REQ_FUA on normal writes.
As far as playing with ordered tags it's just adding a new flag for
it on the bio that gets passed down to the driver. For a final version
you'd need a queue-level feature if it's supported, but you don't
even need that for the initial work. Then you can implement a
variant of blk_do_flush that does away with queueing additional requests
once finish but queues all two or three at the same time with your
new ordered flag set, at which point you are back to the level or
ordered tag usage that the old code allows. You're still left with
all the hard problems of actually implementing error handling for it
and using it higher up in the filesystem and generic page cache code.
But how about file systems doing internal local order-by-drain? Without
converting them to use ordered commands it would be impossible to show
full potential of them and to make the conversion one would need deep
internal FS knowledge. That's my point. But if there's a trivial way to
see all such places in the filesystems code and convert, then OK, I agree.
I'd really love to see your results, up to the point of just trying
that once I get a little spare time. But my theory is that it won't
help us - the problem with ordered tags is that they enforce global
ordering while we currently have local ordering. While it will reduce
the latency for the process waiting for an fsync or similar it will
affect other I/O going on in the background and reduce the devices
ability to reorder that I/O.
The local ordering vs global ordering is relevant only if you have
several applications/threads load. But how about a single
application/thread?
Another point, for which, AFAIU, the ORDERED commands were invented, is
that they make ordering on the _another_ side of the link _after_ all
link/transfer latencies. This is why it's hard to see advantage of them
on local disks.
Vlad
If requested, I can develop the interface further.
I still think the benefit of ordering by tag would be marginal at
best, and what have you guys measured there? Under the current
framework, there's no easy way to measure full ordered-by-tag
implementation. The mechanism for filesystems to communicate the
ordering information (which would be a partially ordered graph) just
isn't there and there is no way the current usage of ordering-by-tag
only for barrier sequence can achieve anything close to that level of
difference.
Basically, I measured how iSCSI link utilization depends from amount of
queued commands and queued data size. This is why I made it as a table.
From it you can see which improvement you will have removing queue
draining after 1, 2, 4, etc. commands depending of commands sizes.
For instance, on my previous XFS rm example, where rm of 4 files took
3.5 minutes with nobarrier option, I could see that XFS was sending 1-3
32K commands in a row. From my table you can see that if it sent all
them at once without draining, it would have about 150-200% speed increase.
Vlad
Hello,
On 08/18/2010 09:30 PM, Vladislav Bolkhovitin wrote:
Basically, I measured how iSCSI link utilization depends from amount
of queued commands and queued data size. This is why I made it as a
table. From it you can see which improvement you will have removing
queue draining after 1, 2, 4, etc. commands depending of commands
sizes.
For instance, on my previous XFS rm example, where rm of 4 files
took 3.5 minutes with nobarrier option, I could see that XFS was
sending 1-3 32K commands in a row. From my table you can see that if
it sent all them at once without draining, it would have about
150-200% speed increase.
You compared barrier off/on. Of course, it will make a big
difference. I think good part of that gain should be realized by the
currently proposed patchset which removes draining. What's needed to
be demonstrated is the difference between ordered-by-waiting and
ordered-by-tag. We've never had code to do that properly.
The original ordered-by-tag we had only applied tag ordering to two or
three command sequences inside a barrier, which doesn't amount to much
(and could even be harmful as it imposes draining of all simple
commands inside the device only to reduce issue latencies for a few
commands). You'll need to hook into filesystem and somehow export the
ordering information down to the driver so that whatever needs
ordering is sent out as ordered commands.
As I've wrote multiple times, I'm pretty skeptical it will bring much.
Ordered tag mandates draining inside the device just like the original
barrier implementation. Sure, it's done at a lower layer and command
issue latencies will be reduced thanks to that but ordered-by-waiting
doesn't require _any_ draining at all. The whole pipeline can be kept
full all the time. I'm often wrong tho, so please feel free to go
ahead and prove me wrong. :-)
Thanks.
--
tejun
Hello,
On 08/18/2010 11:46 AM, Christoph Hellwig wrote:
FYI: One issue with this series is that make_request based drivers
not have to access all REQ_FLUSH and REQ_FUA requests. We'll either
need to add handling to empty REQ_FLUSH requests to all of them or
figure out a way to prevent them getting sent. That is assuming they'll
simply ignore REQ_FLUSH/REQ_FUA on normal writes.
Can you be a bit more specific? In most cases, request based drivers
should be fine. They sit behind the front most request_queue which
would discompose REQ_FLUSH/FUAs into appropriate command sequence.
For the request based drivers, it's not different from the original
REQ_HARDBARRIER mechanism, it'll just see flushes and optionally FUA
writes.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-19 10:20:23
On Thu, Aug 19, 2010 at 11:57:53AM +0200, Tejun Heo wrote:
On 08/18/2010 11:46 AM, Christoph Hellwig wrote:
quoted
FYI: One issue with this series is that make_request based drivers
not have to access all REQ_FLUSH and REQ_FUA requests. We'll either
need to add handling to empty REQ_FLUSH requests to all of them or
figure out a way to prevent them getting sent. That is assuming they'll
simply ignore REQ_FLUSH/REQ_FUA on normal writes.
Can you be a bit more specific? In most cases, request based drivers
should be fine. They sit behind the front most request_queue which
would discompose REQ_FLUSH/FUAs into appropriate command sequence.
I said make_request based drivers, that is drivers taking bios. These
get bios directly from __generic_make_request and need to deal with
REQ_FLUSH/FUA themselves. We have quite a few more than just dm/md of
this kind:
arch/powerpc/sysdev/axonram.c: blk_queue_make_request(bank->disk->queue, axon_ram_make_request);
drivers/block/aoe/aoeblk.c: blk_queue_make_request(d->blkq, aoeblk_make_request);
drivers/block/brd.c: blk_queue_make_request(brd->brd_queue, brd_make_request);
drivers/block/drbd/drbd_main.c: blk_queue_make_request(q, drbd_make_request_26);
drivers/block/loop.c: blk_queue_make_request(lo->lo_queue, loop_make_request);
drivers/block/pktcdvd.c: blk_queue_make_request(q, pkt_make_request);
drivers/block/ps3vram.c: blk_queue_make_request(queue, ps3vram_make_request);
drivers/block/umem.c: blk_queue_make_request(card->queue, mm_make_request);
drivers/s390/block/dcssblk.c: blk_queue_make_request(dev_info->dcssblk_queue, dcssblk_make_request);
drivers/s390/block/xpram.c: blk_queue_make_request(xpram_queues[i], xpram_make_request);
drivers/staging/zram/zram_drv.c:blk_queue_make_request(zram->queue, zram_make_request);
Hello,
On 08/19/2010 12:20 PM, Christoph Hellwig wrote:
I said make_request based drivers, that is drivers taking bios.
Right. Gees, it's confusing.
These get bios directly from __generic_make_request and need to deal
with REQ_FLUSH/FUA themselves. We have quite a few more than just
dm/md of this kind:
arch/powerpc/sysdev/axonram.c
drivers/block/aoe/aoeblk.c
drivers/block/brd.c
Hi Tejun, Christoph,
On Tue, Aug 17, 2010 at 06:41:47PM +0200, Tejun Heo wrote:
quoted
quoted
I wasn't sure about that part. You removed store_flush_error(), but
DM_ENDIO_REQUEUE should still have higher priority than other
failures, no?
Which priority?
IIUC, when any of flushes get DM_ENDIO_REQUEUE (which tells the dm
core layer to retry the whole bio later), it trumps all other failures
and the bio is retried later. That was why DM_ENDIO_REQUEUE was
prioritized over other error codes, which actually is sort of
incorrect in that once a FLUSH fails, it _MUST_ be reported to upper
layers as FLUSH failure implies data already lost. So,
DM_ENDIO_REQUEUE actually should have lower priority than other
failures. But, then again, the error codes still need to be
prioritized.
I think that's correct and changing the priority of DM_ENDIO_REQUEUE
for REQ_FLUSH down to the lowest should be fine.
(I didn't know that FLUSH failure implies data loss possibility.)
But the patch is not enough, you have to change target drivers, too.
E.g. As for multipath, you need to change
drivers/md/dm-mpath.c:do_end_io() to return error for REQ_FLUSH
like the REQ_DISCARD support included in 2.6.36-rc1.
By the way, if these patch-set with the change above are included,
even one path failure for REQ_FLUSH on multipath configuration will
be reported to upper layer as error, although it's retried using
other paths currently.
Then, if an upper layer won't take correct recovery action for the error,
it would be seen as a regression for users. (e.g. Frequent EXT3-error
resulting in read-only mount on multipath configuration.)
Although I think the explicit error is fine rather than implicit data
corruption, please check upper layers carefully so that users won't see
such errors as much as possible.
Thanks,
Kiyoshi Ueda
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-20 13:22:14
FYI: here's a little writeup to document the new cache flushing scheme,
intended to replace Documentation/block/barriers.txt. Any good
suggestion for a filename in the kernel tree?
---
Explicit volatile write cache control
=====================================
Introduction
------------
Many storage devices, especially in the consumer market, come with volatile
write back caches. That means the devices signal I/O completion to the
operating system before data actually has hit the physical medium. This
behavior obviously speeds up various workloads, but it means the operating
system needs to force data out to the physical medium when it performs
a data integrity operation like fsync, sync or an unmount.
The Linux block layer provides a two simple mechanism that lets filesystems
control the caching behavior of the storage device. These mechanisms are
a forced cache flush, and the Force Unit Access (FUA) flag for requests.
Explicit cache flushes
----------------------
The REQ_FLUSH flag can be OR ed into the r/w flags of a bio submitted from the
filesystem and will make sure the volatile cache of the storage device
has been flushed before the actual I/O operation is started. The explicit
guarantees write requests that have completed before the bio was submitted
actually are on the physical medium before this request has started.
In addition the REQ_FLUSH flag can be set on an otherwise empty bio
structure, which causes only an explicit cache flush without any dependent
I/O. It is recommend to use the blkdev_issue_flush() helper for a pure
cache flush.
Forced Unit Access
-----------------
The REQ_FUA flag can be OR ed into the r/w flags of a bio submitted from the
filesystem and will make sure that I/O completion for this requests is not
signaled before the data has made it to non-volatile storage on the
physical medium.
Implementation details for filesystems
--------------------------------------
Filesystem can simply set the REQ_FLUSH and REQ_FUA bits and do not have to
worry if the underlying devices need any explicit cache flushing and how
the Forced Unit Access is implemented. The REQ_FLUSH and REQ_FUA flags
may both be set on a single bio.
Implementation details for make_request_fn based block drivers
--------------------------------------------------------------
These drivers will always see the REQ_FLUSH and REQ_FUA bits as they sit
directly below the submit_bio interface. For remapping drivers the REQ_FUA
bits needs to be propagate to underlying devices, and a global flush needs
to be implemented for bios with the REQ_FLUSH bit set. For real device
drivers that do not have a volatile cache the REQ_FLUSH and REQ_FUA bits
on non-empty bios can simply be ignored, and REQ_FLUSH requests without
data can be completed successfully without doing any work. Drivers for
devices with volatile caches need to implement the support for these
flags themselves without any help from the block layer.
Implementation details for request_fn based block drivers
--------------------------------------------------------------
For devices that do not support volatile write caches there is no driver
support required, the block layer completes empty REQ_FLUSH requests before
entering the driver and strips off the REQ_FLUSH and REQ_FUA bits from
requests that have a payload. For device with volatile write caches the
driver needs to tell the block layer that it supports flushing caches by
doing:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH);
and handle empty REQ_FLUSH requests in it's prep_fn/request_fn. Note that
REQ_FLUSH requests with a payload are automatically turned into a sequence
of empty REQ_FLUSH and the actual write by the block layer. For devices
that also support the FUA bit the block layer needs to be told to pass
through that bit using:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH | REQ_FUA);
and handle write requests that have the REQ_FUA bit set properly in it's
prep_fn/request_fn. If the FUA bit is not natively supported the block
layer turns it into an empty REQ_FLUSH requests after the actual write.
FYI: here's a little writeup to document the new cache flushing scheme,
intended to replace Documentation/block/barriers.txt. Any good
suggestion for a filename in the kernel tree?
---
I was thinking that we might be better off using the "durable writes" term more
since it is well documented (at least in the database world, where it is the "D"
in ACID properties). Maybe "durable_writes_support.txt" ?
Explicit volatile write cache control
=====================================
Introduction
------------
Many storage devices, especially in the consumer market, come with volatile
write back caches. That means the devices signal I/O completion to the
operating system before data actually has hit the physical medium. This
behavior obviously speeds up various workloads, but it means the operating
system needs to force data out to the physical medium when it performs
a data integrity operation like fsync, sync or an unmount.
The Linux block layer provides a two simple mechanism that lets filesystems
control the caching behavior of the storage device. These mechanisms are
a forced cache flush, and the Force Unit Access (FUA) flag for requests.
Should we mention that users can also disable the write cache on the target device?
It might also be worth mentioning that storage needs to be properly configured -
i.e., an internal hardware RAID card with battery backing needs can expose
itself as a writethrough cache *only if* it actually has control over all of the
backend disks and can flush/disable their write caches.
Maybe that is too much detail, but I know that people have lost data with some
of these setups.
The rest of the write up below sounds good, thanks for pulling this together!
Ric
Explicit cache flushes
----------------------
The REQ_FLUSH flag can be OR ed into the r/w flags of a bio submitted from the
filesystem and will make sure the volatile cache of the storage device
has been flushed before the actual I/O operation is started. The explicit
guarantees write requests that have completed before the bio was submitted
actually are on the physical medium before this request has started.
In addition the REQ_FLUSH flag can be set on an otherwise empty bio
structure, which causes only an explicit cache flush without any dependent
I/O. It is recommend to use the blkdev_issue_flush() helper for a pure
cache flush.
Forced Unit Access
-----------------
The REQ_FUA flag can be OR ed into the r/w flags of a bio submitted from the
filesystem and will make sure that I/O completion for this requests is not
signaled before the data has made it to non-volatile storage on the
physical medium.
Implementation details for filesystems
--------------------------------------
Filesystem can simply set the REQ_FLUSH and REQ_FUA bits and do not have to
worry if the underlying devices need any explicit cache flushing and how
the Forced Unit Access is implemented. The REQ_FLUSH and REQ_FUA flags
may both be set on a single bio.
Implementation details for make_request_fn based block drivers
--------------------------------------------------------------
These drivers will always see the REQ_FLUSH and REQ_FUA bits as they sit
directly below the submit_bio interface. For remapping drivers the REQ_FUA
bits needs to be propagate to underlying devices, and a global flush needs
to be implemented for bios with the REQ_FLUSH bit set. For real device
drivers that do not have a volatile cache the REQ_FLUSH and REQ_FUA bits
on non-empty bios can simply be ignored, and REQ_FLUSH requests without
data can be completed successfully without doing any work. Drivers for
devices with volatile caches need to implement the support for these
flags themselves without any help from the block layer.
Implementation details for request_fn based block drivers
--------------------------------------------------------------
For devices that do not support volatile write caches there is no driver
support required, the block layer completes empty REQ_FLUSH requests before
entering the driver and strips off the REQ_FLUSH and REQ_FUA bits from
requests that have a payload. For device with volatile write caches the
driver needs to tell the block layer that it supports flushing caches by
doing:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH);
and handle empty REQ_FLUSH requests in it's prep_fn/request_fn. Note that
REQ_FLUSH requests with a payload are automatically turned into a sequence
of empty REQ_FLUSH and the actual write by the block layer. For devices
that also support the FUA bit the block layer needs to be told to pass
through that bit using:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH | REQ_FUA);
and handle write requests that have the REQ_FUA bit set properly in it's
prep_fn/request_fn. If the FUA bit is not natively supported the block
layer turns it into an empty REQ_FLUSH requests after the actual write.
From: Chris Mason <hidden> Date: 2010-08-20 16:00:09
On Fri, Aug 20, 2010 at 11:18:07AM -0400, Ric Wheeler wrote:
On 08/20/2010 09:22 AM, Christoph Hellwig wrote:
quoted
FYI: here's a little writeup to document the new cache flushing scheme,
intended to replace Documentation/block/barriers.txt. Any good
suggestion for a filename in the kernel tree?
---
I was thinking that we might be better off using the "durable
writes" term more since it is well documented (at least in the
database world, where it is the "D" in ACID properties). Maybe
"durable_writes_support.txt" ?
On Fri, Aug 20, 2010 at 11:18:07AM -0400, Ric Wheeler wrote:
quoted
On 08/20/2010 09:22 AM, Christoph Hellwig wrote:
quoted
FYI: here's a little writeup to document the new cache flushing scheme,
intended to replace Documentation/block/barriers.txt. Any good
suggestion for a filename in the kernel tree?
---
I was thinking that we might be better off using the "durable
writes" term more since it is well documented (at least in the
database world, where it is the "D" in ACID properties). Maybe
"durable_writes_support.txt" ?
Hello,
On 08/20/2010 10:26 AM, Kiyoshi Ueda wrote:
I think that's correct and changing the priority of DM_ENDIO_REQUEUE
for REQ_FLUSH down to the lowest should be fine.
(I didn't know that FLUSH failure implies data loss possibility.)
At least on ATA, FLUSH failure implies that data is already lost, so
the error can't be ignored or retried.
But the patch is not enough, you have to change target drivers, too.
E.g. As for multipath, you need to change
drivers/md/dm-mpath.c:do_end_io() to return error for REQ_FLUSH
like the REQ_DISCARD support included in 2.6.36-rc1.
I'll take a look but is there an easy to test mpath other than having
fancy hardware?
By the way, if these patch-set with the change above are included,
even one path failure for REQ_FLUSH on multipath configuration will
be reported to upper layer as error, although it's retried using
other paths currently.
Then, if an upper layer won't take correct recovery action for the error,
it would be seen as a regression for users. (e.g. Frequent EXT3-error
resulting in read-only mount on multipath configuration.)
Although I think the explicit error is fine rather than implicit data
corruption, please check upper layers carefully so that users won't see
such errors as much as possible.
Argh... then it will have to discern why FLUSH failed. It can retry
for transport errors but if it got aborted by the device it should
report upwards. Maybe just turn off barrier support in mpath for now?
Thanks.
--
tejun
FYI: here's a little writeup to document the new cache flushing scheme,
intended to replace Documentation/block/barriers.txt. Any good
suggestion for a filename in the kernel tree?
I was thinking that we might be better off using the "durable
writes" term more since it is well documented (at least in the
database world, where it is the "D" in ACID properties). Maybe
"durable_writes_support.txt" ?
The term is very foreign to people outside of enterprise / database
loop. writeback-cache.txt or write-cache-control.txt sounds good
enough to me.
quoted
The Linux block layer provides a two simple mechanism that lets filesystems
control the caching behavior of the storage device. These mechanisms are
a forced cache flush, and the Force Unit Access (FUA) flag for requests.
Should we mention that users can also disable the write cache on the
target device?
It might also be worth mentioning that storage needs to be properly
configured - i.e., an internal hardware RAID card with battery
backing needs can expose itself as a writethrough cache *only if* it
actually has control over all of the backend disks and can
flush/disable their write caches.
It might be useful to give several example configurations with
different cache configurations. I don't have much experience with
battery backed arrays but aren't they suppose to report write through
cache automatically?
Thanks.
--
tejun
Hello,
On 08/20/2010 03:22 PM, Christoph Hellwig wrote:
Many storage devices, especially in the consumer market, come with volatile
write back caches. That means the devices signal I/O completion to the
operating system before data actually has hit the physical medium.
A bit nit picky but flash devices can also have writeback caches and
the term physical medium sounds a bit off for those cases. Maybe just
saying "non-volatile media" is better?
Implementation details for filesystems
--------------------------------------
Filesystem can simply set the REQ_FLUSH and REQ_FUA bits and do not have to
worry if the underlying devices need any explicit cache flushing and how
the Forced Unit Access is implemented. The REQ_FLUSH and REQ_FUA flags
may both be set on a single bio.
It may be worthwhile to explain the sequence of operations when
REQ_FLUSH + data + REQ_FUA is executed. It can be extrapolated from
the previous two descriptions but I think giving examples of different
sequences depending on FLUSH/FUA configuration would be helpful to
help understanding the overall picture of things.
Other than those, looks good to me.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-23 12:48:15
On Mon, Aug 23, 2010 at 02:30:33PM +0200, Tejun Heo wrote:
It might be useful to give several example configurations with
different cache configurations. I don't have much experience with
battery backed arrays but aren't they suppose to report write through
cache automatically?
They usually do. I have one that doesn't, but SYNCHRONIZE CACHE on
it is so fast that it effectively must be a no-op.
On Mon, Aug 23, 2010 at 02:30:33PM +0200, Tejun Heo wrote:
quoted
It might be useful to give several example configurations with
different cache configurations. I don't have much experience with
battery backed arrays but aren't they suppose to report write through
cache automatically?
They usually do. I have one that doesn't, but SYNCHRONIZE CACHE on
it is so fast that it effectively must be a no-op.
Arrays are not a problem in general - they normally have internally, redundant
batteries to hold up the cache.
The issue is when you have an internal hardware RAID card with a large cache.
Those cards sit in your server and the batteries on the card protect its
internal cache, but do not have the capacity to hold up the drives behind it.
Normally, those drives should have their write cache disabled, but sometimes
(especially with S-ATA disks) this is not done.
ric
On Mon, Aug 23, 2010 at 02:30:33PM +0200, Tejun Heo wrote:
quoted
It might be useful to give several example configurations with
different cache configurations. I don't have much experience with
battery backed arrays but aren't they suppose to report write through
cache automatically?
They usually do. I have one that doesn't, but SYNCHRONIZE CACHE on
it is so fast that it effectively must be a no-op.
Arrays are not a problem in general - they normally have internally, redundant
batteries to hold up the cache.
The issue is when you have an internal hardware RAID card with a large cache.
Those cards sit in your server and the batteries on the card protect its
internal cache, but do not have the capacity to hold up the drives behind it.
Normally, those drives should have their write cache disabled, but sometimes
(especially with S-ATA disks) this is not done.
The problem purely exists on arrays that report write back cache enabled
AND don't implement SYNC_CACHE as a noop. Do any of them exist, or are
they purely urban legend?
--
Jens Axboe
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-23 14:05:51
Below is an updated version of the documentation. It fixes several
typos Zach Brown noticed and replaces all references to a physical
medium with the term non-volatile storage. I haven't added any examples
yet as I need to figure how they fit into the rest of the document.
---
Explicit volatile write cache control
=====================================
Introduction
------------
Many storage devices, especially in the consumer market, come with volatile
write back caches. That means the devices signal I/O completion to the
operating system before data actually has hit the non-volatile storage. This
behavior obviously speeds up various workloads, but it means the operating
system needs to force data out to the non-volatile storage when it performs
a data integrity operation like fsync, sync or an unmount.
The Linux block layer provides two simple mechanism that lets filesystems
control the caching behavior of the storage device. These mechanisms are
a forced cache flush, and the Force Unit Access (FUA) flag for requests.
Explicit cache flushes
----------------------
The REQ_FLUSH flag can be OR ed into the r/w flags of a bio submitted from
the filesystem and will make sure the volatile cache of the storage device
has been flushed before the actual I/O operation is started. This explicitly
guarantees that previously completed write requests are on non-volatile
storage before the flagged bio starts. In addition the REQ_FLUSH flag can be
set on an otherwise empty bio structure, which causes only an explicit cache
flush without any dependent I/O. It is recommend to use
the blkdev_issue_flush() helper for a pure cache flush.
Forced Unit Access
-----------------
The REQ_FUA flag can be OR ed into the r/w flags of a bio submitted from the
filesystem and will make sure that I/O completion for this requests is only
signaled after the data has been commited to non-volatile storage.
Implementation details for filesystems
--------------------------------------
Filesystem can simply set the REQ_FLUSH and REQ_FUA bits and do not have to
worry if the underlying devices need any explicit cache flushing and how
the Forced Unit Access is implemented. The REQ_FLUSH and REQ_FUA flags
may both be set on a single bio.
Implementation details for make_request_fn based block drivers
--------------------------------------------------------------
These drivers will always see the REQ_FLUSH and REQ_FUA bits as they sit
directly below the submit_bio interface. For remapping drivers the REQ_FUA
bits need to be propagated to underlying devices, and a global flush needs
to be implemented for bios with the REQ_FLUSH bit set. For real device
drivers that do not have a volatile cache the REQ_FLUSH and REQ_FUA bits
on non-empty bios can simply be ignored, and REQ_FLUSH requests without
data can be completed successfully without doing any work. Drivers for
devices with volatile caches need to implement the support for these
flags themselves without any help from the block layer.
Implementation details for request_fn based block drivers
--------------------------------------------------------------
For devices that do not support volatile write caches there is no driver
support required, the block layer completes empty REQ_FLUSH requests before
entering the driver and strips off the REQ_FLUSH and REQ_FUA bits from
requests that have a payload. For devices with volatile write caches the
driver needs to tell the block layer that it supports flushing caches by
doing:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH);
and handle empty REQ_FLUSH requests in its prep_fn/request_fn. Note that
REQ_FLUSH requests with a payload are automatically turned into a sequence
of empty REQ_FLUSH and the actual write by the block layer. For devices
that also support the FUA bit the block layer needs to be told to pass
through that bit using:
blk_queue_flush(sdkp->disk->queue, REQ_FLUSH | REQ_FUA);
and handle write requests that have the REQ_FUA bit set properly in its
prep_fn/request_fn. If the FUA bit is not natively supported the block
layer turns it into an empty REQ_FLUSH request after the actual write.
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-23 14:08:52
On Mon, Aug 23, 2010 at 04:01:15PM +0200, Jens Axboe wrote:
The problem purely exists on arrays that report write back cache enabled
AND don't implement SYNC_CACHE as a noop. Do any of them exist, or are
they purely urban legend?
I haven't seen it. I don't care particularly about this case, but once
it a while people want to disable flushing for testing or because they
really don't care.
What about adding a sysfs attribue to every request_queue that allows
disabling the cache flushing feature? Compared to the barrier option
this controls the feature at the right level and makes it available
to everyone instead of beeing duplicated. After a while we can then
simply ignore the barrier/nobarrier options.
Hello,
On 08/23/2010 04:08 PM, Christoph Hellwig wrote:
On Mon, Aug 23, 2010 at 04:01:15PM +0200, Jens Axboe wrote:
quoted
The problem purely exists on arrays that report write back cache enabled
AND don't implement SYNC_CACHE as a noop. Do any of them exist, or are
they purely urban legend?
I haven't seen it. I don't care particularly about this case, but once
it a while people want to disable flushing for testing or because they
really don't care.
What about adding a sysfs attribue to every request_queue that allows
disabling the cache flushing feature? Compared to the barrier option
this controls the feature at the right level and makes it available
to everyone instead of beeing duplicated. After a while we can then
simply ignore the barrier/nobarrier options.
Yeah, that sounds reasonable. blk_queue_flush() can be called anytime
without locking anyway, so it should be really easy to implement too.
Thanks.
--
tejun
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-23 14:15:12
We need to call blk_rq_init and elv_insert for all cases in queue_next_fseq,
so take these calls into common code. Also move the end_io initialization
from queue_flush into queue_next_fseq and rename queue_flush to
init_flush_request now that it's old name doesn't apply anymore.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/block/blk-flush.c
===================================================================
From: Mike Snitzer <hidden> Date: 2010-08-23 14:17:33
On Mon, Aug 23 2010 at 8:14am -0400,
Tejun Heo [off-list ref] wrote:
Hello,
On 08/20/2010 10:26 AM, Kiyoshi Ueda wrote:
quoted
I think that's correct and changing the priority of DM_ENDIO_REQUEUE
for REQ_FLUSH down to the lowest should be fine.
(I didn't know that FLUSH failure implies data loss possibility.)
At least on ATA, FLUSH failure implies that data is already lost, so
the error can't be ignored or retried.
quoted
But the patch is not enough, you have to change target drivers, too.
E.g. As for multipath, you need to change
drivers/md/dm-mpath.c:do_end_io() to return error for REQ_FLUSH
like the REQ_DISCARD support included in 2.6.36-rc1.
I'll take a look but is there an easy to test mpath other than having
fancy hardware?
It is easy enough to make a single path use mpath. Just verify/modify
/etc/multipath.conf so that your device isn't blacklisted.
multipathd will even work with a scsi-debug device.
You obviously won't get path failover but you'll see the path get marked
faulty, etc.
quoted
By the way, if these patch-set with the change above are included,
even one path failure for REQ_FLUSH on multipath configuration will
be reported to upper layer as error, although it's retried using
other paths currently.
Then, if an upper layer won't take correct recovery action for the error,
it would be seen as a regression for users. (e.g. Frequent EXT3-error
resulting in read-only mount on multipath configuration.)
Although I think the explicit error is fine rather than implicit data
corruption, please check upper layers carefully so that users won't see
such errors as much as possible.
Argh... then it will have to discern why FLUSH failed. It can retry
for transport errors but if it got aborted by the device it should
report upwards.
Yes, we discussed this issue of needing to train dm-multipath to know if
there was a transport failure or not (at LSF). But I'm not sure when
Hannes intends to repost his work in this area (updated to account for
feedback from LSF).
Maybe just turn off barrier support in mpath for now?
I think we'd prefer to have a device fail rather than jeopardize data
integrity. Clearly not ideal but...
From: Christoph Hellwig <hch@lst.de> Date: 2010-08-23 14:19:35
On Mon, Aug 23, 2010 at 04:13:36PM +0200, Tejun Heo wrote:
Yeah, that sounds reasonable. blk_queue_flush() can be called anytime
without locking anyway, so it should be really easy to implement too.
I don't think we can simply call blk_queue_flush - we must ensure to
never set more bits than the device allows. We'll just need two
sets of flags in the request queue, with the sysfs file checking that
it never allows more flags than blk_queue_flush.
I'll prepare a patch for this on top of the current series.
On Mon, Aug 23, 2010 at 02:30:33PM +0200, Tejun Heo wrote:
quoted
It might be useful to give several example configurations with
different cache configurations. I don't have much experience with
battery backed arrays but aren't they suppose to report write through
cache automatically?
They usually do. I have one that doesn't, but SYNCHRONIZE CACHE on
it is so fast that it effectively must be a no-op.
Arrays are not a problem in general - they normally have internally, redundant
batteries to hold up the cache.
The issue is when you have an internal hardware RAID card with a large cache.
Those cards sit in your server and the batteries on the card protect its
internal cache, but do not have the capacity to hold up the drives behind it.
Normally, those drives should have their write cache disabled, but sometimes
(especially with S-ATA disks) this is not done.
The problem purely exists on arrays that report write back cache enabled
AND don't implement SYNC_CACHE as a noop. Do any of them exist, or are
they purely urban legend?
Hi Jens,
There are actually two distinct problems:
(1) arrays with a non-volatile write cache (battery backed, navram, whatever)
that do not NOOP a SYNC_CACHE command. I know of one brand that seems to do
this, but it is not a common brand. If we do not issue flushes for write through
caches, I think that we will avoid this in any case.
(2) hardware raid cards with internal buffer memory and on-card battery backup
(they sit in your server, disks sit in jbod like expansion shelves). These are
fine if the drives in those shelves have write cache disabled.
ric
From: John Robinson <hidden> Date: 2010-08-23 16:28:19
On 23/08/2010 15:15, Christoph Hellwig wrote:
We need to call blk_rq_init and elv_insert for all cases in queue_next_fseq,
so take these calls into common code. Also move the end_io initialization
from queue_flush into queue_next_fseq and rename queue_flush to
init_flush_request now that it's old name doesn't apply anymore.
Nit: it's "its" above, not "it's". If in doubt, if it's "it is" (or "it
has") it's "it's" but if it could be "his" or "hers" it's "its".
I'm guessing English isn't your first language (a) because of your .de
address and (b) because it's better than most British people's, but
still, it's a common mistake. If I can remember any of the German I
studied all those years ago, "its" is roughly equivalent to "sein", and
"it's" to "es ist".
Cheers,
John.
On Mon, Aug 23, 2010 at 11:19:13AM -0400, Ric Wheeler wrote:
[...]
(2) hardware raid cards with internal buffer memory and on-card battery backup
(they sit in your server, disks sit in jbod like expansion shelves). These are
fine if the drives in those shelves have write cache disabled.
Actually some of such cards keep write cache on the drives enabled and
issue FLUSH CACHE commands to the drives. E.g., 3ware 9690SA behaves
like this at least with SATA drives (the FLUSH CACHE commands can be
seen after enabling performance monitoring - they often end up in the
"10 commands having the largest latency" table). This can actually be
safe if the card waits for the FLUSH CACHE completion before making
the write cache data in its battery-backed memory available for reuse
(and the drive implements the FLUSH CACHE command correctly).
On Mon, Aug 23, 2010 at 11:19:13AM -0400, Ric Wheeler wrote:
[...]
quoted
(2) hardware raid cards with internal buffer memory and on-card battery backup
(they sit in your server, disks sit in jbod like expansion shelves). These are
fine if the drives in those shelves have write cache disabled.
Actually some of such cards keep write cache on the drives enabled and
issue FLUSH CACHE commands to the drives. E.g., 3ware 9690SA behaves
like this at least with SATA drives (the FLUSH CACHE commands can be
seen after enabling performance monitoring - they often end up in the
"10 commands having the largest latency" table). This can actually be
safe if the card waits for the FLUSH CACHE completion before making
the write cache data in its battery-backed memory available for reuse
(and the drive implements the FLUSH CACHE command correctly).
Yes - this is certainly one way to do it. Note that this will not work if the
card advertises itself as a write through cache (and we end up not sending down
the SYNC_CACHE commands).
At least one hardware RAID card (I unfortunately cannot mention the brand) did
not do this command forwarding.
ric
Hi Tejun,
On 08/23/2010 11:17 PM +0900, Mike Snitzer wrote:
On Mon, Aug 23 2010 at 8:14am -0400, Tejun Heo [off-list ref] wrote:
quoted
On 08/20/2010 10:26 AM, Kiyoshi Ueda wrote:
quoted
By the way, if these patch-set with the change above are included,
even one path failure for REQ_FLUSH on multipath configuration will
be reported to upper layer as error, although it's retried using
other paths currently.
Then, if an upper layer won't take correct recovery action for the error,
it would be seen as a regression for users. (e.g. Frequent EXT3-error
resulting in read-only mount on multipath configuration.)
Although I think the explicit error is fine rather than implicit data
corruption, please check upper layers carefully so that users won't see
such errors as much as possible.
Argh... then it will have to discern why FLUSH failed. It can retry
for transport errors but if it got aborted by the device it should
report upwards.
Yes, we discussed this issue of needing to train dm-multipath to know if
there was a transport failure or not (at LSF). But I'm not sure when
Hannes intends to repost his work in this area (updated to account for
feedback from LSF).
Yes, checking whether it's a transport error in lower layer is
the right solution.
(Since I know it's not available yet, I just hoped if upper layers
had some other options.)
Anyway, only reporting errors for REQ_FLUSH to upper layer without
such a solution would make dm-multipath almost unusable in real world,
although it's better than implicit data loss.
quoted
Maybe just turn off barrier support in mpath for now?
If it's possible, it could be a workaround for a short term.
But how can you do that?
I think it's not enough to just drop REQ_FLUSH flag from q->flush_flags.
Underlying devices of a mpath device may have write-back cache and
it may be enabled.
So if a mpath device doesn't set REQ_FLUSH flag in q->flush_flags, it
becomes a device which has write-back cache but doesn't support flush.
Then, upper layer can do nothing to ensure cache flush?
Thanks,
Kiyoshi Ueda
Hello,
On 08/24/2010 12:24 PM, Kiyoshi Ueda wrote:
Yes, checking whether it's a transport error in lower layer is
the right solution.
(Since I know it's not available yet, I just hoped if upper layers
had some other options.)
Anyway, only reporting errors for REQ_FLUSH to upper layer without
such a solution would make dm-multipath almost unusable in real world,
although it's better than implicit data loss.
I see.
quoted
quoted
Maybe just turn off barrier support in mpath for now?
If it's possible, it could be a workaround for a short term.
But how can you do that?
I think it's not enough to just drop REQ_FLUSH flag from q->flush_flags.
Underlying devices of a mpath device may have write-back cache and
it may be enabled.
So if a mpath device doesn't set REQ_FLUSH flag in q->flush_flags, it
becomes a device which has write-back cache but doesn't support flush.
Then, upper layer can do nothing to ensure cache flush?
Yeah, I was basically suggesting to forget about cache flush w/ mpath
until it can be fixed. You're saying that if mpath just passes
REQ_FLUSH upwards without retrying, it will be almost unuseable,
right? I'm not sure how to proceed here. How much work would
discerning between transport and IO errors take? If it can't be done
quickly enough the retry logic can be kept around to keep the old
behavior but that already was a broken behavior, so... :-(
Thanks.
--
tejun
I think that's correct and changing the priority of DM_ENDIO_REQUEUE
for REQ_FLUSH down to the lowest should be fine.
(I didn't know that FLUSH failure implies data loss possibility.)
At least on ATA, FLUSH failure implies that data is already lost, so
the error can't be ignored or retried.
In SCSI there are conditions when a command, including FLUSH
(SYNC_CACHE), failed which don't imply lost data. For them the caller
expected to retry the failed command. Most common cases are Unit
Attentions and TASK QUEUE FULL status.
Vlad
From: Mike Snitzer <hidden> Date: 2010-08-24 17:52:16
On Tue, Aug 24 2010 at 12:59pm -0400,
Tejun Heo [off-list ref] wrote:
Hello,
On 08/24/2010 12:24 PM, Kiyoshi Ueda wrote:
quoted
Yes, checking whether it's a transport error in lower layer is
the right solution.
(Since I know it's not available yet, I just hoped if upper layers
had some other options.)
Anyway, only reporting errors for REQ_FLUSH to upper layer without
such a solution would make dm-multipath almost unusable in real world,
although it's better than implicit data loss.
I see.
quoted
quoted
quoted
Maybe just turn off barrier support in mpath for now?
If it's possible, it could be a workaround for a short term.
But how can you do that?
I think it's not enough to just drop REQ_FLUSH flag from q->flush_flags.
Underlying devices of a mpath device may have write-back cache and
it may be enabled.
So if a mpath device doesn't set REQ_FLUSH flag in q->flush_flags, it
becomes a device which has write-back cache but doesn't support flush.
Then, upper layer can do nothing to ensure cache flush?
Yeah, I was basically suggesting to forget about cache flush w/ mpath
until it can be fixed. You're saying that if mpath just passes
REQ_FLUSH upwards without retrying, it will be almost unuseable,
right? I'm not sure how to proceed here.
Seems clear that we must fix mpath to receive the SCSI errors, in some
form, so it can decide if a retry is required/valid or not.
Such error processing was a big selling point for the transition from
bio-based to request-based multipath; so it's unfortunate that this
piece has been left until now.
How much work would discerning between transport and IO errors take?
Hannes already proposed some patches:
https://patchwork.kernel.org/patch/61282/https://patchwork.kernel.org/patch/61283/https://patchwork.kernel.org/patch/61596/
This work was discussed at LSF, see "Error Handling - Hannes Reinecke"
here: http://lwn.net/Articles/400589/
I thought James, Alasdair and others offered some guidance on what he'd
like to see...
Unfortunately, even though I was at this LSF session, I can't recall any
specific consensus on how Hannes' work should be refactored (to avoid
adding SCSI sense processing code directly in dm-mpath). Maybe James,
Hannes or others remember?
Was it enough to just have the SCSI sense processing code split out in a
new sub-section of the SCSI midlayer -- and then DM calls that code?
If it can't be done quickly enough the retry logic can be kept around
to keep the old behavior but that already was a broken behavior, so...
:-(
I'll have to review this thread again to understand why mpath's existing
retry logic is broken behavior. mpath is used with more capable SCSI
devices so I'm missing why a failed FLUSH implies data loss.
Mike
Hello,
On 08/24/2010 07:52 PM, Mike Snitzer wrote:
quoted
If it can't be done quickly enough the retry logic can be kept around
to keep the old behavior but that already was a broken behavior, so...
:-(
I'll have to review this thread again to understand why mpath's existing
retry logic is broken behavior. mpath is used with more capable SCSI
devices so I'm missing why a failed FLUSH implies data loss.
SBC doesn't specify the failure behavior, so it could be that retrying
flush could be safe. But for most disk type devices, flush failure
usually indicates that the device exhausted all the options to commit
some of pending data to NV media - ie. even remapping failed for
whatever reason. Even if retry is safe, it's more likely to simply
delay notification of failure.
In ATA, the situation is clearer, when a device actively fails a
flush, the drive reports the first failed sector it failed to commit
and the next flush will continue _after_ the sector - IOW, data is
already lost.
<speculation>
I think there's no reason mpath should be tasked with retrying flush
failure. That's upto the SCSI EH. If the command failed in 'safe'
transient way - ie. device busy or whatnot, SCSI EH can and does retry
the command. There are several FAILFAST bits already and SCSI EH can
avoid retrying transport errors for mpath (maybe it already does
that?) and just need to be able to tell upper layer that the failure
was a fast one and upper layer is responsible for retrying? Is there
any reason to pass the whole sense information upwards?
</speculation>
Anyways, flush failure is different from read/write failures.
Read/writes can always be retried cleanly. They are stateless. I
don't know how SCSI devices would actually behavior but it's a bit
scary to retry SYNCHRONIZE_CACHE a device failed and report success
upwards.
Thanks.
--
tejun
In SCSI there are conditions when a command, including FLUSH
(SYNC_CACHE), failed which don't imply lost data. For them the caller
expected to retry the failed command. Most common cases are Unit
Attentions and TASK QUEUE FULL status.
ATA expects the command to be retried as well because a failed flush
indicates the specific sector is lost (unless the host still has a copy
of course - which is *very* likely although we don't use it) but the rest
of the flush transaction can be retried to continue to flush sectors
beyond the failed one.
Alan
Hi Tejun,
On 08/25/2010 01:59 AM +0900, Tejun Heo wrote:
On 08/24/2010 12:24 PM, Kiyoshi Ueda wrote:
quoted
Anyway, only reporting errors for REQ_FLUSH to upper layer without
such a solution would make dm-multipath almost unusable in real world,
although it's better than implicit data loss.
I see.
quoted
quoted
Maybe just turn off barrier support in mpath for now?
If it's possible, it could be a workaround for a short term.
But how can you do that?
I think it's not enough to just drop REQ_FLUSH flag from q->flush_flags.
Underlying devices of a mpath device may have write-back cache and
it may be enabled.
So if a mpath device doesn't set REQ_FLUSH flag in q->flush_flags, it
becomes a device which has write-back cache but doesn't support flush.
Then, upper layer can do nothing to ensure cache flush?
Yeah, I was basically suggesting to forget about cache flush w/ mpath
until it can be fixed. You're saying that if mpath just passes
REQ_FLUSH upwards without retrying, it will be almost unuseable,
right?
Right.
If the error is safe/needed to retry using other paths, mpath should
retry even if REQ_FLUSH. Otherwise, only one path failure may result
in system down.
Just passing any REQ_FLUSH error upwards regardless the error type
will make such situations, and users will feel the behavior as
unstable/unusable.
I'm not sure how to proceed here. How much work would
discerning between transport and IO errors take? If it can't be done
quickly enough the retry logic can be kept around to keep the old
behavior but that already was a broken behavior, so... :-(
I'm not sure how long will it take.
Anyway, as you said, the flush error handling of dm-mpath is already
broken if data loss really happens on any storage used by dm-mpath.
Although it's a serious issue and quick fix is required, I think
you may leave the old behavior in your patch-set, since it's
a separate issue.
Thanks,
Kiyoshi Ueda
On Mon, Aug 23, 2010 at 04:01:15PM +0200, Jens Axboe wrote:
quoted
The problem purely exists on arrays that report write back cache enabled
AND don't implement SYNC_CACHE as a noop. Do any of them exist, or are
they purely urban legend?
I haven't seen it. I don't care particularly about this case, but once
it a while people want to disable flushing for testing or because they
really don't care.
What about adding a sysfs attribue to every request_queue that allows
disabling the cache flushing feature? Compared to the barrier option
this controls the feature at the right level and makes it available
to everyone instead of beeing duplicated. After a while we can then
simply ignore the barrier/nobarrier options.