io_uring_peek_cqe and EAGAIN

io_uring_peek_cqe and EAGAIN

[William Dauchy]

Hello,

While doing some tests which are open/read/close files I saw that I
was getting -EAGAIN return value sometimesi on io_uring_peek_cqe,
and more often after dropping caches.
In parrallel, when reading examples provided by liburing, we can see
that getting this error is making the example fail (such as in
io_uring-cp). So I was wondering whether it was stupid to change the
example to something like:

diff --git a/examples/io_uring-cp.c b/examples/io_uring-cp.c
index cc7a227..2d6d190 100644
--- a/examples/io_uring-cp.c
+++ b/examples/io_uring-cp.c
@@ -170,11 +170,11 @@ static int copy_file(struct io_uring *ring, off_t insize)
     ret = io_uring_wait_cqe(ring, &cqe);
     got_comp = 1;
    } else {
-    ret = io_uring_peek_cqe(ring, &cqe);
-    if (ret == -EAGAIN) {
-     cqe = NULL;
-     ret = 0;
-    }
+    do {
+     ret = io_uring_peek_cqe(ring, &cqe)
+     if (ret != -EAGAIN)
+      break;
+    } while (1);
    }
    if (ret < 0) {
     fprintf(stderr, "io_uring_peek_cqe: %s\n",


Best,
-- 
William

Re: io_uring_peek_cqe and EAGAIN

[Jens Axboe]

On 4/20/20 10:27 AM, William Dauchy wrote:
> Hello,
> 
> While doing some tests which are open/read/close files I saw that I
> was getting -EAGAIN return value sometimesi on io_uring_peek_cqe,
> and more often after dropping caches.
> In parrallel, when reading examples provided by liburing, we can see
> that getting this error is making the example fail (such as in
> io_uring-cp). So I was wondering whether it was stupid to change the
> example to something like:
> 
> diff --git a/examples/io_uring-cp.c b/examples/io_uring-cp.c
> index cc7a227..2d6d190 100644
> --- a/examples/io_uring-cp.c
> +++ b/examples/io_uring-cp.c
> @@ -170,11 +170,11 @@ static int copy_file(struct io_uring *ring, off_t insize)
>      ret = io_uring_wait_cqe(ring, &cqe);
>      got_comp = 1;
>     } else {
> -    ret = io_uring_peek_cqe(ring, &cqe);
> -    if (ret == -EAGAIN) {
> -     cqe = NULL;
> -     ret = 0;
> -    }
> +    do {
> +     ret = io_uring_peek_cqe(ring, &cqe)
> +     if (ret != -EAGAIN)
> +      break;
> +    } while (1);

I don't think the change is correct. That's not saying that the original
code is necessarily correct, though! Basically there are two cases there:

1) We haven't gotten a completion yet, we'll wait for it.
2) We already found at least one completion. We don't want
   to _wait_ for more, but we can peek and see if there are more.

Hence we don't want to turn case 2 into a loop, we should just
continue.

How is it currently failing for you?

-- 
Jens Axboe

Re: io_uring_peek_cqe and EAGAIN

[William Dauchy]

Hello Jens,

Thank you for your answer on this newbie question :)

On Wed, Apr 22, 2020 at 10:57 PM Jens Axboe <axboe@kernel.dk> wrote:
> I don't think the change is correct. That's not saying that the original
> code is necessarily correct, though! Basically there are two cases there:
>
> 1) We haven't gotten a completion yet, we'll wait for it.
> 2) We already found at least one completion. We don't want
>    to _wait_ for more, but we can peek and see if there are more.
>
> Hence we don't want to turn case 2 into a loop, we should just
> continue.

ok so in fact I think I understand that my usage is incorrect:
1- if I'm in the case of being able to do other things while waiting
for data available using `io_uring_peek_cqe`, I should use it and come
back later when getting a -EAGAIN.
2- it is useless to do a loop on `io_uring_peek_cqe` because in that
case, I should simply do a `io_uring_wait_cqe`

is that correct?

> How is it currently failing for you?

While trying to open/read/close multiple files, I first thought that,
because I had one successful `io_uring_wait_cqe`, I could then loop on
`io_uring_peek_cqe` and get all my data. I now realise my assumption
was completely wrong and this example was just written that way to
show two different possibilities of getting results.
-- 
William

Re: io_uring_peek_cqe and EAGAIN

[Jens Axboe]

On 4/23/20 8:42 AM, William Dauchy wrote:
> Hello Jens,
> 
> Thank you for your answer on this newbie question :)
> 
> On Wed, Apr 22, 2020 at 10:57 PM Jens Axboe <axboe@kernel.dk> wrote:
>> I don't think the change is correct. That's not saying that the original
>> code is necessarily correct, though! Basically there are two cases there:
>>
>> 1) We haven't gotten a completion yet, we'll wait for it.
>> 2) We already found at least one completion. We don't want
>>    to _wait_ for more, but we can peek and see if there are more.
>>
>> Hence we don't want to turn case 2 into a loop, we should just
>> continue.
> 
> ok so in fact I think I understand that my usage is incorrect:
> 1- if I'm in the case of being able to do other things while waiting
> for data available using `io_uring_peek_cqe`, I should use it and come
> back later when getting a -EAGAIN.
> 2- it is useless to do a loop on `io_uring_peek_cqe` because in that
> case, I should simply do a `io_uring_wait_cqe`
> 
> is that correct?

Right, you rarely want to busy loop on io_uring_peek_cqe(), the normal
use case would be to use io_uring_wait_cqe() if you need to wait for a
completion to become available.

>> How is it currently failing for you?
> 
> While trying to open/read/close multiple files, I first thought that,
> because I had one successful `io_uring_wait_cqe`, I could then loop on
> `io_uring_peek_cqe` and get all my data. I now realise my assumption
> was completely wrong and this example was just written that way to
> show two different possibilities of getting results.

Ah ok, yes that sounds like a misunderstanding. Events are posted as
they become available, availability of one does not mean that everything
has completed.

-- 
Jens Axboe

Re: io_uring_peek_cqe and EAGAIN

[William Dauchy]

On Thu, Apr 23, 2020 at 5:05 PM Jens Axboe <axboe@kernel.dk> wrote:
> Ah ok, yes that sounds like a misunderstanding. Events are posted as
> they become available, availability of one does not mean that everything
> has completed.

Indeed, now that I understood my mistake everything is crystal clear.
Thanks a lot for the clarification!

-- 
William