Re: [PATCH V7 2/4] blk-mq: fix issue directly case when q is stopped or quiesced

[jianchao.wang]

On 11/14/18 5:35 PM, Ming Lei wrote:
> On Wed, Nov 14, 2018 at 05:29:54PM +0800, jianchao.wang wrote:
>> Hi Ming
>>
>> On 11/14/18 5:20 PM, Ming Lei wrote:
>>> On Wed, Nov 14, 2018 at 04:45:29PM +0800, Jianchao Wang wrote:
>>>> When try to issue request directly, if the queue is stopped or
>>>> quiesced, 'bypass' will be ignored and return BLK_STS_OK to caller
>>>> to avoid it dispatch request again. Then the request will be
>>>> inserted with blk_mq_sched_insert_request. This is not correct
>>>> for dm-rq case where we should avoid to pass through the underlying
>>>> path's io scheduler.
>>>>
>>>> To fix it, use blk_mq_request_bypass_insert to insert the request
>>>> to hctx->dispatch when we cannot pass through io scheduler but have
>>>> to insert.
>>>
>>> Not sure if the current behaviour is wrong, or worth of a fix.
>>>
>>> Bypassing io scheduler for dm-rq is only for sake of performance
>>> because there has been io scheduler for dm device already, and we
>>> just don't want to schedule these requests twice.
>>
>> As comment of commit 157f377beb710e84bd8bc7a3c4475c0674ebebd7
>> (block: directly insert blk-mq request from blk_insert_cloned_request())
>>
>>     All said, a request-based DM multipath device's IO scheduler should be
>>     the only one used -- when the original requests are issued to the
>>     underlying paths as cloned requests they are inserted directly in the
>>     underlying dispatch queue(s) rather than through an additional elevator.
>>     
>>     But commit bd166ef18 ("blk-mq-sched: add framework for MQ capable IO
>>     schedulers") switched blk_insert_cloned_request() from using
>>     blk_mq_insert_request() to blk_mq_sched_insert_request().  Which
>>     incorrectly added elevator machinery into a call chain that isn't
>>     supposed to have any.
>>
>> It sounds like a wrong action.
> 
> As I mentioned, it is only for the sake of performance, and IO scheduler
> has to be supported on these devices too, for example, one partition may
> be under dm-rq, and another partition can be accessed directly.
> 
> However, you are fixing the handling when queue is quiesced or stopped.
> Under this situation, it is fine to put requests into scheduler queue,
> given no performance need to be worried.
> 

OK, I drop this one.

Thanks
Jianchao