Re: [PATCH V2 11/20] blk-mq: introduce helpers for operating ->dispatch list

Damien Le Moal
2017-08-24T01:00:07Z

On 8/23/17 05:43, Bart Van Assche wrote:
> On Sat, 2017-08-05 at 14:56 +0800, Ming Lei wrote:
>> +static inline bool blk_mq_has_dispatch_rqs(struct blk_mq_hw_ctx *hctx)
>> +{
>> + return !list_empty_careful(&hctx->dispatch);
>> +}
>> +
>> +static inline void blk_mq_add_rq_to_dispatch(struct blk_mq_hw_ctx *hctx,
>> + struct request *rq)
>> +{
>> + spin_lock(&hctx->lock);
>> + list_add(&rq->queuelist, &hctx->dispatch);
>> + blk_mq_hctx_set_dispatch_busy(hctx);
>> + spin_unlock(&hctx->lock);
>> +}
>> +
>> +static inline void blk_mq_add_list_to_dispatch(struct blk_mq_hw_ctx *hctx,
>> + struct list_head *list)
>> +{
>> + spin_lock(&hctx->lock);
>> + list_splice_init(list, &hctx->dispatch);
>> + blk_mq_hctx_set_dispatch_busy(hctx);
>> + spin_unlock(&hctx->lock);
>> +}
>> +
>> +static inline void blk_mq_add_list_to_dispatch_tail(struct blk_mq_hw_ctx *hctx,
>> + struct list_head *list)
>> +{
>> + spin_lock(&hctx->lock);
>> + list_splice_tail_init(list, &hctx->dispatch);
>> + blk_mq_hctx_set_dispatch_busy(hctx);
>> + spin_unlock(&hctx->lock);
>> +}
>> +
>> +static inline void blk_mq_take_list_from_dispatch(struct blk_mq_hw_ctx *hctx,
>> + struct list_head *list)
>> +{
>> + spin_lock(&hctx->lock);
>> + list_splice_init(&hctx->dispatch, list);
>> + spin_unlock(&hctx->lock);
>> +}
>
> Same comment for this patch: these helper functions are so short that I'm not
> sure it is useful to introduce these helper functions.
>
> Bart.

Personally, I like those very much as they give a place to hook up
different dispatch_list handling without having to change blk-mq.c and
blk-mq-sched.c all over the place.

I am thinking of SMR (zoned block device) support here since we need to
to sort insert write requests in blk_mq_add_rq_to_dispatch() and
blk_mq_add_list_to_dispatch_tail(). For this last one, the name would
become a little awkward though. This sort insert would be to avoid
breaking a sequential write request sequence sent by the disk user. This
is needed since this reordering breakage cannot be solved only from the
SCSI layer.

blk_mq_add_list_to_dispatch() and blk_mq_add_list_to_dispatch_tail()
could be combined together into blk_mq_add_list_to_dispatch() with the
addition of a "where" argument (BLK_MQ_INSERT_HEAD or
BLK_MQ_INSERT_TAIL), or like some other functions in blk-mq, the
addition of a simple "bool at_head" argument.

Best regards.

--
Damien Le Moal,
Western Digital Research