mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Issue with fread() and unaligned readv()
@ 2021-03-15 21:39 Dominic Chen
  2021-03-15 21:51 ` Rich Felker
  0 siblings, 1 reply; 8+ messages in thread
From: Dominic Chen @ 2021-03-15 21:39 UTC (permalink / raw)
  To: musl

Not sure this counts as a problem in musl or the application, but I've 
been debugging a return error of EINVAL from `fread(&buf, 8, 16, f)`, 
where `f = fopen("/proc/self/pagemap", "r")`. Internally, musl converts 
this into a call to `readv(f->fd, iov, 2)`, where `iov = {{iov_base = 
buf, iov_len = 127}, {iov_base = f->buf, iov_len = 1024}}`. However, it 
turns out that the kernel VFS read implementation inside `pagemap_read` 
checks that both the file position and count are divisible by 
PM_ENTRY_BYTES (8 on x86_64), otherwise it rejects the read with EINVAL. 
In comparison, glibc's `_IO_file_xsgetn` does appear to try to maintain 
read alignment, although I haven't looked at it in detail.

Thanks,

Dominic


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-15 21:39 [musl] Issue with fread() and unaligned readv() Dominic Chen
@ 2021-03-15 21:51 ` Rich Felker
  2021-03-15 22:09   ` Alexander Monakov
  0 siblings, 1 reply; 8+ messages in thread
From: Rich Felker @ 2021-03-15 21:51 UTC (permalink / raw)
  To: Dominic Chen; +Cc: musl

On Mon, Mar 15, 2021 at 05:39:43PM -0400, Dominic Chen wrote:
> Not sure this counts as a problem in musl or the application, but
> I've been debugging a return error of EINVAL from `fread(&buf, 8,
> 16, f)`, where `f = fopen("/proc/self/pagemap", "r")`. Internally,
> musl converts this into a call to `readv(f->fd, iov, 2)`, where `iov
> = {{iov_base = buf, iov_len = 127}, {iov_base = f->buf, iov_len =
> 1024}}`. However, it turns out that the kernel VFS read
> implementation inside `pagemap_read` checks that both the file
> position and count are divisible by PM_ENTRY_BYTES (8 on x86_64),
> otherwise it rejects the read with EINVAL. In comparison, glibc's
> `_IO_file_xsgetn` does appear to try to maintain read alignment,
> although I haven't looked at it in detail.

You can't use stdio to read or write special files/devices that depend
on the reads or writes happening in particular units, because the
relationship between stdio operations and the underlying
buffer-fill/flush operations on the underlying fd is unspecified. It's
really unfortunate that the kernel lies that procfs files are regular
files but doesn't give them regular-file semantics, but you really
need to use direct operations on the fd in the units the interface
requires, rather than stdio, to work with these files.

Rich

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-15 21:51 ` Rich Felker
@ 2021-03-15 22:09   ` Alexander Monakov
  2021-03-15 22:29     ` Rich Felker
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Monakov @ 2021-03-15 22:09 UTC (permalink / raw)
  To: musl; +Cc: Dominic Chen

On Mon, 15 Mar 2021, Rich Felker wrote:

> On Mon, Mar 15, 2021 at 05:39:43PM -0400, Dominic Chen wrote:
> > Not sure this counts as a problem in musl or the application, but
> > I've been debugging a return error of EINVAL from `fread(&buf, 8,
> > 16, f)`, where `f = fopen("/proc/self/pagemap", "r")`. Internally,
> > musl converts this into a call to `readv(f->fd, iov, 2)`, where `iov
> > = {{iov_base = buf, iov_len = 127}, {iov_base = f->buf, iov_len =
> > 1024}}`. However, it turns out that the kernel VFS read
> > implementation inside `pagemap_read` checks that both the file
> > position and count are divisible by PM_ENTRY_BYTES (8 on x86_64),
> > otherwise it rejects the read with EINVAL. In comparison, glibc's
> > `_IO_file_xsgetn` does appear to try to maintain read alignment,
> > although I haven't looked at it in detail.
> 
> You can't use stdio to read or write special files/devices that depend
> on the reads or writes happening in particular units, because the
> relationship between stdio operations and the underlying
> buffer-fill/flush operations on the underlying fd is unspecified. It's
> really unfortunate that the kernel lies that procfs files are regular
> files but doesn't give them regular-file semantics, but you really
> need to use direct operations on the fd in the units the interface
> requires, rather than stdio, to work with these files.

Where does iov_len = 127 for the first iov tuple come from, though?
From fread arguments I'd expect 8 * 16 = 128.

If musl always does such off-by-one, it is an efficiency issue (forces
a copy with mismatching source/dest alignment).

Alexander

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-15 22:09   ` Alexander Monakov
@ 2021-03-15 22:29     ` Rich Felker
  2021-03-15 22:42       ` Alexander Monakov
  0 siblings, 1 reply; 8+ messages in thread
From: Rich Felker @ 2021-03-15 22:29 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: musl, Dominic Chen

On Tue, Mar 16, 2021 at 01:09:16AM +0300, Alexander Monakov wrote:
> On Mon, 15 Mar 2021, Rich Felker wrote:
> 
> > On Mon, Mar 15, 2021 at 05:39:43PM -0400, Dominic Chen wrote:
> > > Not sure this counts as a problem in musl or the application, but
> > > I've been debugging a return error of EINVAL from `fread(&buf, 8,
> > > 16, f)`, where `f = fopen("/proc/self/pagemap", "r")`. Internally,
> > > musl converts this into a call to `readv(f->fd, iov, 2)`, where `iov
> > > = {{iov_base = buf, iov_len = 127}, {iov_base = f->buf, iov_len =
> > > 1024}}`. However, it turns out that the kernel VFS read
> > > implementation inside `pagemap_read` checks that both the file
> > > position and count are divisible by PM_ENTRY_BYTES (8 on x86_64),
> > > otherwise it rejects the read with EINVAL. In comparison, glibc's
> > > `_IO_file_xsgetn` does appear to try to maintain read alignment,
> > > although I haven't looked at it in detail.
> > 
> > You can't use stdio to read or write special files/devices that depend
> > on the reads or writes happening in particular units, because the
> > relationship between stdio operations and the underlying
> > buffer-fill/flush operations on the underlying fd is unspecified. It's
> > really unfortunate that the kernel lies that procfs files are regular
> > files but doesn't give them regular-file semantics, but you really
> > need to use direct operations on the fd in the units the interface
> > requires, rather than stdio, to work with these files.
> 
> Where does iov_len = 127 for the first iov tuple come from, though?
> >From fread arguments I'd expect 8 * 16 = 128.
> 
> If musl always does such off-by-one, it is an efficiency issue (forces
> a copy with mismatching source/dest alignment).

It's necessary to work around a kernel bug, whereby the kernel fails
to honor the requirement that a readv of total length n behave
identically, except for where the data is stored, as a single read of
length n. For vfs backends that don't implement a proper readv
operation, the kernel executes readv as a sequence of reads. When this
happens, if the amount of data to read is exactly the length of the
first iov (the length requested by the application), continuing to the
second iov with no more data available will cause the operation to
block indefinitely until more data is available. By reducing the
length of the first iov (the caller's buffer) by 1, we ensure that at
least 1 byte of the second iov (the FILE's buffer) is actually needed
to satisfy the caller, and thus that the call will return without
blocking as soon as everything the caller requested is available.

This exact situation arises all the time with one very common type of
file: tty devices. :(

Rich

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-15 22:29     ` Rich Felker
@ 2021-03-15 22:42       ` Alexander Monakov
  2021-03-16  2:13         ` Rich Felker
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Monakov @ 2021-03-15 22:42 UTC (permalink / raw)
  To: musl; +Cc: Dominic Chen

On Mon, 15 Mar 2021, Rich Felker wrote:

