Reliable cancelation or wait for completion of specified sqe

[CarterLi]

When I cancel a sqe, I usually want to free the buffers used by this sqe. This is rather critical in languages which applies RAII pattern.

Following pseudo code is written in C++ because C++ has destructors.

io_uring ring;

struct async_read {
    async_read(int fd) {
        buf = (char *)malloc(512);
        io_uring_sqe *sqe = io_uring_get_sqe(&ring);
        io_uring_prep_read(sqe, fd, buf, 512, 0);
        io_uring_sqe_set_data(sqe, this);
        io_uring_submit(&ring);
        complete = false;
    }

    // destructor, will be called when async_read instance runs out of scope, or an exception is thrown
    ~async_read() {
        if (!complete) {
            io_uring_sqe *sqe = io_uring_get_sqe(&ring);
            io_uring_prep_cancel(sqe, this, 0);
            io_uring_submit(&ring);
            // How can I make sure io_uring_prep_read is canceled or finished here?
        }
        free(buf);
    }

    char* buf;
    bool complete;
};

I must make sure that all resources allocated in constructors be freed before destructors return. If the destructor is called when the async_read task is still pending, I must make sure that the task is settled ( completed or errored / canceled ) before freeing the buffer or use-after-free will happen.

io_uring_prep_cancel provides a way to stop a task immediately. However io_uring_prep_cancel is not reliable:

1. IORING_OP_ASYNC_CANCEL itself is an async task. Even if the task to be canceled is cancelable, it may not be canceled immediately when the cancel task is submitted.
2. IORING_OP_ASYNC_CANCEL may fail. According to the manpage, disk IO requests cannot be canceled if already started. I don't know if there's other cases.

My questions:

1. Will IORING_OP_ASYNC_CANCEL returns immediately? That is to say, always generates a cqe before io_uring_submit returns
2. Since there can be other cqes in the completion queue. Even if IORING_OP_ASYNC_CANCEL returns immediately, I must find its cqe without dequeueing other cqes ( maybe using io_uring_for_each_cqe ), and mark the cqe is handled ( maybe set user_data to NULL ). Is it safe to do so? What if CQ is full?
3. Is there a way to wait ( synchronously / blocking ) for specified sqe's completion? When IORING_OP_ASYNC_CANCEL failed, I must wait for the task's completion.

BTW: I always think that cancelation should not be async. It should either a seperate blocking syscall, or an op code for existing blocking syscall ( for example io_uring_register(ringfd, IORING_REGISTER_CANCEL, &user_data, 1) ), which only returns when task being canceled is either truly canceled or completed / errored.

[axboe]

A CQE is always generated by the time the cancel request has been submitted, that part isn't async at all. But as noted, it may indeed return -EALREADY, which means that the act of canceling the request has been started, but hasn't been completed yet.

For all cases, you cannot rely on cancelation until the request targeted for cancelation has posted the CQE associated with it. For cancelations that return 0 then both will have been posted by the time submission returns, for -EALREADY it may take a bit of time to do so.

If you need sync cancelation, wrap it in something that submits the cancelation request for a specific user_data and then also waits for the completion event of that user_data to be posted? That would be one way to add a sync cancelation interface on top of what io_uring presents. io_uring doesn't provide any way to do that, and I don't think it should as you can implement it on top of the existing interface.

[CarterLi]

then also waits for the completion event of that user_data to be posted?

How can I do that? Only I can come up with is an ugly io_uring_submit - for_each_cqe loop.

What if CQ is full? It is way too hard to handle despite that all these problems wont exist if it is a blocking call

[CarterLi]

A CQE is always generated by the time the cancel request has been submitted, that part isn't async at all.

For cancelations that return 0 then both will have been posted by the time submission returns

Great! We should better write it in the man page. It is very useful in certain cases

[axboe]

Great! We should better write it in the man page. It is very useful in certain cases

Agree, would be awesome if you could send a patch for that :-). Not that I can't do it, but it's often better done by someone who found it confusing before.

[CarterLi]

Leave it to me

[axboe]

You don't necessarily need to reap the particular event, you just need to know if it arrived or not.

But yes, it does depend on how you handle completions. If another thread does that, it becomes more complicated and you'd have to communicate with that.

CQ full is an application thing, there's nothing the kernel can do about that.

