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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 15 Nov 2018 09:37:39 +0800
From:   "jianchao.wang" <jianchao.w.wang@...cle.com>
To:     Ming Lei <ming.lei@...hat.com>
Cc:     axboe@...nel.dk, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V7 2/4] blk-mq: fix issue directly case when q is stopped
 or quiesced



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