Re: [PATCH v3 8/9] block: mq-deadline: Handle requeued requests correctly

[Bart Van Assche <bvanassche@xxxxxxx>]

On 4/27/23 22:54, Christoph Hellwig wrote:
> On Mon, Apr 24, 2023 at 01:33:28PM -0700, Bart Van Assche wrote:
> > @@ -821,7 +833,16 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
> >   		 * set expire time and add to fifo list
> >   		 */
> >   		rq->fifo_time = jiffies + dd->fifo_expire[data_dir];
> > -		list_add_tail(&rq->queuelist, &per_prio->fifo_list[data_dir]);
> > +		insert_before = &per_prio->fifo_list[data_dir];
> > +#ifdef CONFIG_BLK_DEV_ZONED
> > +		if (blk_rq_is_seq_zoned_write(rq)) {
> > +			struct request *rq2 = deadline_latter_request(rq);
> > +
> > +			if (rq2 && blk_rq_zone_no(rq2) == blk_rq_zone_no(rq))
> > +				insert_before = &rq2->queuelist;
> > +		}
> > +#endif
>
> Why does this need an ifdef?

Because the blk_rq_zone_no() definition is surrounded with #ifdef 
CONFIG_BLK_DEV_ZONED / #endif. Without the #ifdef the above code would 
trigger a compilation error with CONFIG_BLK_DEV_ZONED=n. I have 
considered to add a definition of blk_rq_zone_no() for 
CONFIG_BLK_DEV_ZONED=n. I prefer not to do this because I think it's 
better to cause a compiler error if blk_rq_zone_no() is used in code 
that is also used for the CONFIG_BLK_DEV_ZONED=n case.

> Also can you please always add comments for these special cases?

Will do.

Thanks,

Bart.