Re: [PATCH 4/4] blk-mq: allocate tags in batches

[Jens Axboe <axboe@kernel.dk>]

On 12/30/19 7:18 PM, Ming Lei wrote:
>> +static int blk_mq_get_tag_batch(struct blk_mq_alloc_data *data)
>> +{
>> +	struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
>> +	struct sbitmap_queue *bt = &tags->bitmap_tags;
>> +	struct blk_mq_ctx *ctx = data->ctx;
>> +	int tag, cpu;
>> +
>> +	if (!ctx)
>> +		return -1;
>> +
>> +	preempt_disable();
>> +
>> +	/* bad luck if we got preempted coming in here, should be rare */
>> +	cpu = smp_processor_id();
>> +	if (unlikely(ctx->cpu != cpu)) {
>> +		ctx = data->ctx = __blk_mq_get_ctx(data->q, cpu);
>> +		data->hctx = blk_mq_map_queue(data->q, data->cmd_flags, ctx);
> 
> When blk_mq_get_tag_batch is called for getting driver tag, rq->mq_hctx
> has been bound to the current request in blk_mq_rq_ctx_init(), so looks
> we shouldn't change hctx here.

->ctx is also NULL for that case, so it gets skipped. Probably needs a
comment...

I'll need to check if all cases are covered, the batched tags should be
exclusive to the non-reserved, regular tags. Or just the scheduler tags,
if a scheduler is attached. Not sure it makes a lot of sense to handle
the scheduler case as well (and have two sets of batches, driver tags
and scheduler tags), but if it does, we could extend it for that and
just bail if we're not on the right ctx anymore. In all honesty, I wrote
that above code without ever checking that it happens. It most certainly
could, but I'd consider it a very low risk. So might be better to simply
just return -1 for that case and ignore it.

-- 
Jens Axboe