> On 1/20/18 7:23 PM, Goldwyn Rodrigues wrote:
> > On 01/20/2018 08:11 PM, Andi Kleen wrote:
> >> Goldwyn Rodrigues <rgold...@suse.de> writes:
> >>> From: Goldwyn Rodrigues <rgold...@suse.com>
> >>> In case direct I/O encounters an error midway, it returns the error.
> >>> Instead it should be returning the number of bytes transferred so far.
> >>
> >> It's likely there's a lot of code in user space that does
> >>
> >> if (write(..., N) < 0) handle error
> >>
> >> With your change it would need to be
> >>
> >> if (write(..., N) != N) handle error
> >>
> >> How much code is actually doing that?
> >>
> >> I can understand it fixes your artifical test suite, but it seems to me
> >> your
> >> change has a high potential to break a lot of existing user code
> >> in subtle ways. So it seems to be a bad idea.
> >>
> >> -Andi
> >
> > Quoting 'man 2 write':
> >
> > RETURN VALUE
> > On success, the number of bytes written is returned (zero indicates
> > nothing was written). It is not an error if this number is smaller
> > than the number of bytes requested; this may happen for example because
> > the disk device was filled. See also NOTES.
>
> You can quote as much man page as you want - Andi is well aware of how
> read/write system call works, as I'm sure all of us are, that is not the
> issue. The issue is that there are potentially LOTS of applications out
> there that do not check for short writes, they do exactly what Andi
> speculated above. If you break it with this change, it doesn't matter
> what's in the man page. What matters is previous behavior, and that
> you are breaking user space. At that point nobody cares what's in the
> man page.
fio engines/sg.c fio_sgio_rw_doio() has that pattern:
ret = write(f->fd, hdr, sizeof(*hdr));
if (ret < 0)
return ret;
...
return FIO_Q_QUEUED; [which is 1]
although there might be special circumstances for the sg interface
making that safe.
---
Robert Elliott, HPE Persistent Memory
