Re: [PATCH 1/1] io_uring: fix leaks on IOPOLL and CQE_SKIP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/15/22 23:41, Pavel Begunkov wrote:
On 4/15/22 23:03, Jens Axboe wrote:
On 4/15/22 3:05 PM, Pavel Begunkov wrote:
On 4/12/22 17:46, Jens Axboe wrote:
On 4/12/22 10:41 AM, Jens Axboe wrote:
On 4/12/22 10:24 AM, Pavel Begunkov wrote:
If all completed requests in io_do_iopoll() were marked with
REQ_F_CQE_SKIP, we'll not only skip CQE posting but also
io_free_batch_list() leaking memory and resources.

Move @nr_events increment before REQ_F_CQE_SKIP check. We'll potentially
return the value greater than the real one, but iopolling will deal with
it and the userspace will re-iopoll if needed. In anyway, I don't think
there are many use cases for REQ_F_CQE_SKIP + IOPOLL.

Ah good catch - yes probably not much practical concern, as the lack of
ordering for file IO means that CQE_SKIP isn't really useful for that
scenario.

One potential snag is with the change we're now doing
io_cqring_ev_posted_iopoll() even if didn't post an event. Again
probably not a practical concern, but it is theoretically a violation
if an eventfd is used.
Looks this didn't get applied. Are you concerned about eventfd?

Yep, was hoping to get a reply back, so just deferred it for now.

Is there any good reason why the userspace can't tolerate spurious
eventfd events? Because I don't think we should care this case

I always forget the details on that, but we've had cases like this in
the past where some applications assume that if they got N eventfd
events, then are are also N events in the ring. Which granted is a bit
odd, but it does also make some sense. Why would you have more eventfd
events posted than events?

For the same reason why it can get less eventfd events than there are
CQEs, as for me it's only a communication channel but not a

s/communication/notification/

replacement for completion events.

--
Pavel Begunkov



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux