On 12/6/18 7:16 PM, Jens Axboe wrote:
> On 12/6/18 7:06 PM, Jens Axboe wrote:
>> On 12/6/18 6:58 PM, Mike Snitzer wrote:
>>>> There is another way to fix this - still do the direct dispatch, but have
>>>> dm track if it failed and do bypass insert in that case. I didn't want do
>>>> to that since it's more involved, but it's doable.
>>>>
>>>> Let me cook that up and test it... Don't like it, though.
>>>
>>> Not following how DM can track if issuing the request worked if it is
>>> always told it worked with BLK_STS_OK. We care about feedback when the
>>> request is actually issued because of the elaborate way blk-mq elevators
>>> work. DM is forced to worry about all these details, as covered some in
>>> the header for commit 396eaf21ee17c476e8f66249fb1f4a39003d0ab4, it is
>>> trying to have its cake and eat it too. It just wants IO scheduling to
>>> work for request-based DM devices. That's it.
>>
>> It needs the feedback, I don't disagree on that part at all. If we
>> always return OK, then that loop is broken. How about something like the
>> below? Totally untested right now...
>>
>> We track if a request ever saw BLK_STS_RESOURCE from direct dispatch,
>> and if it did, we store that information with RQF_DONTPREP. When we then
>> next time go an insert a request, if it has RQF_DONTPREP set, then we
>> ask blk_insert_cloned_request() to bypass insert.
>>
>> I'll go test this now.
>
> Passes the test case for me.
Here's one that doesn't re-arrange the return value check into a switch.
Turns out cleaner (and less LOC changes), and also doesn't fiddle with
request after freeing it if we got OK return...
Will give this a whirl too, just in case.
diff --git a/block/blk-core.c b/block/blk-core.c
index deb56932f8c4..cccda51e165f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2617,7 +2617,8 @@ static int blk_cloned_rq_check_limits(struct
request_queue *q,
* @q: the queue to submit the request
* @rq: the request being queued
*/
-blk_status_t blk_insert_cloned_request(struct request_queue *q, struct request
*rq)
+blk_status_t blk_insert_cloned_request(struct request_queue *q, struct request
*rq,
+ bool force_insert)
{
unsigned long flags;
int where = ELEVATOR_INSERT_BACK;
@@ -2637,7 +2638,11 @@ blk_status_t blk_insert_cloned_request(struct
request_queue *q, struct request *
* bypass a potential scheduler on the bottom device for
* insert.
*/
- return blk_mq_request_issue_directly(rq);
+ if (force_insert) {
+ blk_mq_request_bypass_insert(rq, true);
+ return BLK_STS_OK;
+ } else
+ return blk_mq_request_issue_directly(rq);
}
spin_lock_irqsave(q->queue_lock, flags);
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 7cd36e4d1310..e497a2ab6766 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -299,16 +299,20 @@ static void end_clone_request(struct request *clone,
blk_status_t error)
static blk_status_t dm_dispatch_clone_request(struct request *clone, struct
request *rq)
{
+ bool was_busy = (rq->rq_flags & RQF_DONTPREP) != 0;
blk_status_t r;
if (blk_queue_io_stat(clone->q))
clone->rq_flags |= RQF_IO_STAT;
clone->start_time_ns = ktime_get_ns();
- r = blk_insert_cloned_request(clone->q, clone);
- if (r != BLK_STS_OK && r != BLK_STS_RESOURCE && r !=
BLK_STS_DEV_RESOURCE)
+ r = blk_insert_cloned_request(clone->q, clone, was_busy);
+ if (r == BLK_STS_RESOURCE || r == BLK_STS_DEV_RESOURCE)
+ rq->rq_flags |= RQF_DONTPREP;
+ else if (r != BLK_STS_OK)
/* must complete clone in terms of original request */
dm_complete_request(rq, r);
+
return r;
}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4293dc1cd160..7cb84ee4c9f4 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -994,7 +994,7 @@ extern int blk_rq_prep_clone(struct request *rq, struct
request *rq_src,
void *data);
extern void blk_rq_unprep_clone(struct request *rq);
extern blk_status_t blk_insert_cloned_request(struct request_queue *q,
- struct request *rq);
+ struct request *rq, bool force_insert);
extern int blk_rq_append_bio(struct request *rq, struct bio **bio);
extern void blk_delay_queue(struct request_queue *, unsigned long);
extern void blk_queue_split(struct request_queue *, struct bio **);
--
Jens Axboe
