Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH V7 2/4] blk-mq: fix issue directly case when q is stopped
 or quiesced
To:     Ming Lei <ming.lei@redhat.com>
Cc:     axboe@kernel.dk, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1542185131-15029-1-git-send-email-jianchao.w.wang@oracle.com>
 <1542185131-15029-3-git-send-email-jianchao.w.wang@oracle.com>
 <20181114092022.GC20550@ming.t460p>
From:   "jianchao.wang" <jianchao.w.wang@oracle.com>
Message-ID: <6b29fb1a-31bc-ac3e-cdbd-80b2a9c95e11@oracle.com>
Date:   Wed, 14 Nov 2018 17:29:54 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181114092022.GC20550@ming.t460p>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

Thanks
Jianchao

> 
> thanks,
> Ming
>