That said, it would be possible to implement sync cancelations in the kernel, but it'd be overhead added to basically everything. I'm not so sure this is worth the investment in time and added overhead, to be honest. When we find a request that would cause us to return -EALREADY, we could flag it as such and check for this flag when the completion is posted. That posting would then trigger the sync cancelation request to finish. This might be more appropriately done through not an SQE for cancelation, but an io_uring_register() method that requests cancelation based on similar criteria as the SQE based one. That seems more natural than using an SQE for sync cancelation, exactly because of the same situation where it's hard to wait on a specific CQE rather than just "any CQE" being posted.

[CarterLi]

That said, it would be possible to implement sync cancelations in the kernel, but it'd be overhead added to basically everything. I'm not so sure this is worth the investment in time and added overhead, to be honest.

Okey. So can we support a nonblocking IORING_REGISTER_CANCEL opcode, which does the same thing as IORING_OP_ASYNC_CANCEL, but uses function return value as the result value instead of generating a CQE? In this case, users don't have to find the result in CQ.

In addition, since io_uring_register is always sync, it is easier to know that the cancelation request is run synchronously rather than having to read the manpage to see so.

If we want to have blocking cancelation in the future, we can easily extend functions of this opcode.

[axboe]

Yes, I think that's possible and I've been thinking about a way to do it that is basically free. Here's the idea:

1. For returns that would do 0 or -ENOENT, these are already sync. Doing that off IORING_REGISTER_CANCEL would work the exact same way and no changes are needed.
2. For -EALREADY, if we get that return, then we need to add ourselves to the completion wait queue. That one is woken every time a completion posts, if it's active. Then it's simply the case of retrying the cancelation every time we get woken until we get -ENOENT. Once we do, the request is no longer active and we can return.

With that approach, we have all the bits needed for a sync cancelation API through io_uring_register().

[CarterLi]

Alternatively we could just define that if tv_sec and tv_nsec are both 0, then that means no timeout. If either one is actually set, then that's the absolute timeout to use.

That's what I thought. And if tv_sec and tv_nsec are both -1, then that means wait infinite

Must we use timespec? Can we just use __u64 timeout_ms? Everything should be easier

[axboe]

Yep, that's how I implemented it, check the current branch. We can use a pure msec based timeout, might actually make it easier. The nice thing about the timespec is that we use that consistently elsewhere in io_uring.

I'll be gone for a few hours and will then hack up a test case so I can see if this works as expected.

[CarterLi]

Doesn't seem to be right.

If tv_sec and tv_nsec are both 0, IORING_REGISTER_SYNC_CANCEL should return immediately, just work as IORING_OP_ASYNC_CANCEL. Wait for 0sec should mean I don't want to wait instead of waiting forever.

Wait for -1 should mean waiting forever

[CarterLi]

I mean that

        if (sc.timeout.tv_sec == 0 && sc.timeout.tv_nsec == 0)
                return -ETIME;

	if (sc.timeout.tv_sec != -1 && sc.timeout.tv_nsec != -1) {
		struct timespec64 ts = {
			.tv_sec		= sc.timeout.tv_sec,
			.tv_nsec	= sc.timeout.tv_nsec
		};

		timeout = ktime_add_ns(timespec64_to_ktime(ts), ktime_get_ns());
	}

[axboe]

Yep that looks reasonable and is more logical. I think the check for both being zero should just be ignore, and we'll go through cancelation. If we need to wait, it'll naturally return -ETIME anyway at that point. The timeout only applies to the sleeping part anyway.

[CarterLi]

Some easy code for liburing

commit 0dc983591c2c1bd77ec952d9ebe2a0dc7618a828
Author: 李通洲 <[email protected]>
Date:   Sun Jun 19 01:46:30 2022 +0800

    [WIP] Add io_uring_register_sync_cancel()
    
    Signed-off-by: 李通洲 <[email protected]>

diff --git a/man/io_uring_register.2 b/man/io_uring_register.2
index 1e91caf..1f4517f 100644
--- a/man/io_uring_register.2
+++ b/man/io_uring_register.2
@@ -5,7 +5,7 @@
 .\"
 .TH IO_URING_REGISTER 2 2019-01-17 "Linux" "Linux Programmer's Manual"
 .SH NAME
-io_uring_register \- register files or user buffers for asynchronous I/O 
+io_uring_register \- register files or user buffers for asynchronous I/O
 .SH SYNOPSIS
 .nf
 .BR "#include <linux/io_uring.h>"
@@ -692,3 +692,8 @@ undergoing file or buffer registration, or is being torn down.
 .TP
 .B EOPNOTSUPP
 User buffers point to file-backed memory.
+
+.TP
+.B IORING_REGISTER_SYNC_CANCEL
+
+Available since 5.20.
diff --git a/man/io_uring_register_sync_cancel.3 b/man/io_uring_register_sync_cancel.3
new file mode 100644
index 0000000..d78b85a
--- /dev/null
+++ b/man/io_uring_register_sync_cancel.3
@@ -0,0 +1,21 @@
+.\" Copyright (C) 2022 Jens Axboe <[email protected]>
+.\"
+.\" SPDX-License-Identifier: LGPL-2.0-or-later
+.\"
+.TH io_uring_register_sync_cancel 3 "June 19, 2022" "liburing-2.1" "liburing Manual"
+.SH NAME
+io_uring_register_sync_cancel \- cancel requests synchronously
+.SH SYNOPSIS
+.nf
+.B #include <liburing.h>
+.PP
+.BI "int io_uring_register_sync_cancel(struct io_uring *" ring ","
+.BI "                                  struct io_uring_sync_cancel_reg *" reg ");"
+.fi
+.SH DESCRIPTION
+.PP
+
+.SH RETURN VALUE
+
+.SH SEE ALSO
+.BR io_uring_prep_cancel (3)
diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
index 2f391c9..34a49cc 100644
--- a/src/include/liburing/io_uring.h
+++ b/src/include/liburing/io_uring.h
@@ -411,6 +411,9 @@ enum {
 	IORING_REGISTER_PBUF_RING		= 22,
 	IORING_UNREGISTER_PBUF_RING		= 23,
 
+	/* sync cancelation API */
+	IORING_REGISTER_SYNC_CANCEL		= 24,
+
 	/* this goes last */
 	IORING_REGISTER_LAST
 };
@@ -520,6 +523,15 @@ struct io_uring_buf_reg {
 	__u64	resv[3];
 };
 
+/* argument for IORING_REGISTER_SYNC_CANCEL */
+struct io_uring_sync_cancel_reg {
+	__u64				addr;
+	__s32				fd;
+	__u32				flags;
+	struct __kernel_timespec	timeout;
+	__u64				pad[2];
+};
+
 /*
  * io_uring_restriction->opcode values
  */
diff --git a/src/register.c b/src/register.c
index 993c450..f2b1026 100644
--- a/src/register.c
+++ b/src/register.c
@@ -345,3 +345,10 @@ int io_uring_unregister_buf_ring(struct io_uring *ring, int bgid)
 	return ____sys_io_uring_register(ring->ring_fd,
 					 IORING_UNREGISTER_PBUF_RING, &reg, 1);
 }
+
+int io_uring_register_sync_cancel(struct io_uring *ring,
+				  struct io_uring_sync_cancel_reg *reg)
+{
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_SYNC_CANCEL, reg, 1);
+}

It's about 2am in my timezone. I'm going to go to bed. See you tomorrow.

[axboe]

Tested it and apart from having forgotten that the register parts are invoked with the uring_lock already held, it worked fine. Force pushed the branch with that locking error fixed up, and using -1 for tv_sec/tv_nsec as meaning no timeout. Test case:

/* SPDX-License-Identifier: MIT */
/*
 * Description: test io_uring_register_sync_cancel()
 *
 */
#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>

#include "liburing.h"
#include "helpers.h"

static int test_sync_cancel_timeout(struct io_uring *ring, int async)
{
	struct io_uring_sync_cancel_reg reg = { };
	struct io_uring_sqe *sqe;
	struct io_uring_cqe *cqe;
	int ret, fds[2], to_prep;
	char buf[32];

	if (pipe(fds) < 0) {
		perror("pipe");
		return 1;
	}

	to_prep = 1;
	sqe = io_uring_get_sqe(ring);
	io_uring_prep_read(sqe, fds[0], buf, sizeof(buf), 0);
	sqe->user_data = 0x89;
	if (async)
		sqe->flags |= IOSQE_ASYNC;

	ret = io_uring_submit(ring);
	if (ret != to_prep) {
		fprintf(stderr, "submit=%d\n", ret);
		return 1;
	}

	usleep(10000);

	reg.addr = 0x89;
	reg.timeout.tv_nsec = 1;
	ret = io_uring_register_sync_cancel(ring, &reg);
	if (async) {
		/* we expect -ETIME here, but can race and get 0 */
		if (ret != -ETIME && ret != 0) {
			fprintf(stderr, "sync_cancel=%d\n", ret);
			return 1;
		}
	} else {
		if (ret < 0) {
			fprintf(stderr, "sync_cancel=%d\n", ret);
			return 1;
		}
	}

	/*
	 * we could _almost_ use peek_cqe() here, but there is still
	 * a small gap where io-wq is done with the request and on
	 * its way to posting a completion, but hasn't done it just
	 * yet. the request is canceled and won't be doing any IO
	 * to buffers etc, but the cqe may not have quite arrived yet.
	 */
	ret = io_uring_wait_cqe(ring, &cqe);
	if (ret) {
		fprintf(stderr, "peek=%d\n", ret);
		return 1;
	}
	if (cqe->res >= 0) {
		fprintf(stderr, "cqe->res=%d\n", cqe->res);
		return 1;
	}
	io_uring_cqe_seen(ring, cqe);
	return 0;
}

