From: Martin Wilck <mwilck@xxxxxxxx>
io_cancel() never succeeds, and even if it did, io_getevents() must
still be called to reclaim the IO resources from the kernel.
Don't pretend the opposite by resetting ct->running, or freeing the
memory, before io_getevents() has indicated that the request is finished.
In the test code, don't bother about the return value of __wrap_io_cancel().
Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
---
libmultipath/checkers/directio.c | 14 ++++----------
tests/directio.c | 27 ++-------------------------
2 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 25b2383..83ab29f 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -232,15 +232,15 @@ void libcheck_free (struct checker * c)
}
}
- if (ct->running &&
- (ct->req->state != PATH_PENDING ||
- io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event) == 0))
+ if (ct->running && ct->req->state != PATH_PENDING)
ct->running = 0;
if (!ct->running) {
free(ct->req->buf);
free(ct->req);
ct->aio_grp->holders--;
} else {
+ /* Currently a no-op */
+ io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
ct->req->state = PATH_REMOVED;
list_add(&ct->req->node, &ct->aio_grp->orphans);
check_orphaned_group(ct->aio_grp);
@@ -351,13 +351,7 @@ check_state(int fd, struct directio_context *ct, int sync, int timeout_secs)
LOG(3, "abort check on timeout");
- r = io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
- /*
- * Only reset ct->running if we really
- * could abort the pending I/O
- */
- if (!r)
- ct->running = 0;
+ io_cancel(ct->aio_grp->ioctx, &ct->req->io, &event);
rc = PATH_DOWN;
} else {
LOG(4, "async io pending");
diff --git a/tests/directio.c b/tests/directio.c
index db9643e..5201d21 100644
--- a/tests/directio.c
+++ b/tests/directio.c
@@ -141,10 +141,9 @@ int __real_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt);
int __wrap_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt)
{
#ifdef DIO_TEST_DEV
- mock_type(int);
return __real_io_cancel(ctx, iocb, evt);
#else
- return mock_type(int);
+ return 0;
#endif
}
@@ -439,14 +438,8 @@ static void test_check_state_timeout(void **state)
do_libcheck_init(&c, 4096, NULL);
aio_grp = get_aio_grp(&c);
return_io_getevents_none();
- will_return(__wrap_io_cancel, 0);
do_check_state(&c, 1, 30, PATH_DOWN);
check_aio_grp(aio_grp, 1, 0);
-#ifdef DIO_TEST_DEV
- /* io_cancel will return negative value on timeout, so it happens again
- * when freeing the checker */
- will_return(__wrap_io_cancel, 0);
-#endif
libcheck_free(&c);
do_libcheck_reset(1);
}
@@ -468,12 +461,8 @@ static void test_check_state_async_timeout(void **state)
return_io_getevents_none();
do_check_state(&c, 0, 3, PATH_PENDING);
return_io_getevents_none();
- will_return(__wrap_io_cancel, 0);
do_check_state(&c, 0, 3, PATH_DOWN);
check_aio_grp(aio_grp, 1, 0);
-#ifdef DIO_TEST_DEV
- will_return(__wrap_io_cancel, 0);
-#endif
libcheck_free(&c);
do_libcheck_reset(1);
}
@@ -501,13 +490,8 @@ static void test_free_with_pending(void **state)
check_aio_grp(aio_grp, 2, 0);
libcheck_free(&c[0]);
check_aio_grp(aio_grp, 1, 0);
- will_return(__wrap_io_cancel, 0);
libcheck_free(&c[1]);
-#ifdef DIO_TEST_DEV
- check_aio_grp(aio_grp, 1, 1); /* real cancel doesn't remove request */
-#else
- check_aio_grp(aio_grp, 0, 0);
-#endif
+ check_aio_grp(aio_grp, 1, 1); /* cancel doesn't remove request */
do_libcheck_reset(1);
}
@@ -533,7 +517,6 @@ static void test_orphaned_aio_group(void **state)
assert_int_equal(i, 1);
for (i = 0; i < AIO_GROUP_SIZE; i++) {
assert_true(is_checker_running(&c[i]));
- will_return(__wrap_io_cancel, -1);
if (i == AIO_GROUP_SIZE - 1) {
/* remove the orphaned group and create a new one */
will_return(__wrap_io_destroy, 0);
@@ -559,12 +542,10 @@ static void test_timeout_cancel_failed(void **state)
do_libcheck_init(&c[i], 4096, &reqs[i]);
aio_grp = get_aio_grp(c);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 1, 30, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
check_aio_grp(aio_grp, 2, 0);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 1, 30, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
return_io_getevents_nr(NULL, 1, &reqs[0], &res[0]);
@@ -600,7 +581,6 @@ static void test_async_timeout_cancel_failed(void **state)
return_io_getevents_none();
do_check_state(&c[1], 0, 2, PATH_PENDING);
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 0, 2, PATH_DOWN);
#ifndef DIO_TEST_DEV
/* can't pick which even gets returned on real devices */
@@ -608,7 +588,6 @@ static void test_async_timeout_cancel_failed(void **state)
do_check_state(&c[1], 0, 2, PATH_UP);
#endif
return_io_getevents_none();
- will_return(__wrap_io_cancel, -1);
do_check_state(&c[0], 0, 2, PATH_DOWN);
assert_true(is_checker_running(&c[0]));
return_io_getevents_nr(NULL, 2, reqs, res);
@@ -637,7 +616,6 @@ static void test_orphan_checker_cleanup(void **state)
aio_grp = get_aio_grp(c);
return_io_getevents_none();
do_check_state(&c[0], 0, 30, PATH_PENDING);
- will_return(__wrap_io_cancel, -1);
check_aio_grp(aio_grp, 2, 0);
libcheck_free(&c[0]);
check_aio_grp(aio_grp, 2, 1);
@@ -662,7 +640,6 @@ static void test_orphan_reset_cleanup(void **state)
orphan_aio_grp = get_aio_grp(&c);
return_io_getevents_none();
do_check_state(&c, 0, 30, PATH_PENDING);
- will_return(__wrap_io_cancel, -1);
check_aio_grp(orphan_aio_grp, 1, 0);
libcheck_free(&c);
check_aio_grp(orphan_aio_grp, 1, 1);
--
2.42.0
