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
