Hi Pavel,
On Wed, 2021-05-05 at 18:56 +0100, Pavel Begunkov wrote:
> On 5/4/21 7:06 PM, Olivier Langlois wrote:
> >
> >
> > 2. I don't understand what I am looking at. Why am I receiving a
> > completion notification for a POLL request that has just been
> > cancelled? What is the logic behind silently discarding a
> > IORING_OP_POLL_ADD sqe meant to replace an existing one?
>
> I'm lost in your message, so let's start with simple reasons. All
> requests post one CQE (almost true), including poll_remove requests.
>
> io_uring_prep_poll_remove(sqe, iouring_build_user_data(IOURING_POLL,
> fd, anfds [fd].egen));
> // io_uring_sqe_set_data(sqe, iouring_build_user_data(IOURING_POLL, fd,
> anfds [fd].egen));
>
> If poll remove and poll requests have identical user_data, as in
> the second (commented?) line you'll get two CQEs with that user_data.
>
> Did you check return value (in CQE) of poll remove? I'd recommend
> set its user_data to something unique. Did you consider that it
> may fail?
Your comments does help me to see clearer!
You are correct that setting the poll remove user_data is not done.
(hence the commented out statement for that purpose but I must have
entertain the idea to set it at some point to see what good it would
do)
The reason being that I do not care about whether or not it succeeds
because the very next thing that I do is to rearm the poll for the same
fd with a different event mask.
Beside, the removed poll original sqe is reported back as ECANCELED (-
125):
85 gen 1 res -125
This appear to be the code returned in io_poll_remove_one()
Does the poll remove operation generates 2 cqes?
1 for the canceled sqe and and 1 for the poll remove sqe itself?
I am not too sure what good setting a distinct user_data to the poll
remove sqe could do but I will definitely give it a shot to perhaps see
clearer.
Note that the poll remove sqe and the following poll add sqe don't have
exactly the same user_data.
I have this statement in between:
/* increment generation counter to avoid handling old events */
++anfds [fd].egen;
poll remove cancel the previous poll add having gen 1 in its user data.
the next poll add has it user_data storing gen var set to 2:
1 3 remove 85 1
1 3 add 85 2
85 gen 1 res -125
85 gen 1 res 4
I'll try to be more succinct this time.
If the poll add sqe having gen 1 in its user_data is cancelled, how can
its completion can be reported in the very next cqe?
and I never hear back about the poll add gen 2 sqe...
I'll try to get more familiar with the fs/io_uring.c code but it feels
like it could be some optimization done where because the cancelled
poll result is available while inside io_uring_enter(), instead of
discarding it to immediately rearm it for the new poll add request,
io_uring_enter() instead decide to return it back to reply to the gen 2
request but it forgets to update the user_data field before doing so...
Maybe what is confusing is that the heading "1 3" in my traces is the
EV_READ EV_WRITE bitmask which values are:
EV_READ = 1
EV_WRITE = 2
while
POLLIN = 1
POLLOUT = 4
So my poll add gen 1 request was for be notified for POLLIN. This is
what I got with the question #1 "195" result.
Therefore the:
85 gen 1 res 4
can only be for my poll add gen 2 requesting for POLLIN|POLLOUT. Yet,
it is reported with the previous request user_data...
I feel the mystery is almost solved with your help... I'll continue my
investigation and I'll report back if I finally solve the mystery.
>
> > 3. As I am writing this email, I have just noticed that it was
> > possible
> > to update an existing POLL entry with IORING_OP_POLL_REMOVE through
> > io_uring_prep_poll_update(). Is this what I should do to eliminate my
> > problems? What are the possible race conditions scenarios that I
> > should
> > be prepared to handle by using io_uring_prep_poll_update() (ie:
> > completion of the poll entry to update while my process is inside
> > io_uring_enter() to update it...)?
>
> Update is preferable, but it's Linux kernel 5.13.
> Both remove and update may fail. e.g. with -EALREADY
>
I am just about to install 5.12 on my system and this and the new
multishot poll feature makes me already crave 5.13!
Greetings,
Olivier
