[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1.1 5/6] tools/libxs: Use writev()/sendmsg() instead of write()
On 2024-07-23 08:30, Juergen Gross wrote: On 23.07.24 14:25, Andrew Cooper wrote:On 23/07/2024 10:37 am, Jürgen Groß wrote:On 22.07.24 18:25, Andrew Cooper wrote:With the input data now conveniently arranged, use writev()/sendmsg() instead of decomposing it into write() calls. This causes all requests to be submitted with a single system call, rather than at least two. While in principle short writes can occur, the chances ofit happening are slim given that most xenbus comms are only a handful ofbytes. Nevertheless, provide {writev,sendmsg}_exact() wrappers which take care of resubmitting on EINTR or short write. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> --- CC: Anthony PERARD <anthony.perard@xxxxxxxxxx> CC: Juergen Gross <jgross@xxxxxxxx> CC: Frediano Ziglio <frediano.ziglio@xxxxxxxxx> v1.1: * Fix iov overread, spotted by Frediano. Factor the common updating logic out into update_iov(). ---tools/libs/store/xs.c | 94 +++++++++++++++++++++++++++++++++++++++++--1 file changed, 91 insertions(+), 3 deletions(-) diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c index e820cccc2314..f80ac7558cbe 100644 --- a/tools/libs/store/xs.c +++ b/tools/libs/store/xs.c @@ -563,6 +563,95 @@ static void *read_reply( return body; } +/* + * Update an iov/nr pair after an incomplete writev()/sendmsg(). + * + * Awkwardly, nr has different widths and signs between writev() and + * sendmsg(), so we take it and return it by value, rather than by pointer. + */ +static size_t update_iov(struct iovec **p_iov, size_t nr, size_t res) +{ + struct iovec *iov = *p_iov; + + /* Skip fully complete elements, including empty elements. */ + while (nr && res >= iov->iov_len) { + res -= iov->iov_len; + nr--; + iov++; + } + + /* Partial element, adjust base/len. */ + if (res) { + iov->iov_len -= res; + iov->iov_base += res; + } + + *p_iov = iov; + + return nr; +} + +/* + * Wrapper around sendmsg() to resubmit on EINTR or short write. Returns + * @true if all data was transmitted, or @false with errno for an error. + * Note: May alter @iov in place on resubmit. + */ +static bool sendmsg_exact(int fd, struct iovec *iov, unsigned int nr) +{ + struct msghdr hdr = { + .msg_iov = iov, + .msg_iovlen = nr, + }; + + /* Sanity check first element isn't empty */ + assert(iov->iov_len == sizeof(struct xsd_sockmsg));Can you please move this assert() into write_request(), avoiding to have 2 copies of it?It was more relevant before update_iov() was split out. But, there's exactly the same assertion in the write_request()'s caller, so I'd prefer to simply drop it if that's ok? The writev()/sendmsg() won't malfunction if the first element is 0, and update_iov() will now cope too, so I don't think it's necessary.Fine with me. Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>Looks like xs_write_all() is now unused internally, but it's an exposed library function. I guess it can just be kept instead of bumping the library version. Regards, Jason
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |