* [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev()
@ 2019-08-07 7:02 piaojun
2019-08-07 15:42 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 4+ messages in thread
From: piaojun @ 2019-08-07 7:02 UTC (permalink / raw)
To: virtio-fs
Define fuse_buf_writev() which use pwritev and writev to improve io
bandwidth.
Signed-off-by: Jun Piao <piaojun@huawei.com>
Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
contrib/virtiofsd/buffer.c | 37 +++++++++++++++++++++++++++++++++++++
contrib/virtiofsd/fuse_common.h | 14 ++++++++++++++
contrib/virtiofsd/seccomp.c | 2 ++
3 files changed, 53 insertions(+)
diff --git a/contrib/virtiofsd/buffer.c b/contrib/virtiofsd/buffer.c
index 655be137..642830f 100644
--- a/contrib/virtiofsd/buffer.c
+++ b/contrib/virtiofsd/buffer.c
@@ -15,6 +15,7 @@
#include <unistd.h>
#include <errno.h>
#include <assert.h>
+#include <stdlib.h>
size_t fuse_buf_size(const struct fuse_bufvec *bufv)
{
@@ -71,6 +72,42 @@ static ssize_t fuse_buf_write(const struct fuse_buf *dst, size_t dst_off,
return copied;
}
+ssize_t fuse_buf_writev(fuse_req_t req,
+ struct fuse_bufvec *out_buf,
+ struct fuse_bufvec *in_buf,
+ enum fuse_buf_copy_flags flags)
+{
+ ssize_t res, i, buf_index, iovcnt;
+ struct iovec * iov;
+ int fd = out_buf->buf[0].fd;
+ off_t pos = out_buf->buf[0].pos;
+
+ if (in_buf->count > 2)
+ iovcnt = in_buf->count - 1;
+ else
+ iovcnt = 1;
+
+ iov = malloc(iovcnt * sizeof(struct iovec));
+ if (!iov)
+ return -ENOMEM;
+
+ for (i = 0, buf_index = 1; i < iovcnt; i++, buf_index++) {
+ iov[i].iov_base = in_buf->buf[buf_index].mem;
+ iov[i].iov_len = in_buf->buf[buf_index].size;
+ }
+
+ if (flags & FUSE_BUF_FD_SEEK)
+ res = pwritev(fd, iov, iovcnt, pos);
+ else
+ res = writev(fd, iov, iovcnt);
+
+ if (res == -1 || res == 0)
+ res = -errno;
+
+ free(iov);
+ return res;
+}
+
static ssize_t fuse_buf_read(const struct fuse_buf *dst, size_t dst_off,
const struct fuse_buf *src, size_t src_off,
size_t len)
diff --git a/contrib/virtiofsd/fuse_common.h b/contrib/virtiofsd/fuse_common.h
index 001e6d6..7c07da7 100644
--- a/contrib/virtiofsd/fuse_common.h
+++ b/contrib/virtiofsd/fuse_common.h
@@ -730,6 +730,20 @@ ssize_t fuse_buf_copy(fuse_req_t req,
enum fuse_buf_copy_flags flags);
/**
+ * Write data from one buffer vector to another
+ *
+ * @param req The request this copy is part of
+ * @param out_buf destination buffer vector
+ * @param in_buf source buffer vector
+ * @param flags flags controlling the copy
+ * @return actual number of bytes written or -errno on error
+ */
+ssize_t fuse_buf_writev(fuse_req_t req,
+ struct fuse_bufvec *out_buf,
+ struct fuse_bufvec *in_buf,
+ enum fuse_buf_copy_flags flags);
+
+/**
* Memory buffer iterator
*
*/
diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c
index 5f1c873..4bba30b 100644
--- a/contrib/virtiofsd/seccomp.c
+++ b/contrib/virtiofsd/seccomp.c
@@ -60,6 +60,7 @@ static const int syscall_whitelist[] = {
SCMP_SYS(ppoll),
SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */
SCMP_SYS(preadv),
+ SCMP_SYS(pwritev),
SCMP_SYS(pwrite64),
SCMP_SYS(read),
SCMP_SYS(readlinkat),
@@ -78,6 +79,7 @@ static const int syscall_whitelist[] = {
SCMP_SYS(unlinkat),
SCMP_SYS(utimensat),
SCMP_SYS(write),
+ SCMP_SYS(writev),
};
/* Syscalls used when --syslog is enabled */
--
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev()
2019-08-07 7:02 [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev() piaojun
@ 2019-08-07 15:42 ` Dr. David Alan Gilbert
2019-08-08 1:37 ` piaojun
2019-08-08 6:27 ` piaojun
0 siblings, 2 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 15:42 UTC (permalink / raw)
To: piaojun; +Cc: virtio-fs
* piaojun (piaojun@huawei.com) wrote:
> Define fuse_buf_writev() which use pwritev and writev to improve io
> bandwidth.
>
> Signed-off-by: Jun Piao <piaojun@huawei.com>
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> contrib/virtiofsd/buffer.c | 37 +++++++++++++++++++++++++++++++++++++
> contrib/virtiofsd/fuse_common.h | 14 ++++++++++++++
> contrib/virtiofsd/seccomp.c | 2 ++
> 3 files changed, 53 insertions(+)
>
> diff --git a/contrib/virtiofsd/buffer.c b/contrib/virtiofsd/buffer.c
> index 655be137..642830f 100644
> --- a/contrib/virtiofsd/buffer.c
> +++ b/contrib/virtiofsd/buffer.c
> @@ -15,6 +15,7 @@
> #include <unistd.h>
> #include <errno.h>
> #include <assert.h>
> +#include <stdlib.h>
>
> size_t fuse_buf_size(const struct fuse_bufvec *bufv)
> {
> @@ -71,6 +72,42 @@ static ssize_t fuse_buf_write(const struct fuse_buf *dst, size_t dst_off,
> return copied;
> }
>
> +ssize_t fuse_buf_writev(fuse_req_t req,
> + struct fuse_bufvec *out_buf,
> + struct fuse_bufvec *in_buf,
> + enum fuse_buf_copy_flags flags)
> +{
> + ssize_t res, i, buf_index, iovcnt;
> + struct iovec * iov;
> + int fd = out_buf->buf[0].fd;
> + off_t pos = out_buf->buf[0].pos;
That's again assuming that the 'pos' in subsequent elements of out_buf
is sequential; is it?
What about any other flags on in_buf?
> + if (in_buf->count > 2)
> + iovcnt = in_buf->count - 1;
> + else
> + iovcnt = 1;
> +
> + iov = malloc(iovcnt * sizeof(struct iovec));
You could use calloc.
> + if (!iov)
> + return -ENOMEM;
> +
> + for (i = 0, buf_index = 1; i < iovcnt; i++, buf_index++) {
> + iov[i].iov_base = in_buf->buf[buf_index].mem;
> + iov[i].iov_len = in_buf->buf[buf_index].size;
> + }
> +
> + if (flags & FUSE_BUF_FD_SEEK)
> + res = pwritev(fd, iov, iovcnt, pos);
> + else
> + res = writev(fd, iov, iovcnt);
> +
> + if (res == -1 || res == 0)
> + res = -errno;
Does errno get set on a res == 0 ?
Dave
> + free(iov);
> + return res;
> +}
> +
> static ssize_t fuse_buf_read(const struct fuse_buf *dst, size_t dst_off,
> const struct fuse_buf *src, size_t src_off,
> size_t len)
> diff --git a/contrib/virtiofsd/fuse_common.h b/contrib/virtiofsd/fuse_common.h
> index 001e6d6..7c07da7 100644
> --- a/contrib/virtiofsd/fuse_common.h
> +++ b/contrib/virtiofsd/fuse_common.h
> @@ -730,6 +730,20 @@ ssize_t fuse_buf_copy(fuse_req_t req,
> enum fuse_buf_copy_flags flags);
>
> /**
> + * Write data from one buffer vector to another
> + *
> + * @param req The request this copy is part of
> + * @param out_buf destination buffer vector
> + * @param in_buf source buffer vector
> + * @param flags flags controlling the copy
> + * @return actual number of bytes written or -errno on error
> + */
> +ssize_t fuse_buf_writev(fuse_req_t req,
> + struct fuse_bufvec *out_buf,
> + struct fuse_bufvec *in_buf,
> + enum fuse_buf_copy_flags flags);
> +
> +/**
> * Memory buffer iterator
> *
> */
> diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c
> index 5f1c873..4bba30b 100644
> --- a/contrib/virtiofsd/seccomp.c
> +++ b/contrib/virtiofsd/seccomp.c
> @@ -60,6 +60,7 @@ static const int syscall_whitelist[] = {
> SCMP_SYS(ppoll),
> SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */
> SCMP_SYS(preadv),
> + SCMP_SYS(pwritev),
> SCMP_SYS(pwrite64),
> SCMP_SYS(read),
> SCMP_SYS(readlinkat),
> @@ -78,6 +79,7 @@ static const int syscall_whitelist[] = {
> SCMP_SYS(unlinkat),
> SCMP_SYS(utimensat),
> SCMP_SYS(write),
> + SCMP_SYS(writev),
> };
>
> /* Syscalls used when --syslog is enabled */
> --
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev()
2019-08-07 15:42 ` Dr. David Alan Gilbert
@ 2019-08-08 1:37 ` piaojun
2019-08-08 6:27 ` piaojun
1 sibling, 0 replies; 4+ messages in thread
From: piaojun @ 2019-08-08 1:37 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs
Hi Dave,
On 2019/8/7 23:42, Dr. David Alan Gilbert wrote:
> * piaojun (piaojun@huawei.com) wrote:
>> Define fuse_buf_writev() which use pwritev and writev to improve io
>> bandwidth.
>>
>> Signed-off-by: Jun Piao <piaojun@huawei.com>
>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>> contrib/virtiofsd/buffer.c | 37 +++++++++++++++++++++++++++++++++++++
>> contrib/virtiofsd/fuse_common.h | 14 ++++++++++++++
>> contrib/virtiofsd/seccomp.c | 2 ++
>> 3 files changed, 53 insertions(+)
>>
>> diff --git a/contrib/virtiofsd/buffer.c b/contrib/virtiofsd/buffer.c
>> index 655be137..642830f 100644
>> --- a/contrib/virtiofsd/buffer.c
>> +++ b/contrib/virtiofsd/buffer.c
>> @@ -15,6 +15,7 @@
>> #include <unistd.h>
>> #include <errno.h>
>> #include <assert.h>
>> +#include <stdlib.h>
>>
>> size_t fuse_buf_size(const struct fuse_bufvec *bufv)
>> {
>> @@ -71,6 +72,42 @@ static ssize_t fuse_buf_write(const struct fuse_buf *dst, size_t dst_off,
>> return copied;
>> }
>>
>> +ssize_t fuse_buf_writev(fuse_req_t req,
>> + struct fuse_bufvec *out_buf,
>> + struct fuse_bufvec *in_buf,
>> + enum fuse_buf_copy_flags flags)
>> +{
>> + ssize_t res, i, buf_index, iovcnt;
>> + struct iovec * iov;
>> + int fd = out_buf->buf[0].fd;
>> + off_t pos = out_buf->buf[0].pos;
>
> That's again assuming that the 'pos' in subsequent elements of out_buf
> is sequential; is it?
> What about any other flags on in_buf?
out_buf->buf[0].pos records the offset which the write starts, and I
just moved it from lo_write_buf() without changing the logic. Then I
will handle all the flags just as the last reply.
>
>> + if (in_buf->count > 2)
>> + iovcnt = in_buf->count - 1;
>> + else
>> + iovcnt = 1;
>> +
>> + iov = malloc(iovcnt * sizeof(struct iovec));
>
> You could use calloc.
Accepted.
>
>> + if (!iov)
>> + return -ENOMEM;
>> +
>> + for (i = 0, buf_index = 1; i < iovcnt; i++, buf_index++) {
>> + iov[i].iov_base = in_buf->buf[buf_index].mem;
>> + iov[i].iov_len = in_buf->buf[buf_index].size;
>> + }
>> +
>> + if (flags & FUSE_BUF_FD_SEEK)
>> + res = pwritev(fd, iov, iovcnt, pos);
>> + else
>> + res = writev(fd, iov, iovcnt);
>> +
>> + if (res == -1 || res == 0)
>> + res = -errno;
>
> Does errno get set on a res == 0 ?
No, this should be fixed and I plan returning res when it's 0. Could we
just consider this as copied none bytes?
>
> Dave
>
>> + free(iov);
>> + return res;
>> +}
>> +
>> static ssize_t fuse_buf_read(const struct fuse_buf *dst, size_t dst_off,
>> const struct fuse_buf *src, size_t src_off,
>> size_t len)
>> diff --git a/contrib/virtiofsd/fuse_common.h b/contrib/virtiofsd/fuse_common.h
>> index 001e6d6..7c07da7 100644
>> --- a/contrib/virtiofsd/fuse_common.h
>> +++ b/contrib/virtiofsd/fuse_common.h
>> @@ -730,6 +730,20 @@ ssize_t fuse_buf_copy(fuse_req_t req,
>> enum fuse_buf_copy_flags flags);
>>
>> /**
>> + * Write data from one buffer vector to another
>> + *
>> + * @param req The request this copy is part of
>> + * @param out_buf destination buffer vector
>> + * @param in_buf source buffer vector
>> + * @param flags flags controlling the copy
>> + * @return actual number of bytes written or -errno on error
>> + */
>> +ssize_t fuse_buf_writev(fuse_req_t req,
>> + struct fuse_bufvec *out_buf,
>> + struct fuse_bufvec *in_buf,
>> + enum fuse_buf_copy_flags flags);
>> +
>> +/**
>> * Memory buffer iterator
>> *
>> */
>> diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c
>> index 5f1c873..4bba30b 100644
>> --- a/contrib/virtiofsd/seccomp.c
>> +++ b/contrib/virtiofsd/seccomp.c
>> @@ -60,6 +60,7 @@ static const int syscall_whitelist[] = {
>> SCMP_SYS(ppoll),
>> SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */
>> SCMP_SYS(preadv),
>> + SCMP_SYS(pwritev),
>> SCMP_SYS(pwrite64),
>> SCMP_SYS(read),
>> SCMP_SYS(readlinkat),
>> @@ -78,6 +79,7 @@ static const int syscall_whitelist[] = {
>> SCMP_SYS(unlinkat),
>> SCMP_SYS(utimensat),
>> SCMP_SYS(write),
>> + SCMP_SYS(writev),
>> };
>>
>> /* Syscalls used when --syslog is enabled */
>> --
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev()
2019-08-07 15:42 ` Dr. David Alan Gilbert
2019-08-08 1:37 ` piaojun
@ 2019-08-08 6:27 ` piaojun
1 sibling, 0 replies; 4+ messages in thread
From: piaojun @ 2019-08-08 6:27 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: virtio-fs
Hi Dave,
As Stefan's suggestion, I make the patch set in one thread [PATCH v3]
which is more convenient for reviewing. Please make comments on the new
patch set if you have any question. I'll be very grateful for your
advice.
Thanks,
Jun
On 2019/8/7 23:42, Dr. David Alan Gilbert wrote:
> * piaojun (piaojun@huawei.com) wrote:
>> Define fuse_buf_writev() which use pwritev and writev to improve io
>> bandwidth.
>>
>> Signed-off-by: Jun Piao <piaojun@huawei.com>
>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>> contrib/virtiofsd/buffer.c | 37 +++++++++++++++++++++++++++++++++++++
>> contrib/virtiofsd/fuse_common.h | 14 ++++++++++++++
>> contrib/virtiofsd/seccomp.c | 2 ++
>> 3 files changed, 53 insertions(+)
>>
>> diff --git a/contrib/virtiofsd/buffer.c b/contrib/virtiofsd/buffer.c
>> index 655be137..642830f 100644
>> --- a/contrib/virtiofsd/buffer.c
>> +++ b/contrib/virtiofsd/buffer.c
>> @@ -15,6 +15,7 @@
>> #include <unistd.h>
>> #include <errno.h>
>> #include <assert.h>
>> +#include <stdlib.h>
>>
>> size_t fuse_buf_size(const struct fuse_bufvec *bufv)
>> {
>> @@ -71,6 +72,42 @@ static ssize_t fuse_buf_write(const struct fuse_buf *dst, size_t dst_off,
>> return copied;
>> }
>>
>> +ssize_t fuse_buf_writev(fuse_req_t req,
>> + struct fuse_bufvec *out_buf,
>> + struct fuse_bufvec *in_buf,
>> + enum fuse_buf_copy_flags flags)
>> +{
>> + ssize_t res, i, buf_index, iovcnt;
>> + struct iovec * iov;
>> + int fd = out_buf->buf[0].fd;
>> + off_t pos = out_buf->buf[0].pos;
>
> That's again assuming that the 'pos' in subsequent elements of out_buf
> is sequential; is it?
> What about any other flags on in_buf?
>
>> + if (in_buf->count > 2)
>> + iovcnt = in_buf->count - 1;
>> + else
>> + iovcnt = 1;
>> +
>> + iov = malloc(iovcnt * sizeof(struct iovec));
>
> You could use calloc.
>
>> + if (!iov)
>> + return -ENOMEM;
>> +
>> + for (i = 0, buf_index = 1; i < iovcnt; i++, buf_index++) {
>> + iov[i].iov_base = in_buf->buf[buf_index].mem;
>> + iov[i].iov_len = in_buf->buf[buf_index].size;
>> + }
>> +
>> + if (flags & FUSE_BUF_FD_SEEK)
>> + res = pwritev(fd, iov, iovcnt, pos);
>> + else
>> + res = writev(fd, iov, iovcnt);
>> +
>> + if (res == -1 || res == 0)
>> + res = -errno;
>
> Does errno get set on a res == 0 ?
>
> Dave
>
>> + free(iov);
>> + return res;
>> +}
>> +
>> static ssize_t fuse_buf_read(const struct fuse_buf *dst, size_t dst_off,
>> const struct fuse_buf *src, size_t src_off,
>> size_t len)
>> diff --git a/contrib/virtiofsd/fuse_common.h b/contrib/virtiofsd/fuse_common.h
>> index 001e6d6..7c07da7 100644
>> --- a/contrib/virtiofsd/fuse_common.h
>> +++ b/contrib/virtiofsd/fuse_common.h
>> @@ -730,6 +730,20 @@ ssize_t fuse_buf_copy(fuse_req_t req,
>> enum fuse_buf_copy_flags flags);
>>
>> /**
>> + * Write data from one buffer vector to another
>> + *
>> + * @param req The request this copy is part of
>> + * @param out_buf destination buffer vector
>> + * @param in_buf source buffer vector
>> + * @param flags flags controlling the copy
>> + * @return actual number of bytes written or -errno on error
>> + */
>> +ssize_t fuse_buf_writev(fuse_req_t req,
>> + struct fuse_bufvec *out_buf,
>> + struct fuse_bufvec *in_buf,
>> + enum fuse_buf_copy_flags flags);
>> +
>> +/**
>> * Memory buffer iterator
>> *
>> */
>> diff --git a/contrib/virtiofsd/seccomp.c b/contrib/virtiofsd/seccomp.c
>> index 5f1c873..4bba30b 100644
>> --- a/contrib/virtiofsd/seccomp.c
>> +++ b/contrib/virtiofsd/seccomp.c
>> @@ -60,6 +60,7 @@ static const int syscall_whitelist[] = {
>> SCMP_SYS(ppoll),
>> SCMP_SYS(prctl), /* TODO restrict to just PR_SET_NAME? */
>> SCMP_SYS(preadv),
>> + SCMP_SYS(pwritev),
>> SCMP_SYS(pwrite64),
>> SCMP_SYS(read),
>> SCMP_SYS(readlinkat),
>> @@ -78,6 +79,7 @@ static const int syscall_whitelist[] = {
>> SCMP_SYS(unlinkat),
>> SCMP_SYS(utimensat),
>> SCMP_SYS(write),
>> + SCMP_SYS(writev),
>> };
>>
>> /* Syscalls used when --syslog is enabled */
>> --
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-08 6:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-07 7:02 [Virtio-fs] [PATCH v2 1/2][RFC] virtiofsd: virtiofsd: add definition of fuse_buf_writev() piaojun
2019-08-07 15:42 ` Dr. David Alan Gilbert
2019-08-08 1:37 ` piaojun
2019-08-08 6:27 ` piaojun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.