On 10/26/23 01:14, Benjamin Marzinski wrote:
On Tue, Oct 24, 2023 at 06:49:33PM +0200, mwilck@xxxxxxxx wrote:
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.
Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
---
libmultipath/checkers/directio.c | 13 ++-----------
tests/directio.c | 10 +---------
2 files changed, 3 insertions(+), 20 deletions(-)
diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 25b2383..71ce4cb 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -216,7 +216,6 @@ out:
void libcheck_free (struct checker * c)
{
struct directio_context * ct = (struct directio_context *)c->context;
- struct io_event event;
long flags;
if (!ct)
@@ -232,9 +231,7 @@ 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))
Does io_cancel() do nothing? If does make the IO likely to end sooner,
even if it always returns failure, then we should probably still run it
when we have an outstanding request that we are going to orphan. We
should just move the io_cancel() to the else block, where we add the
request to the orphans list.
Well, it doesn't to _nothing_, but what matters is that it doesn't
actually affect the running I/O. It just cancels the element in the
libaio infrastructure, not the I/O itself.
So Martin is perfectly right here.
Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
Cheers,
Hannes
