Hello,
On Fri, Aug 12, 2011 at 03:07:51PM -0400, Jeff Moyer wrote:
> Changes from v1->v2:
> - Moved the detection of empty flush requests into blk_insert_flush.
> - Got rid of REQ_FLUSH_SEQ in the CLONE_FLAGS.
Heh yeah, this looks pretty good to me. :)
> @@ -312,6 +309,19 @@ void blk_insert_flush(struct request *rq)
> rq->cmd_flags &= ~REQ_FUA;
>
> /*
> + * An empty flush handed down from a stacking driver may
> + * translate into nothing if the underlying device does not
> + * advertise a write-back cache. In this case, simply
> + * complete the request.
> + */
> + if (!policy && !blk_rq_bytes(rq)) {
> + __blk_end_bidi_request(rq, 0, 0, 0);
> + return;
> + }
Hmmm... doesn't !policy imply !blk_rq_bytes() with your change just
merged to Jens' tree?
> @@ -319,6 +329,7 @@ void blk_insert_flush(struct request *rq)
> if ((policy & REQ_FSEQ_DATA) &&
> !(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) {
> list_add_tail(&rq->queuelist, &q->queue_head);
> + blk_run_queue_async(q);
> return;
> }
In the other message, you said,
> Well, the only time we need to run the queue is when the request has
> data, has REQ_FUA set, and the underlying queue's flush flags contain
> only REQ_FUA. In code:
>
> if (rq->cmd_flags & REQ_FUA && q->flush_flags == REQ_FUA)
> blk_run_queue_async(q);
But this can't happen because a queue can't have REQ_FUA without
REQ_FLUSH (it doesn't make any sense). blk_queue_flush() will trigger
WARN_ON_ONCE() and turn off REQ_FUA in such cases.
That said, it's kinda unclear who should be responsible for kicking
the queue. __elv_add_request() does it for some but not all.
__make_request() always activates the queue which sometimes ends up
doing it again after __elv_add_request(). I think kicking the queue
after short circuit insert probably is the right thing to do.
Thank you.
--
tejun
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
