| Message ID | 20170428073250.9579-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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.
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
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.
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; - } /*
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(-)