Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper way to wait for multiple CQEs no longer than the specified period of time #693

Open
dmantipov opened this issue Oct 19, 2022 · 9 comments

Comments

@dmantipov
Copy link
Contributor

dmantipov commented Oct 19, 2022

For the submission like the following:

for (i = 0; i < nr; i++) {
	struct io_uring_sqe *sqe = io_uring_get_sqe(&ring);
	io_uring_prep_WHATEVER(sqe, ...);
}
io_uring_submit(ring);

what it the proper way to wait for nr CQEs no longer than the specified period of time? Doing

do {
	struct io_uring_cqe *cqe;
	if (io_uring_wait_cqe_timeout(&ring, &cqe, &timeout)) {
		process(cqe);
                nr--;
	}
} while (nr > 0)

may yield in nr * timeout time spent waiting. OTOH io_uring_wait_cqes() seems always returns the only CQE regardless on how much of the expected ones was specified in wait_nr argument.

@davidzeng0
Copy link
Contributor

davidzeng0 commented Oct 21, 2022

may yield in nr * timeout time spent waiting

I'm quite sure io_uring_wait_cqe_timeout only waits timeout time, not multilpied

@DylanZA
Copy link
Contributor

DylanZA commented Oct 21, 2022

OTOH io_uring_wait_cqes() seems always returns the only CQE regardless on how much of the expected ones was specified in wait_nr argument

this is true, but the rest of the CQE's will be available. you could either use io_uring_peek_batch_cqe or just io_uring_peek_cqe after this call.

Perhaps there should be a io_uring_wait_cqes_batch method too

@isilence
Copy link
Collaborator

io_uring_peek_batch_cqe() is there but it's only a waste of time copying pointers of ready cqes into an array. Personally, I'd use io_uring_for_each_cqe(), but be beware that it wouldn't work if IORING_FEAT_EXT_ARG is not set. The flag is pretty old and should be there for any new enough kernel though.

@davidzeng0
Copy link
Contributor

may yield in nr * timeout time spent waiting

I'm quite sure io_uring_wait_cqe_timeout only waits timeout time, not multilpied

Forget I said this

In your code, you only process one cqe before looping again (and waiting on the timeout, again) resulting in your nr * timeout timeout. After you wait for a cqe, you should batch process all available cqes before waiting for more.

@axboe
Copy link
Owner

axboe commented Oct 21, 2022

Like was mentioned in previous comments, your initial wait may return more than one cqe. You iterate over them once the call returns, and then advance the cq ring to indicate you consumed them.

I do think it'd be nice if we have a io_uring_wait_cqes_timeout(ring, nr, timeout) so this could be done easily. That would return when either nr events had been completed, or time out if we didn't get nr events before the timeout completed. This is mostly an efficiency thing as you could do the same with io_uring_wait_cqe_timeout(), decrementing timeout for each call. But that ends up doing multiple system calls, where an explicit helper that allowed you to specify both would just do a single system call, if needed. As a work-around, io_uring_submit_and_wait_timeout() could be used, even if not submitting.

@dmantipov
Copy link
Contributor Author

dmantipov commented Oct 25, 2022

it'd be nice if we have a io_uring_wait_cqes_timeout(ring, nr, timeout)

What about making io_uring_wait_cqes() return -ETIME if no events has been completed or non-negative number of events has been completed before the timeout has expired? This is similar to poll() which returns a non-negative value which is the number of elements in the pollfds whose revents fields have been set to a nonzero. The most common reason is that you might want to pre-allocate some data structures to store the data from CQEs, and so there will be an easy way to find the number of elements you need to allocate - without calling the helpers like io_uring_cq_ready().

@davidzeng0
Copy link
Contributor

It already does that.
Also, io_uring_cq_ready is not a syscall, it is simply two reads and a subtraction :)

@dmantipov
Copy link
Contributor Author

It already does that

As of liburing 2.2, the manual page explicitly states that

On  success io_uring_wait_cqes(3) returns 0 and the cqe_ptr parm is filled in.

@davidzeng0
Copy link
Contributor

davidzeng0 commented Oct 27, 2022

Try upgrading your liburing library if that's really the case. But also, when zero cqes are returned, it's not success.
It's an error and should be -ETIME

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants