[PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Ming Lei]

We have freezed queue already, not necessary to call
blk_mq_quiesce_queue() any more, so remove it.

Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/elevator.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/elevator.c b/block/elevator.c
index bf11e70f008b..c7a4ee682033 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -950,7 +950,6 @@ static int elevator_switch_mq(struct request_queue *q,
 	int ret;
 
 	blk_mq_freeze_queue(q);
-	blk_mq_quiesce_queue(q);
 
 	if (q->elevator) {
 		if (q->elevator->registered)
@@ -978,9 +977,7 @@ static int elevator_switch_mq(struct request_queue *q,
 
 out:
 	blk_mq_unfreeze_queue(q);
-	blk_mq_start_stopped_hw_queues(q, true);
 	return ret;
-
 }
 
 /*
-- 
2.9.3

Re: [PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Jens Axboe]

On 04/28/2017 01:32 AM, Ming Lei wrote:
> We have freezed queue already, not necessary to call
> blk_mq_quiesce_queue() any more, so remove it.

Are you sure? It ensures that we also aren't in the middle of
blk_mq_make_request(), we need a stable view of the sched
status throughout that.

Similarly with updating the request maps.

-- 
Jens Axboe

Re: [PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Ming Lei]

Hi Jens,

On Fri, Apr 28, 2017 at 07:42:05AM -0600, Jens Axboe wrote:
> On 04/28/2017 01:32 AM, Ming Lei wrote:
> > We have freezed queue already, not necessary to call
> > blk_mq_quiesce_queue() any more, so remove it.
> 
> Are you sure? It ensures that we also aren't in the middle of
> blk_mq_make_request(), we need a stable view of the sched
> status throughout that.

After blk_mq_freeze_queue() returned, no requests belonging to this queue
can be in use, so there shouldn't be any .queue_rq() running for this
queue.

Or you mean any .queue_rq()(even not belongs to this queue) can't be
running during mq scheduler switch? If yes, per-hctx srcu can't guarantee
that too for BLOCKING case.

Thanks,
Ming

Re: [PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Bart Van Assche]

On Fri, 2017-04-28 at 07:42 -0600, Jens Axboe wrote:
> On 04/28/2017 01:32 AM, Ming Lei wrote:
> > We have freezed queue already, not necessary to call
> > blk_mq_quiesce_queue() any more, so remove it.
>=20
> Are you sure? It ensures that we also aren't in the middle of
> blk_mq_make_request(), we need a stable view of the sched
> status throughout that.

Hello Jens,

My understanding is that blk_mq_freeze_queue() provides stronger guarantees
than=A0blk_mq_quiesce_queue(). The former waits until all pending requests =
have
finished while the latter only waits until pending .queue_rq() calls have
finished. blk_mq_freeze_queue() also causes new blk_get_request() calls to
wait until blk_mq_unfreeze_queue() is called while=A0blk_get_request() can
still succeed after blk_mq_quiesce_queue() returned and before
blk_mq_start_stopped_hw_queues() is called.

Regarding blk_mq_make_request(): I think that the blk_queue_enter() call in
generic_make_request() prevents that blk_mq_make_request() gets called afte=
r
a queue has been frozen.

Bart.=

Re: [PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Ming Lei]

On Fri, Apr 28, 2017 at 03:59:20PM +0000, Bart Van Assche wrote:
> On Fri, 2017-04-28 at 07:42 -0600, Jens Axboe wrote:
> > On 04/28/2017 01:32 AM, Ming Lei wrote:
> > > We have freezed queue already, not necessary to call
> > > blk_mq_quiesce_queue() any more, so remove it.
> > 
> > Are you sure? It ensures that we also aren't in the middle of
> > blk_mq_make_request(), we need a stable view of the sched
> > status throughout that.
> 
> Hello Jens,
> 
> My understanding is that blk_mq_freeze_queue() provides stronger guarantees
> than�blk_mq_quiesce_queue(). The former waits until all pending requests have
> finished while the latter only waits until pending .queue_rq() calls have
> finished. blk_mq_freeze_queue() also causes new blk_get_request() calls to
> wait until blk_mq_unfreeze_queue() is called while�blk_get_request() can
> still succeed after blk_mq_quiesce_queue() returned and before
> blk_mq_start_stopped_hw_queues() is called.
> 
> Regarding blk_mq_make_request(): I think that the blk_queue_enter() call in
> generic_make_request() prevents that blk_mq_make_request() gets called after
> a queue has been frozen.

Jens & Bart, so I understand you don't object to this patch any more,
then I will post a v1 for covering blk_mq_update_nr_requests().

Thanks,
Ming

Re: [PATCH ] block: don't call blk_mq_quiesce_queue() during switching mq sched

[Jens Axboe]

On 04/28/2017 09:59 AM, Bart Van Assche wrote:
> On Fri, 2017-04-28 at 07:42 -0600, Jens Axboe wrote:
>> On 04/28/2017 01:32 AM, Ming Lei wrote:
>>> We have freezed queue already, not necessary to call
>>> blk_mq_quiesce_queue() any more, so remove it.
>>
>> Are you sure? It ensures that we also aren't in the middle of
>> blk_mq_make_request(), we need a stable view of the sched
>> status throughout that.
> 
> Hello Jens,
> 
> My understanding is that blk_mq_freeze_queue() provides stronger
> guarantees than blk_mq_quiesce_queue(). The former waits until all
> pending requests have finished while the latter only waits until
> pending .queue_rq() calls have finished. blk_mq_freeze_queue() also
> causes new blk_get_request() calls to wait until
> blk_mq_unfreeze_queue() is called while blk_get_request() can still
> succeed after blk_mq_quiesce_queue() returned and before
> blk_mq_start_stopped_hw_queues() is called.
> 
> Regarding blk_mq_make_request(): I think that the blk_queue_enter()
> call in generic_make_request() prevents that blk_mq_make_request()
> gets called after a queue has been frozen.

Bart, you are right, I'm fine with the patch.

-- 
Jens Axboe