static int test_sync_cancel(struct io_uring *ring, int async, int nr_all,
			    int use_fd)
{
	struct io_uring_sync_cancel_reg reg = { };
	struct io_uring_sqe *sqe;
	struct io_uring_cqe *cqe;
	int ret, fds[2], to_prep, i;
	char buf[32];

	if (pipe(fds) < 0) {
		perror("pipe");
		return 1;
	}

	to_prep = 1;
	if (nr_all)
		to_prep = 4;
	for (i = 0; i < to_prep; i++) {
		sqe = io_uring_get_sqe(ring);
		io_uring_prep_read(sqe, fds[0], buf, sizeof(buf), 0);
		sqe->user_data = 0x89;
		if (async)
			sqe->flags |= IOSQE_ASYNC;
	}

	ret = io_uring_submit(ring);
	if (ret != to_prep) {
		fprintf(stderr, "submit=%d\n", ret);
		return 1;
	}

	usleep(10000);

	if (!use_fd)
		reg.addr = 0x89;
	else
		reg.fd = fds[0];
	reg.timeout.tv_sec = 200;
	if (nr_all)
		reg.flags |= IORING_ASYNC_CANCEL_ALL;
	if (use_fd)
		reg.flags |= IORING_ASYNC_CANCEL_FD;
	ret = io_uring_register_sync_cancel(ring, &reg);
	if (ret < 0) {
		fprintf(stderr, "sync_cancel=%d\n", ret);
		return 1;
	}

	for (i = 0; i < to_prep; i++) {
		/*
		 * we could _almost_ use peek_cqe() here, but there is still
		 * a small gap where io-wq is done with the request and on
		 * its way to posting a completion, but hasn't done it just
		 * yet. the request is canceled and won't be doing any IO
		 * to buffers etc, but the cqe may not have quite arrived yet.
		 */
		ret = io_uring_wait_cqe(ring, &cqe);
		if (ret) {
			fprintf(stderr, "peek=%d\n", ret);
			return 1;
		}
		if (cqe->res >= 0) {
			fprintf(stderr, "cqe->res=%d\n", cqe->res);
			return 1;
		}
		io_uring_cqe_seen(ring, cqe);
	}

	return 0;
}

int main(int argc, char *argv[])
{
	struct io_uring ring;
	int ret;

	if (argc > 1)
		return 0;

	ret = t_create_ring(7, &ring, 0);
	if (ret == T_SETUP_SKIP)
		return 0;
	else if (ret != T_SETUP_OK)
		return ret;

	ret = test_sync_cancel(&ring, 0, 0, 0);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 0 0 0 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 1, 0, 0);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 1 0 0 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 0, 1, 0);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 0 1 0 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 1, 1, 0);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 1 1 0 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 0, 0, 1);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 0 0 1 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 1, 0, 1);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 1 0 1 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 0, 1, 1);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 0 1 1 failed\n");
		return ret;
	}

	ret = test_sync_cancel(&ring, 1, 1, 1);
	if (ret) {
		fprintf(stderr, "test_sync_cancel 1 1 1 failed\n");
		return ret;
	}

	ret = test_sync_cancel_timeout(&ring, 0);
	if (ret) {
		fprintf(stderr, "test_sync_cancel_timeout 0\n");
		return ret;
	}

	/* must be last, leaves request */
	ret = test_sync_cancel_timeout(&ring, 1);
	if (ret) {
		fprintf(stderr, "test_sync_cancel_timeout 1\n");
		return ret;
	}

	return 0;
}