Re: [PATCH] io_uring/rw: always clear ->bytes_done on io_async_rw setup

[Jens Axboe <axboe@kernel.dk>]

On 12/30/24 9:08 AM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> A previous commit mistakenly moved the clearing of the in-progress byte
>> count into the section that's dependent on having a cached iovec or not,
>> but it should be cleared for any IO. If not, then extra bytes may be
>> added at IO completion time, causing potentially weird behavior like
>> over-reporting the amount of IO done.
> 
> Hi Jens,
> 
> Sorry for the delay.  I went completely offline during the christmas
> week.

No worries, sounds like a good plan!

> Did this solve the sysbot report?  I'm failing to understand how it can
> happen.  This could only be hit if the allocation returned a cached
> object that doesn't have a free_iov, since any newly kmalloc'ed object
> will have this field cleaned inside the io_rw_async_data_init callback.
> But I don't understand where we can cache the rw object without having a
> valid free_iov - it didn't seem possible to me before or now.

Not sure I follow - you may never have a valid free_iov, it completely
depends on whether or not the existing rw user needed to allocate an iov
or not. Hence it's indeed possible that there's a free_iov and the user
doesn't need or use it, or the opposite of there not being one and the
user then allocating one that persists.

In any case, it's of course orthogonal to the issue here, which is that
->bytes_done must _always_ be initialized, it has no dependency on a
free_iovec or not. Whenever someone gets an 'rw', it should be pristine
in that sense.

-- 
Jens Axboe