Re: [PATCH 2/7] block: Remove bio->bi_ioc

From: Jens Axboe <axboe@kernel.dk>
To: Damien Le Moal <Damien.LeMoal@wdc.com>, Ming Lei <ming.lei@redhat.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Adam Manzanares <Adam.Manzanares@wdc.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 2/7] block: Remove bio->bi_ioc
Date: Tue, 20 Nov 2018 19:10:57 -0700	[thread overview]
Message-ID: <68806243-0094-5cbb-39fe-cea697f40b1c@kernel.dk> (raw)
In-Reply-To: <BN3PR0401MB164000EE62FE6BFF2ABE0069E7D90@BN3PR0401MB1640.namprd04.prod.outlook.com>

On 11/20/18 4:58 PM, Damien Le Moal wrote:
> On 2018/11/21 2:31, Jens Axboe wrote:
>> I think the below should fix it, we haven't necessarily setup an
>> ioc if we're just doing as passthrough request.
>>
>>
>> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
>> index 13b8dc332541..f096d8989773 100644
>> --- a/block/blk-mq-sched.c
>> +++ b/block/blk-mq-sched.c
>> @@ -34,9 +34,16 @@ EXPORT_SYMBOL_GPL(blk_mq_sched_free_hctx_data);
>>  void blk_mq_sched_assign_ioc(struct request *rq)
>>  {
>>  	struct request_queue *q = rq->q;
>> -	struct io_context *ioc = current->io_context;
>> +	struct io_context *ioc;
>>  	struct io_cq *icq;
>>  
>> +	/*
>> +	 * May not have an IO context if it's a passthrough request
>> +	 */
>> +	ioc = current->io_context;
>> +	if (!ioc)
>> +		return;
>> +
>>  	spin_lock_irq(&q->queue_lock);
>>  	icq = ioc_lookup_icq(ioc, q);
>>  	spin_unlock_irq(&q->queue_lock);
> 
> This seems reasonable to me, but I wonder why this problem was not triggering
> before. The previous code getting the ioc with the rq_ioc(bio) call was
> essentially the same and there was no "if (!ioc) return;" in
> blk_mq_sched_assign_ioc() before the patch.
> Any idea why this is popping up now ?
> 
> Ming,
> 
> Is this a new test your are running ? If this same problem triggers on stable
> kernels, Jens patch needs to go to stable too.

No, it's definitely introduced in your patches:

-                       if (e->type->icq_cache && rq_ioc(bio))
-                               blk_mq_sched_assign_ioc(rq, bio);
+                       if (e->type->icq_cache)
+                               blk_mq_sched_assign_ioc(rq);

Please run blktests on a series. Always. There's no excuse not to.

-- 
Jens Axboe