| Message ID | 20210601161118.18986-5-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leftover from 2020 | expand |
On 01.06.21 18:10, Olaf Hering wrote: > Read a batch of iovec's. > > In the common case of short reads, finish individual iov's with read_exact. > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > tools/libs/ctrl/xc_private.c | 55 +++++++++++++++++++++++++++++++++++- > tools/libs/ctrl/xc_private.h | 1 + > 2 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/tools/libs/ctrl/xc_private.c b/tools/libs/ctrl/xc_private.c > index d94f846686..ea420b9ba8 100644 > --- a/tools/libs/ctrl/xc_private.c > +++ b/tools/libs/ctrl/xc_private.c > @@ -659,8 +659,23 @@ int write_exact(int fd, const void *data, size_t size) > > #if defined(__MINIOS__) > /* > - * MiniOS's libc doesn't know about writev(). Implement it as multiple write()s. > + * MiniOS's libc doesn't know about readv/writev(). > + * Implement it as multiple read/write()s. > */ > +int readv_exact(int fd, const struct iovec *iov, int iovcnt) > +{ > + int rc, i; > + > + for ( i = 0; i < iovcnt; ++i ) > + { > + rc = read_exact(fd, iov[i].iov_base, iov[i].iov_len); > + if ( rc ) > + return rc; > + } > + > + return 0; > +} > + > int writev_exact(int fd, const struct iovec *iov, int iovcnt) > { > int rc, i; > @@ -675,6 +690,44 @@ int writev_exact(int fd, const struct iovec *iov, int iovcnt) > return 0; > } > #else > +int readv_exact(int fd, const struct iovec *iov, int iovcnt) > +{ > + int rc = 0, idx = 0; > + ssize_t len; > + > + while ( idx < iovcnt ) > + { > + len = readv(fd, &iov[idx], min(iovcnt - idx, IOV_MAX)); > + if ( len == -1 && errno == EINTR ) > + continue; > + if ( len <= 0 ) > + { > + rc = -1; Is EOF really an error? > + goto out; > + } > + while ( len > 0 && idx < iovcnt ) > + { > + if ( len >= iov[idx].iov_len ) > + { > + len -= iov[idx].iov_len; > + } > + else > + { > + void *p = iov[idx].iov_base + len; > + size_t l = iov[idx].iov_len - len; > + > + rc = read_exact(fd, p, l); > + if ( rc ) > + goto out; > + len = 0; This will stop the loop, even if idx hasn't reached iovcnt. > + } > + idx++; > + } > + } > +out: > + return rc; > +} > + Juergen
Am Wed, 2 Jun 2021 08:30:08 +0200 schrieb Juergen Gross <jgross@suse.com>: > On 01.06.21 18:10, Olaf Hering wrote: > > +int readv_exact(int fd, const struct iovec *iov, int iovcnt) > > + if ( len <= 0 ) > > + { > > + rc = -1; > Is EOF really an error? I think yes, that's what "exact" implies IMO. > This will stop the loop, even if idx hasn't reached iovcnt. Yes, it will trigger yet another readv(). The "while" might be a leftover from a variant which used repeated read_exact to finish the function. It should become a "if", and the "len = 0" can be removed. Olaf
Am Wed, 2 Jun 2021 12:57:10 +0200 schrieb Olaf Hering <olaf@aepfle.de>: > > This will stop the loop, even if idx hasn't reached iovcnt. > > Yes, it will trigger yet another readv(). > > The "while" might be a leftover from a variant which used repeated read_exact to finish the function. It should become a "if", and the "len = 0" can be removed. I think the code is correct. The while() loop checks if the last iov had a short read, and finishes that iov with read_exact. I will add a comment, if that helps. Olaf
On 02.06.21 12:57, Olaf Hering wrote: > Am Wed, 2 Jun 2021 08:30:08 +0200 > schrieb Juergen Gross <jgross@suse.com>: > >> On 01.06.21 18:10, Olaf Hering wrote: >>> +int readv_exact(int fd, const struct iovec *iov, int iovcnt) > >>> + if ( len <= 0 ) >>> + { >>> + rc = -1; >> Is EOF really an error? > > I think yes, that's what "exact" implies IMO. Shouldn't you check for zero length iovec elements as in the writev_exact() case then? >> This will stop the loop, even if idx hasn't reached iovcnt. > > Yes, it will trigger yet another readv(). Ah, right. Juergen
Am Wed, 2 Jun 2021 13:41:02 +0200 schrieb Juergen Gross <jgross@suse.com>: > Shouldn't you check for zero length iovec elements as in the > writev_exact() case then? I will double check if this is a hard requirement. Olaf
Am Wed, 2 Jun 2021 13:41:02 +0200 schrieb Juergen Gross <jgross@suse.com>: > Shouldn't you check for zero length iovec elements as in the > writev_exact() case then? It is not clear to me what the purpose of skipping zero-length iov's at the beginning of the iov array in writev_exact is. I assume the syscall (or libc) will just skip them anyway. The code would not handle empty iov's in the middle of the array. Perhaps writev_exact should be done in the same way as my readv_exact, that would also remove the need for malloc. I think in the past I had a patch to do just that. Maybe just locally, maybe I sent it to this list. In case of readv_exact, only the trailing incomplete iov will be filled with read_exact. I think the length parameter for read_exact can not become zero, "len" would already be zero. Zero-length iov's will be skipped by the loop AFAICS. Olaf
diff --git a/tools/libs/ctrl/xc_private.c b/tools/libs/ctrl/xc_private.c index d94f846686..ea420b9ba8 100644 --- a/tools/libs/ctrl/xc_private.c +++ b/tools/libs/ctrl/xc_private.c @@ -659,8 +659,23 @@ int write_exact(int fd, const void *data, size_t size) #if defined(__MINIOS__) /* - * MiniOS's libc doesn't know about writev(). Implement it as multiple write()s. + * MiniOS's libc doesn't know about readv/writev(). + * Implement it as multiple read/write()s. */ +int readv_exact(int fd, const struct iovec *iov, int iovcnt) +{ + int rc, i; + + for ( i = 0; i < iovcnt; ++i ) + { + rc = read_exact(fd, iov[i].iov_base, iov[i].iov_len); + if ( rc ) + return rc; + } + + return 0; +} + int writev_exact(int fd, const struct iovec *iov, int iovcnt) { int rc, i; @@ -675,6 +690,44 @@ int writev_exact(int fd, const struct iovec *iov, int iovcnt) return 0; } #else +int readv_exact(int fd, const struct iovec *iov, int iovcnt) +{ + int rc = 0, idx = 0; + ssize_t len; + + while ( idx < iovcnt ) + { + len = readv(fd, &iov[idx], min(iovcnt - idx, IOV_MAX)); + if ( len == -1 && errno == EINTR ) + continue; + if ( len <= 0 ) + { + rc = -1; + goto out; + } + while ( len > 0 && idx < iovcnt ) + { + if ( len >= iov[idx].iov_len ) + { + len -= iov[idx].iov_len; + } + else + { + void *p = iov[idx].iov_base + len; + size_t l = iov[idx].iov_len - len; + + rc = read_exact(fd, p, l); + if ( rc ) + goto out; + len = 0; + } + idx++; + } + } +out: + return rc; +} + int writev_exact(int fd, const struct iovec *iov, int iovcnt) { struct iovec *local_iov = NULL; diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h index f0b5f83ac8..5d2c7274fb 100644 --- a/tools/libs/ctrl/xc_private.h +++ b/tools/libs/ctrl/xc_private.h @@ -441,6 +441,7 @@ int xc_flush_mmu_updates(xc_interface *xch, struct xc_mmu *mmu); /* Return 0 on success; -1 on error setting errno. */ int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */ +int readv_exact(int fd, const struct iovec *iov, int iovcnt); int write_exact(int fd, const void *data, size_t size); int writev_exact(int fd, const struct iovec *iov, int iovcnt);
Read a batch of iovec's. In the common case of short reads, finish individual iov's with read_exact. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/libs/ctrl/xc_private.c | 55 +++++++++++++++++++++++++++++++++++- tools/libs/ctrl/xc_private.h | 1 + 2 files changed, 55 insertions(+), 1 deletion(-)