[PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[Ming Lei]

blk_mq_requeue_request() should kick off the requeue list so
that the request can be scheduled to queue finally.

In nvme conversion patch, blk_mq_requeue_request() is called
to requeue failed request from completion handler, this patch
fixes the issue and makes the block API working as expected.

Cc: Matias Bjørling <m@bjorling.me>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 block/blk-mq.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index cb4c785..4c78539 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -462,6 +462,7 @@ void blk_mq_requeue_request(struct request *rq)
 
 	BUG_ON(blk_queued_rq(rq));
 	blk_mq_add_to_requeue_list(rq, true);
+	blk_mq_kick_requeue_list(rq->q);
 }
 EXPORT_SYMBOL(blk_mq_requeue_request);
 
-- 
1.7.9.5

Re: [PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[Christoph Hellwig]

On Tue, Jun 10, 2014 at 12:52:05PM +0800, Ming Lei wrote:
> blk_mq_requeue_request() should kick off the requeue list so
> that the request can be scheduled to queue finally.
> 
> In nvme conversion patch, blk_mq_requeue_request() is called
> to requeue failed request from completion handler, this patch
> fixes the issue and makes the block API working as expected.

I've intentionally kepts the two separate so that we can batch operations
and just do a single kick for multiple requeued commands.

Re: [PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[Ming Lei]

On Tue, Jun 10, 2014 at 7:28 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Jun 10, 2014 at 12:52:05PM +0800, Ming Lei wrote:
>> blk_mq_requeue_request() should kick off the requeue list so
>> that the request can be scheduled to queue finally.
>>
>> In nvme conversion patch, blk_mq_requeue_request() is called
>> to requeue failed request from completion handler, this patch
>> fixes the issue and makes the block API working as expected.
>
> I've intentionally kepts the two separate so that we can batch operations
> and just do a single kick for multiple requeued commands.
>

OK, but the API name is a bit confusing, and that is easy to cause
misuse, like in nvme.

Also requeue one request in completion handler may be a
common case, so maybe it is better to let blk_mq_requeue_request()
handle both adding to request list and kicking off, and introduce
blk_mq_prep_requeue_req()(or better name) to batch operations.


Thanks,
-- 
Ming Lei

Re: [PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[Christoph Hellwig]

On Tue, Jun 10, 2014 at 07:45:47PM +0800, Ming Lei wrote:
> OK, but the API name is a bit confusing, and that is easy to cause
> misuse, like in nvme.
> 
> Also requeue one request in completion handler may be a
> common case, so maybe it is better to let blk_mq_requeue_request()
> handle both adding to request list and kicking off, and introduce
> blk_mq_prep_requeue_req()(or better name) to batch operations.

For now please just add some documentation.  I'm happy to revisit
these once we have a few user in tree, but for now I'd like to keep
blk-mq a little more stable to not impact the merges of the major
users.

Re: [PATCH] blk-mq: kick requeue list in blk_mq_requeue_request()

[Ming Lei]

On Tue, Jun 10, 2014 at 8:45 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Jun 10, 2014 at 07:45:47PM +0800, Ming Lei wrote:
>> OK, but the API name is a bit confusing, and that is easy to cause
>> misuse, like in nvme.
>>
>> Also requeue one request in completion handler may be a
>> common case, so maybe it is better to let blk_mq_requeue_request()
>> handle both adding to request list and kicking off, and introduce
>> blk_mq_prep_requeue_req()(or better name) to batch operations.
>
> For now please just add some documentation.  I'm happy to revisit
> these once we have a few user in tree, but for now I'd like to keep
> blk-mq a little more stable to not impact the merges of the major
> users.

There are two users: one is nvme, another is scsi-mq, and both
don't need batch operations now.


Thanks,
-- 
Ming Lei