Re: Data Corruption bug with Samba's vfs_iouring and Linux 5.6.7/5.7rc3

[Pavel Begunkov <asml.silence@gmail.com>]

On 05/05/2020 23:19, Stefan Metzmacher wrote:
> Hi Jens,
> 
>> Got it working, but apparently the arch samba doesn't come with io_uring...
>> One question, though, from looking at the source:
> 
> Thanks for taking a look!
> 
>> static ssize_t vfs_io_uring_pread_recv(struct tevent_req *req,
>> 				  struct vfs_aio_state *vfs_aio_state)
>> {
>> [...]
>> 	if (state->ur.cqe.res < 0) {
>> 		vfs_aio_state->error = -state->ur.cqe.res;
>> 		ret = -1;
>> 	} else {
>> 		vfs_aio_state->error = 0;
>> 		ret = state->ur.cqe.res;
>> 	}
>>
>> 	tevent_req_received(req);
>> [...]
>>
>> I'm assuming this is dealing with short reads?
>>
>> I'll try and see if I can get an arch binary build that has the
>> vfs_io_uring module and reproduce.
> 
> I guess I don't expect short reads for files unless the client asked
> for a read beyond EOF. Does IORING_OP_READV may return short reads
> similar to preadv2 with RWF_NOWAIT? And if so, did this got changed
> since 5.3?

AFAIK, it can. io_uring first tries to submit a request with IOCB_NOWAIT,
in short for performance reasons. And it have been doing so from the beginning
or so. The same is true for writes.

> 
> By default Samba uses pread()/pwrite() from within a helper thread
> and I modeled the io_uring module with the same expecations that
> we wouldn't get a short read if only part of the requested buffer (can
> be up to 8MB) is returned because only some of it is already in the
> buffer cache.
> 
> I'll try the ubuntu 5.4 kernel tomorrow.
> 
> metze
> 

-- 
Pavel Begunkov