diff options
author | Yufen Yu <yuyufen@huawei.com> | 2019-04-24 11:19:05 -0400 |
---|---|---|
committer | Mike Snitzer <snitzer@redhat.com> | 2019-04-25 15:38:52 -0400 |
commit | 5de719e3d01b4abe0de0d7b857148a880ff2a90b (patch) | |
tree | 88cacc02c0a64090ce907c8544f6e7e1d0b13421 | |
parent | 873f258becca87f4dd973fe0ba09b88b737c9b14 (diff) |
dm mpath: fix missing call of path selector type->end_io
After commit 396eaf21ee17 ("blk-mq: improve DM's blk-mq IO merging via
blk_insert_cloned_request feedback"), map_request() will requeue the tio
when issued clone request return BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE.
Thus, if device driver status is error, a tio may be requeued multiple
times until the return value is not DM_MAPIO_REQUEUE. That means
type->start_io may be called multiple times, while type->end_io is only
called when IO complete.
In fact, even without commit 396eaf21ee17, setup_clone() failure can
also cause tio requeue and associated missed call to type->end_io.
The service-time path selector selects path based on in_flight_size,
which is increased by st_start_io() and decreased by st_end_io().
Missed calls to st_end_io() can lead to in_flight_size count error and
will cause the selector to make the wrong choice. In addition,
queue-length path selector will also be affected.
To fix the problem, call type->end_io in ->release_clone_rq before tio
requeue. map_info is passed to ->release_clone_rq() for map_request()
error path that result in requeue.
Fixes: 396eaf21ee17 ("blk-mq: improve DM's blk-mq IO merging via blk_insert_cloned_request feedback")
Cc: stable@vger.kernl.org
Signed-off-by: Yufen Yu <yuyufen@huawei.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
-rw-r--r-- | drivers/md/dm-mpath.c | 17 | ||||
-rw-r--r-- | drivers/md/dm-rq.c | 8 | ||||
-rw-r--r-- | drivers/md/dm-target.c | 3 | ||||
-rw-r--r-- | include/linux/device-mapper.h | 3 |
4 files changed, 24 insertions, 7 deletions
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 2ee5e357a0a7..07cfef67c0ba 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c | |||
@@ -544,8 +544,23 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq, | |||
544 | return DM_MAPIO_REMAPPED; | 544 | return DM_MAPIO_REMAPPED; |
545 | } | 545 | } |
546 | 546 | ||
547 | static void multipath_release_clone(struct request *clone) | 547 | static void multipath_release_clone(struct request *clone, |
548 | union map_info *map_context) | ||
548 | { | 549 | { |
550 | if (unlikely(map_context)) { | ||
551 | /* | ||
552 | * non-NULL map_context means caller is still map | ||
553 | * method; must undo multipath_clone_and_map() | ||
554 | */ | ||
555 | struct dm_mpath_io *mpio = get_mpio(map_context); | ||
556 | struct pgpath *pgpath = mpio->pgpath; | ||
557 | |||
558 | if (pgpath && pgpath->pg->ps.type->end_io) | ||
559 | pgpath->pg->ps.type->end_io(&pgpath->pg->ps, | ||
560 | &pgpath->path, | ||
561 | mpio->nr_bytes); | ||
562 | } | ||
563 | |||
549 | blk_put_request(clone); | 564 | blk_put_request(clone); |
550 | } | 565 | } |
551 | 566 | ||
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c index b66745bd08bb..5f7063f05ae0 100644 --- a/drivers/md/dm-rq.c +++ b/drivers/md/dm-rq.c | |||
@@ -168,7 +168,7 @@ static void dm_end_request(struct request *clone, blk_status_t error) | |||
168 | struct request *rq = tio->orig; | 168 | struct request *rq = tio->orig; |
169 | 169 | ||
170 | blk_rq_unprep_clone(clone); | 170 | blk_rq_unprep_clone(clone); |
171 | tio->ti->type->release_clone_rq(clone); | 171 | tio->ti->type->release_clone_rq(clone, NULL); |
172 | 172 | ||
173 | rq_end_stats(md, rq); | 173 | rq_end_stats(md, rq); |
174 | blk_mq_end_request(rq, error); | 174 | blk_mq_end_request(rq, error); |
@@ -201,7 +201,7 @@ static void dm_requeue_original_request(struct dm_rq_target_io *tio, bool delay_ | |||
201 | rq_end_stats(md, rq); | 201 | rq_end_stats(md, rq); |
202 | if (tio->clone) { | 202 | if (tio->clone) { |
203 | blk_rq_unprep_clone(tio->clone); | 203 | blk_rq_unprep_clone(tio->clone); |
204 | tio->ti->type->release_clone_rq(tio->clone); | 204 | tio->ti->type->release_clone_rq(tio->clone, NULL); |
205 | } | 205 | } |
206 | 206 | ||
207 | dm_mq_delay_requeue_request(rq, delay_ms); | 207 | dm_mq_delay_requeue_request(rq, delay_ms); |
@@ -398,7 +398,7 @@ static int map_request(struct dm_rq_target_io *tio) | |||
398 | case DM_MAPIO_REMAPPED: | 398 | case DM_MAPIO_REMAPPED: |
399 | if (setup_clone(clone, rq, tio, GFP_ATOMIC)) { | 399 | if (setup_clone(clone, rq, tio, GFP_ATOMIC)) { |
400 | /* -ENOMEM */ | 400 | /* -ENOMEM */ |
401 | ti->type->release_clone_rq(clone); | 401 | ti->type->release_clone_rq(clone, &tio->info); |
402 | return DM_MAPIO_REQUEUE; | 402 | return DM_MAPIO_REQUEUE; |
403 | } | 403 | } |
404 | 404 | ||
@@ -408,7 +408,7 @@ static int map_request(struct dm_rq_target_io *tio) | |||
408 | ret = dm_dispatch_clone_request(clone, rq); | 408 | ret = dm_dispatch_clone_request(clone, rq); |
409 | if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE) { | 409 | if (ret == BLK_STS_RESOURCE || ret == BLK_STS_DEV_RESOURCE) { |
410 | blk_rq_unprep_clone(clone); | 410 | blk_rq_unprep_clone(clone); |
411 | tio->ti->type->release_clone_rq(clone); | 411 | tio->ti->type->release_clone_rq(clone, &tio->info); |
412 | tio->clone = NULL; | 412 | tio->clone = NULL; |
413 | return DM_MAPIO_REQUEUE; | 413 | return DM_MAPIO_REQUEUE; |
414 | } | 414 | } |
diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c index 314d17ca6466..64dd0b34fcf4 100644 --- a/drivers/md/dm-target.c +++ b/drivers/md/dm-target.c | |||
@@ -136,7 +136,8 @@ static int io_err_clone_and_map_rq(struct dm_target *ti, struct request *rq, | |||
136 | return DM_MAPIO_KILL; | 136 | return DM_MAPIO_KILL; |
137 | } | 137 | } |
138 | 138 | ||
139 | static void io_err_release_clone_rq(struct request *clone) | 139 | static void io_err_release_clone_rq(struct request *clone, |
140 | union map_info *map_context) | ||
140 | { | 141 | { |
141 | } | 142 | } |
142 | 143 | ||
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index b0672756d056..e1f51d607cc5 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h | |||
@@ -62,7 +62,8 @@ typedef int (*dm_clone_and_map_request_fn) (struct dm_target *ti, | |||
62 | struct request *rq, | 62 | struct request *rq, |
63 | union map_info *map_context, | 63 | union map_info *map_context, |
64 | struct request **clone); | 64 | struct request **clone); |
65 | typedef void (*dm_release_clone_request_fn) (struct request *clone); | 65 | typedef void (*dm_release_clone_request_fn) (struct request *clone, |
66 | union map_info *map_context); | ||
66 | 67 | ||
67 | /* | 68 | /* |
68 | * Returns: | 69 | * Returns: |