On 2020/06/02 18:32, Ming Lei wrote:
> On Tue, Jun 02, 2020 at 09:25:01AM +0000, Damien Le Moal wrote:
>> On 2020/06/02 18:15, Ming Lei wrote:
>>> All requests in the 'list' of blk_mq_dispatch_rq_list belong to same
>>> hctx, so it is better to pass hctx instead of request queue, because
>>> blk-mq's dispatch target is hctx instead of request queue.
>>>
>>> Cc: Sagi Grimberg <sagi@xxxxxxxxxxx>
>>> Cc: Baolin Wang <baolin.wang7@xxxxxxxxx>
>>> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
>>> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>>> Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
>>> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
>>> Tested-by: Baolin Wang <baolin.wang7@xxxxxxxxx>
>>> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
>>> ---
>>> block/blk-mq-sched.c | 14 ++++++--------
>>> block/blk-mq.c | 6 +++---
>>> block/blk-mq.h | 2 +-
>>> 3 files changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
>>> index a31e281e9d31..632c6f8b63f7 100644
>>> --- a/block/blk-mq-sched.c
>>> +++ b/block/blk-mq-sched.c
>>> @@ -96,10 +96,9 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
>>> struct elevator_queue *e = q->elevator;
>>> LIST_HEAD(rq_list);
>>> int ret = 0;
>>> + struct request *rq;
>>>
>>> do {
>>> - struct request *rq;
>>> -
>>> if (e->type->ops.has_work && !e->type->ops.has_work(hctx))
>>> break;
>>>
>>> @@ -131,7 +130,7 @@ static int blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx)
>>> * in blk_mq_dispatch_rq_list().
>>> */
>>> list_add(&rq->queuelist, &rq_list);
>>> - } while (blk_mq_dispatch_rq_list(q, &rq_list, true));
>>> + } while (blk_mq_dispatch_rq_list(rq->mq_hctx, &rq_list, true));
>>
>> Why not use the hctx argument passed to the function instead of rq->mq_hctx ?
>
> e->type->ops.dispatch_request(hctx) may return one request which's
> .mq_hctx isn't same with the 'hctx' argument, so far bfq and deadline
> may do that.
Ah, OK. But then all requests in rq_list may have different hctx. So is it wise
to pass hctx as an argument to blk_mq_dispatch_rq_list() ? The loop in that
function will still need to look at each rq hctx (hctx = rq->mq_hctx) for the
budget. So the hctx argument may not be needed at all, no ? Am I missing something ?
>
>
> Thanks,
> Ming
>
>
--
Damien Le Moal
Western Digital Research
