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.
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..8d4c5020ccaa 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -299,16 +299,27 @@ 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);
+ switch (r) {
+ default:
/* must complete clone in terms of original request */
dm_complete_request(rq, r);
+ /* fall through */
+ case BLK_STS_RESOURCE:
+ case BLK_STS_DEV_RESOURCE:
+ rq->rq_flags |= RQF_DONTPREP;
+ case BLK_STS_OK:
+ break;
+ }
+
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
