| Message ID | 20230407235822.1672286-10-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Submit zoned writes in order | expand |
On 4/8/23 08:58, Bart Van Assche wrote: > Make sure that zoned writes are submitted in LBA order. > > Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Mike Snitzer <snitzer@kernel.org> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/mq-deadline.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/mq-deadline.c b/block/mq-deadline.c > index 50a9d3b0a291..891ee0da73ac 100644 > --- a/block/mq-deadline.c > +++ b/block/mq-deadline.c > @@ -798,7 +798,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, > > trace_block_rq_insert(rq); > > - if (at_head) { > + if (at_head && !blk_rq_is_seq_zoned_write(rq)) { > list_add(&rq->queuelist, &per_prio->dispatch); > rq->fifo_time = jiffies; > } else { Looks OK, but I would prefer us addressing the caller site using at_head = true, as that is almost always completely wrong for sequential zone writes. That would reduce the number of places we check for blk_rq_is_seq_zoned_write().
On 4/10/23 01:10, Damien Le Moal wrote: > On 4/8/23 08:58, Bart Van Assche wrote: >> Make sure that zoned writes are submitted in LBA order. >> >> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Ming Lei <ming.lei@redhat.com> >> Cc: Mike Snitzer <snitzer@kernel.org> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> block/mq-deadline.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/block/mq-deadline.c b/block/mq-deadline.c >> index 50a9d3b0a291..891ee0da73ac 100644 >> --- a/block/mq-deadline.c >> +++ b/block/mq-deadline.c >> @@ -798,7 +798,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, >> >> trace_block_rq_insert(rq); >> >> - if (at_head) { >> + if (at_head && !blk_rq_is_seq_zoned_write(rq)) { >> list_add(&rq->queuelist, &per_prio->dispatch); >> rq->fifo_time = jiffies; >> } else { > > Looks OK, but I would prefer us addressing the caller site using at_head = true, > as that is almost always completely wrong for sequential zone writes. > That would reduce the number of places we check for blk_rq_is_seq_zoned_write(). Hi Damien, The code for deciding whether or not to use head insertion is spread all over the block layer. I prefer a single additional check to disable head insertion instead of modifying all the code that decides whether or not to use head-insertion. Additionally, the call to blk_rq_is_seq_zoned_write() would remain if the decision whether or not to use head insertion is moved into the callers of elevator_type.insert_request. Thanks, Bart.
On Mon, Apr 10, 2023 at 10:09:40AM -0700, Bart Van Assche wrote: > The code for deciding whether or not to use head insertion is spread all > over the block layer. I prefer a single additional check to disable head > insertion instead of modifying all the code that decides whether or not to > use head-insertion. Additionally, the call to blk_rq_is_seq_zoned_write() > would remain if the decision whether or not to use head insertion is moved > into the callers of elevator_type.insert_request. I think it's time to do a proper audit and sort this out. at_head insertations make absolute sense for certain passthrough commands, so the path in from blk_execute_rq/blk_execute_rq_nowait is fine, and it should always be passed on, even for zoned devices. For everything else head insertations looks very questionable and I think we need to go through them and figure out if any of them should be kept.
diff --git a/block/mq-deadline.c b/block/mq-deadline.c index 50a9d3b0a291..891ee0da73ac 100644 --- a/block/mq-deadline.c +++ b/block/mq-deadline.c @@ -798,7 +798,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, trace_block_rq_insert(rq); - if (at_head) { + if (at_head && !blk_rq_is_seq_zoned_write(rq)) { list_add(&rq->queuelist, &per_prio->dispatch); rq->fifo_time = jiffies; } else {
Make sure that zoned writes are submitted in LBA order. Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Cc: Mike Snitzer <snitzer@kernel.org> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/mq-deadline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)