On 8/25/20 10:38 AM, Josef wrote:
>> Not sure this is an actual bug, but depends on how you look at it. Your
>> poll command has a reference to the file, which means that when you close
>> it here:
>>
>> assert(close(sock_listen_fd1) == 0);
>>
>> then that's not the final close. If you move the io_uring_queue_exit()
>> before that last create_server_socket() it should work, since the poll
>> will have been canceled (and hence the file closed) at that point.
>>
>> That said, I don't believe we actually need the file after arming the
>> poll, so we could potentially close it once we've armed it. That would
>> make your example work.
>
>
> ah okay that makes sense
>
>> Actually we do need the file, in case we're re-arming poll. But as stated
>> in the above email, this isn't unexpected behavior. You could cancel the
>> poll before trying to setup the new server socket, that'd close it as
>> well. Then the close() would actually close it. Ordering of the two
>> operations wouldn't matter.
>>
>> Just to wrap this one up, the below patch would make it behave like you
>> expect, and still retain the re-poll behavior we use on poll armed on
>> behalf of an IO request. At this point we're not holding a reference to
>> the file across the poll handler, and your close() would actually close
>> the file since it's putting the last reference to it.
>>
>> But... Not actually sure this is warranted. Any io_uring request that
>> operates on a file will hold a reference to it until it completes. The
>> poll request in your example never completes. If you run poll(2) on a
>> file and you close that file, you won't get a poll event triggered.
>> It'll just sit there and wait on events that won't come in. poll(2)
>> doesn't hold a reference to the file once it's armed the handler, so
>> your example would work there.
>
> oh thanks I'm gonna test that :) yeah I expected exactly the same
> behaviour as in epoll(2) & pol(2) that's why I'm asking
> to be honest it would be quite handy to have this patch(for netty), so
> I don't have to cancel a poll or close ring file descriptor(I do of
> course understand that if you won't push this patch)
In order for the patch to be able to move ahead, we'd need to be able
to control this behavior. Right now we rely on the file being there if
we need to repoll, see:
commit a6ba632d2c249a4390289727c07b8b55eb02a41d
Author: Jens Axboe <axboe@xxxxxxxxx>
Date: Fri Apr 3 11:10:14 2020 -0600
io_uring: retry poll if we got woken with non-matching mask
If this never happened, we would not need the file at all and we could
make it the default behavior. But don't think that's solvable.
> is there no other way around to close the file descriptor? Even if I
> remove the poll, it doesn't work
If you remove the poll it should definitely work, as nobody is holding a
reference to it as you have nothing else in flight. Can you clarify what
you mean here?
I don't think there's another way, outside of having a poll (io_uring
or poll(2), doesn't matter, the behavior is the same) being triggered in
error. That doesn't happen, as mentioned if you do epoll/poll on a file
and you close it, it won't trigger an event.
> btw if understood correctly poll remove operation refers to all file
> descriptors which arming a poll in the ring buffer right?
> Is there a way to cancel a specific file descriptor poll?
You can cancel specific requests by identifying them with their
->user_data. You can cancel a poll either with POLL_REMOVE or
ASYNC_CANCEL, either one will find it. So as long as you have that, and
it's unique, it'll only cancel that one specific request.
--
Jens Axboe
