On 6/9/22 4:32 AM, Hao Xu wrote: > On 6/9/22 18:19, Jens Axboe wrote: >> On 6/9/22 4:14 AM, Hao Xu wrote: >>> On 6/9/22 18:06, Jens Axboe wrote: >>>> On 6/9/22 1:53 AM, Hao Xu wrote: >>>>> Hi all, >>>>> I haven't done tests to demonstrate it. It is for partial io case, we >>>>> don't consume/release the buffer before arm_poll in ring-mapped mode. >>>>> But seems we should? Otherwise ring head isn't moved and other requests >>>>> may take that buffer. What do I miss? >>>> >>>> On vacation this week, so can't take a look at the code. But the >>>> principle is precisely not to consume the buffer if we arm poll, because >>>> then the next one can grab it instead. We don't want to consume a buffer >>>> over poll, as that defeats the purpose of a provided buffer. It should >>>> be grabbed and consumed only if we can use it right now. >>>> >>>> Hence the way it should work is that we DON'T consume the buffer in this >>>> case, and that someone else can just use it. At the same time, we should >>>> ensure that we grab a NEW buffer for this case, whenever the poll >>> >>> If we grab a new buffer for it, then we have to copy the data since we >>> have done partial io...this also defeats the purpose of this feature. >> >> For partial IO, we never drop the buffer. See the logic in >> io_kbuf_recycle(). It should be as follows: > > Yea, in io_kbuf_recycle(), if it's partial io, we just return. For > legacy mode, this means we keep the buffer. For ring-mapped mode, this > means we then release the uring_lock without moving the ring->head, > and then other requests may take that buffer which is in use.. > And next time we do (for example) recv(), we lost the data which we got > at the previous time. > Do I miss something? If we don't commit for ring mapped buffers, then yeah that's definitely a bug. Please send a fix :-) Pavel can take care of it this week. -- Jens Axboe
