Mailing List Archive: [PATCH v20210601 04/38] tools: add readv

[PATCH v20210601 04/38] tools: add readv_exact to libxenctrl

Jun 1, 2021, 9:10 AM

Post #1 of 7 (6 views)

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;
+ 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);

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

jgross at suse

Jun 1, 2021, 11:30 PM

Post #2 of 7 (6 views)

Permalink

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

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

olaf at aepfle

Jun 2, 2021, 3:57 AM

Post #3 of 7 (6 views)

Permalink

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

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

olaf at aepfle

Jun 2, 2021, 4:05 AM

Post #4 of 7 (6 views)

Permalink

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

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

jgross at suse

Jun 2, 2021, 4:41 AM

Post #5 of 7 (6 views)

Permalink

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

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

olaf at aepfle

Jun 7, 2021, 2:46 AM

Post #6 of 7 (6 views)

Permalink

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

Re: [PATCH v20210601 04/38] tools: add readv_exact to libxenctrl [ In reply to ]

olaf at aepfle

Jun 7, 2021, 4:31 AM

Post #7 of 7 (6 views)

Permalink

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

Mailing List Archive

Attached Files:

Attached Files: