From 116a4afcc35ec45d3e05113c710df4e65800f3cb Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Sun, 12 Jan 2020 17:46:58 -0500 Subject: [PATCH] bdevperf: Unregister pollers immediately when target exits with error _target_gone() and end_target() do almost same operations. We had not unregistered pollers at the cases that sets target->is_draining to true. The later had hit assert when the test ended with I/O error before completion because when target->run_timer is expired, end_target() would be called after target is freed. Hence we do both in this patch. Factor out the operations from _target_gone() and end_target() into a helper function. Then, when the target exits with error, unregister pollers of the target immediately by replacing setting target->is_draining to true by _end_target(target). We had seen the assert for a single case but apply the fix for all cases. Fixes issue #1140 Signed-off-by: Shuhei Matsumoto Change-Id: If39779817ed99c9441e368c1d4512c66d13d3378 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479911 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki --- test/bdev/bdevperf/bdevperf.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/test/bdev/bdevperf/bdevperf.c b/test/bdev/bdevperf/bdevperf.c index 9e1f788af..5f23d35da 100644 --- a/test/bdev/bdevperf/bdevperf.c +++ b/test/bdev/bdevperf/bdevperf.c @@ -290,10 +290,8 @@ bdevperf_free_targets(void) } static void -_target_gone(void *arg1, void *arg2) +_end_target(struct io_target *target) { - struct io_target *target = arg1; - spdk_poller_unregister(&target->run_timer); if (g_reset) { spdk_poller_unregister(&target->reset_timer); @@ -302,6 +300,14 @@ _target_gone(void *arg1, void *arg2) target->is_draining = true; } +static void +_target_gone(void *arg1, void *arg2) +{ + struct io_target *target = arg1; + + _end_target(target); +} + static void bdevperf_target_gone(void *arg) { @@ -517,7 +523,7 @@ bdevperf_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) if (!success) { if (!g_reset && !g_continue_on_failure) { - target->is_draining = true; + _end_target(target); g_run_rc = -1; printf("task offset: %lu on target bdev=%s fails\n", task->offset_blocks, target->name); @@ -532,7 +538,7 @@ bdevperf_complete(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) spdk_bdev_get_md_size(target->bdev), target->io_size_blocks, md_check)) { printf("Buffer mismatch! Disk Offset: %lu\n", task->offset_blocks); - target->is_draining = true; + _end_target(target); g_run_rc = -1; } } @@ -587,7 +593,7 @@ bdevperf_verify_submit_read(void *cb_arg) bdevperf_queue_io_wait_with_cb(task, bdevperf_verify_submit_read); } else if (rc != 0) { printf("Failed to submit read: %d\n", rc); - target->is_draining = true; + _end_target(target); g_run_rc = rc; } } @@ -733,7 +739,7 @@ bdevperf_submit_task(void *arg) return; } else if (rc != 0) { printf("Failed to submit bdev_io: %d\n", rc); - target->is_draining = true; + _end_target(target); g_run_rc = rc; return; } @@ -750,7 +756,7 @@ bdevperf_zcopy_get_buf_complete(struct spdk_bdev_io *bdev_io, bool success, void int iovcnt; if (!success) { - target->is_draining = true; + _end_target(target); g_run_rc = -1; return; } @@ -881,12 +887,7 @@ end_target(void *arg) { struct io_target *target = arg; - spdk_poller_unregister(&target->run_timer); - if (g_reset) { - spdk_poller_unregister(&target->reset_timer); - } - - target->is_draining = true; + _end_target(target); return -1; } @@ -901,7 +902,7 @@ reset_cb(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) if (!success) { printf("Reset blockdev=%s failed\n", spdk_bdev_get_name(target->bdev)); - target->is_draining = true; + _end_target(target); g_run_rc = -1; } @@ -927,7 +928,7 @@ reset_target(void *arg) reset_cb, task); if (rc) { printf("Reset failed: %d\n", rc); - target->is_draining = true; + _end_target(target); g_run_rc = -1; }