Re: [PATCH 4/8] blk-mq: Facilitate a shared sbitmap per tagset

[Ming Lei <ming.lei@redhat.com>]

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