> > If musl always does such off-by-one, it is an efficiency issue (forces
> > a copy with mismatching source/dest alignment).
> 
> It's necessary to work around a kernel bug, whereby the kernel fails
> to honor the requirement that a readv of total length n behave
> identically, except for where the data is stored, as a single read of
> length n. For vfs backends that don't implement a proper readv
> operation, the kernel executes readv as a sequence of reads. When this
> happens, if the amount of data to read is exactly the length of the
> first iov (the length requested by the application), continuing to the
> second iov with no more data available will cause the operation to
> block indefinitely until more data is available. By reducing the
> length of the first iov (the caller's buffer) by 1, we ensure that at
> least 1 byte of the second iov (the FILE's buffer) is actually needed
> to satisfy the caller, and thus that the call will return without
> blocking as soon as everything the caller requested is available.

Thanks. Can musl reduce the first iov tuple by, say, 8 bytes rather than
1 byte, to avoid forcing the kernel to perform a misaligned copy?

Alexander

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-15 22:42       ` Alexander Monakov
@ 2021-03-16  2:13         ` Rich Felker
  2021-03-16  9:30           ` Alexander Monakov
  0 siblings, 1 reply; 8+ messages in thread
From: Rich Felker @ 2021-03-16  2:13 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: musl, Dominic Chen

On Tue, Mar 16, 2021 at 01:42:48AM +0300, Alexander Monakov wrote:
> On Mon, 15 Mar 2021, Rich Felker wrote:
> 
> > > If musl always does such off-by-one, it is an efficiency issue (forces
> > > a copy with mismatching source/dest alignment).
> > 
> > It's necessary to work around a kernel bug, whereby the kernel fails
> > to honor the requirement that a readv of total length n behave
> > identically, except for where the data is stored, as a single read of
> > length n. For vfs backends that don't implement a proper readv
> > operation, the kernel executes readv as a sequence of reads. When this
> > happens, if the amount of data to read is exactly the length of the
> > first iov (the length requested by the application), continuing to the
> > second iov with no more data available will cause the operation to
> > block indefinitely until more data is available. By reducing the
> > length of the first iov (the caller's buffer) by 1, we ensure that at
> > least 1 byte of the second iov (the FILE's buffer) is actually needed
> > to satisfy the caller, and thus that the call will return without
> > blocking as soon as everything the caller requested is available.
> 
> Thanks. Can musl reduce the first iov tuple by, say, 8 bytes rather than
> 1 byte, to avoid forcing the kernel to perform a misaligned copy?

Well then you have to do more copy in userspace afterwards, and reduce
the effective buffer size by a bit, going back to kernel slightly more
often or spending extra memory to compensate. There's also no strong
reason to believe one will be aligned and the other won't, except at
beginning of file. The alignment mod 8 depends on file position and
access history, and neither the caller's buffer nor the FILE buffer
have any inherent alignment.

Rich

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-16  2:13         ` Rich Felker
@ 2021-03-16  9:30           ` Alexander Monakov
  2021-03-16 23:54             ` Rich Felker
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Monakov @ 2021-03-16  9:30 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl, Dominic Chen

On Mon, 15 Mar 2021, Rich Felker wrote:

> > Thanks. Can musl reduce the first iov tuple by, say, 8 bytes rather than
> > 1 byte, to avoid forcing the kernel to perform a misaligned copy?
> 
> Well then you have to do more copy in userspace afterwards, and reduce
> the effective buffer size by a bit, going back to kernel slightly more
> often or spending extra memory to compensate.

Of course, but shouldn't you consider how it balances against the
cost to perform a 1K (BUFSIZ) misaligned copy on each read?

> There's also no strong
> reason to believe one will be aligned and the other won't, except at
> beginning of file. The alignment mod 8 depends on file position and
> access history, and neither the caller's buffer nor the FILE buffer
> have any inherent alignment.

The alignment of caller's buffer is another matter, I was talking about
misaligned copy into internal FILE buffer (and even then, at least when
user buffer was malloc'ed it will be sufficiently aligned).

The buffer in FILE obtained from fopen will be aligned to _Alignof(_IO_FILE)
in musl thanks to UNGET being 8.

If the file has been repositioned, yes, bets are off, but I think with stdio
it is quite common to read a file without seeks (could be non-seekable
in the first place).

Alexander

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] Issue with fread() and unaligned readv()
  2021-03-16  9:30           ` Alexander Monakov
@ 2021-03-16 23:54             ` Rich Felker
  0 siblings, 0 replies; 8+ messages in thread
From: Rich Felker @ 2021-03-16 23:54 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: musl, Dominic Chen

On Tue, Mar 16, 2021 at 12:30:11PM +0300, Alexander Monakov wrote:
> On Mon, 15 Mar 2021, Rich Felker wrote:
> 
> > > Thanks. Can musl reduce the first iov tuple by, say, 8 bytes rather than
> > > 1 byte, to avoid forcing the kernel to perform a misaligned copy?
> > 
> > Well then you have to do more copy in userspace afterwards, and reduce
> > the effective buffer size by a bit, going back to kernel slightly more
> > often or spending extra memory to compensate.
> 
> Of course, but shouldn't you consider how it balances against the
> cost to perform a 1K (BUFSIZ) misaligned copy on each read?

Do we have an idea what this cost actually is on popular platforms?
That would help determine if this direction is useful to consider.

> > There's also no strong
> > reason to believe one will be aligned and the other won't, except at
> > beginning of file. The alignment mod 8 depends on file position and
> > access history, and neither the caller's buffer nor the FILE buffer
> > have any inherent alignment.
> 
> The alignment of caller's buffer is another matter, I was talking about
> misaligned copy into internal FILE buffer (and even then, at least when
> user buffer was malloc'ed it will be sufficiently aligned).
> 
> The buffer in FILE obtained from fopen will be aligned to _Alignof(_IO_FILE)
> in musl thanks to UNGET being 8.

Yes, it's just stdin/out/err that have no inherent alignment. The
buffers from fopen will be aligned to at least 4 and perhaps 8
depending on arch ABI and members of FILE.

> If the file has been repositioned, yes, bets are off, but I think with stdio
> it is quite common to read a file without seeks (could be non-seekable
> in the first place).

Well it's plenty common to work with network, terminal, pipe, etc.
containing non-aligned-record content. But these are probably less
interesting for performance.

Looking at the source, we could probably, instead of using
-!!f->buf_size, setup a negative offset based on len and buf_size, up
to 8, that's computed and saved in the first line of the function. If
the offset is equal to len, the first iov can be skipped entirely,
which tends to make the syscall considerably faster -- optimizing this
out was already desirable. Then, the final if (f->buf_size) buf[len-1]
= *f->rpos++; could be replaced with a loop up to this offset that
collapses out if it's zero. This would avoid introducing significant
amounts of new code and might improve things in other cases too.

Rich

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-16 23:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 21:39 [musl] Issue with fread() and unaligned readv() Dominic Chen
2021-03-15 21:51 ` Rich Felker
2021-03-15 22:09   ` Alexander Monakov
2021-03-15 22:29     ` Rich Felker
2021-03-15 22:42       ` Alexander Monakov
2021-03-16  2:13         ` Rich Felker
2021-03-16  9:30           ` Alexander Monakov
2021-03-16 23:54             ` Rich Felker

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).