On Tue, Nov 26, 2019 at 12:27:50PM +0100, Hannes Reinecke wrote:
> On 11/26/19 12:05 PM, Ming Lei wrote:
> > On Tue, Nov 26, 2019 at 10:14:12AM +0100, Hannes Reinecke wrote:
> [ .. ]
> >> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> >> index ca22afd47b3d..f3589f42b96d 100644
> >> --- a/block/blk-mq-sched.c
> >> +++ b/block/blk-mq-sched.c
> >> @@ -452,7 +452,7 @@ static void blk_mq_sched_free_tags(struct blk_mq_tag_set *set,
> >> {
> >> if (hctx->sched_tags) {
> >> blk_mq_free_rqs(set, hctx->sched_tags, hctx_idx);
> >> - blk_mq_free_rq_map(hctx->sched_tags);
> >> + blk_mq_free_rq_map(hctx->sched_tags, false);
> >> hctx->sched_tags = NULL;
> >> }
> >> }
> >> @@ -462,10 +462,14 @@ static int blk_mq_sched_alloc_tags(struct request_queue *q,
> >> unsigned int hctx_idx)
> >> {
> >> struct blk_mq_tag_set *set = q->tag_set;
> >> + int flags = set->flags;
> >> int ret;
> >>
> >> + /* Scheduler tags are never shared */
> >> + set->flags &= ~BLK_MQ_F_TAG_HCTX_SHARED;
> >> hctx->sched_tags = blk_mq_alloc_rq_map(set, hctx_idx, q->nr_requests,
> >> set->reserved_tags);
> >> + set->flags = flags;
> >
> > This way is very fragile, race is made against other uses of
> > blk_mq_is_sbitmap_shared().
> >
> We are allocating tags, I don't think we're even able to modify it at
> this point.
Sched tags is allocated when setting up scheduler, which can be done
anytime from writing to queue/scheduler.
>
> > From performance viewpoint, all hctx belonging to this request queue should
> > share one scheduler tagset in case of BLK_MQ_F_TAG_HCTX_SHARED, cause
> > driver tag queue depth isn't changed.
> >
> Hmm. Now you get me confused.
> In an earlier mail you said:
>
> > This kind of sharing is wrong, sched tags should be request
> > queue wide instead of tagset wide, and each request queue has
> > its own & independent scheduler queue.
>
> as in v2 we _had_ shared scheduler tags, too.
> Did I misread your comment above?
Yes, what I meant is that we can't share sched tags in tagset wide.
Now I mean we should share sched tags among all hctxs in same request
queue, and I believe I have described it clearly.
Thanks,
Ming
