On 11/20/18 7:45 PM, Damien Le Moal wrote:
> On 2018/11/21 11:11, Jens Axboe wrote:
>> 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.
>
> By the way, should I send an updated patch 2 to include your fix ?
> Or will you add an incremental fix ?
I had to add the incremental fix, I already merged your patches
earlier. It's all pushed out now.
--
Jens Axboe