* [PATCH] splice: only read in as much information as there is pipe buffer space
@ 2019-08-29 16:11 Darrick J. Wong
2019-08-30 0:44 ` [RFC PATCH] generic: test splice() with pipes Darrick J. Wong
2019-08-30 21:06 ` [PATCH v2] splice: only read in as much information as there is pipe buffer space Darrick J. Wong
0 siblings, 2 replies; 15+ messages in thread
From: Darrick J. Wong @ 2019-08-29 16:11 UTC (permalink / raw)
To: viro, andreas.gruenbacher; +Cc: xfs, linux-fsdevel
From: Darrick J. Wong <darrick.wong@oracle.com>
Andreas Grünbacher reports that on the two filesystems that support
iomap directio, it's possible for splice() to return -EAGAIN (instead of
a short splice) if the pipe being written to has less space available in
its pipe buffers than the length supplied by the calling process.
Months ago we fixed splice_direct_to_actor to clamp the length of the
read request to the size of the splice pipe. Do the same to do_splice.
Fixes: 17614445576b6 ("splice: don't read more than available pipe space")
Reported-by: Andreas Grünbacher <andreas.gruenbacher@gmail.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/splice.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/splice.c b/fs/splice.c
index 98412721f056..7865a3bb6d88 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1101,6 +1101,7 @@ static long do_splice(struct file *in, loff_t __user *off_in,
struct pipe_inode_info *ipipe;
struct pipe_inode_info *opipe;
loff_t offset;
+ unsigned int pipe_pages;
long ret;
ipipe = get_pipe_info(in);
@@ -1123,6 +1124,10 @@ static long do_splice(struct file *in, loff_t __user *off_in,
if ((in->f_flags | out->f_flags) & O_NONBLOCK)
flags |= SPLICE_F_NONBLOCK;
+ /* Don't try to read more the pipe has space for. */
+ pipe_pages = opipe->buffers - opipe->nrbufs;
+ len = min(len, (size_t)pipe_pages << PAGE_SHIFT);
+
return splice_pipe_to_pipe(ipipe, opipe, len, flags);
}
@@ -1180,8 +1185,13 @@ static long do_splice(struct file *in, loff_t __user *off_in,
pipe_lock(opipe);
ret = wait_for_space(opipe, flags);
- if (!ret)
+ if (!ret) {
+ /* Don't try to read more the pipe has space for. */
+ pipe_pages = opipe->buffers - opipe->nrbufs;
+ len = min(len, (size_t)pipe_pages << PAGE_SHIFT);
+
ret = do_splice_to(in, &offset, opipe, len, flags);
+ }
pipe_unlock(opipe);
if (ret > 0)
wakeup_pipe_readers(opipe);
^ permalink raw reply related [flat|nested] 15+ messages in thread* [RFC PATCH] generic: test splice() with pipes 2019-08-29 16:11 [PATCH] splice: only read in as much information as there is pipe buffer space Darrick J. Wong @ 2019-08-30 0:44 ` Darrick J. Wong 2019-09-02 2:20 ` Zorro Lang 2019-11-21 17:01 ` Darrick J. Wong 2019-08-30 21:06 ` [PATCH v2] splice: only read in as much information as there is pipe buffer space Darrick J. Wong 1 sibling, 2 replies; 15+ messages in thread From: Darrick J. Wong @ 2019-08-30 0:44 UTC (permalink / raw) To: andreas.gruenbacher; +Cc: xfs, viro, linux-fsdevel, fstests From: Darrick J. Wong <darrick.wong@oracle.com> Andreas Grünbacher reports that on the two filesystems that support iomap directio, it's possible for splice() to return -EAGAIN (instead of a short splice) if the pipe being written to has less space available in its pipe buffers than the length supplied by the calling process. This is a regression test to check for correct operation. XXX Andreas: Since you wrote the C reproducer, can you send me the proper copyright and author attribution statement for the C program? Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- .gitignore | 1 src/Makefile | 2 - src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/720 | 41 ++++++++++++ tests/generic/720.out | 7 ++ tests/generic/group | 1 6 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 src/splice-test.c create mode 100755 tests/generic/720 create mode 100644 tests/generic/720.out diff --git a/.gitignore b/.gitignore index c8c815f9..26d4da11 100644 --- a/.gitignore +++ b/.gitignore @@ -112,6 +112,7 @@ /src/runas /src/seek_copy_test /src/seek_sanity_test +/src/splice-test /src/stale_handle /src/stat_test /src/swapon diff --git a/src/Makefile b/src/Makefile index c4fcf370..2920dfb1 100644 --- a/src/Makefile +++ b/src/Makefile @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ dio-invalidate-cache stat_test t_encrypted_d_revalidate \ attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ - fscrypt-crypt-util bulkstat_null_ocount + fscrypt-crypt-util bulkstat_null_ocount splice-test SUBDIRS = log-writes perf diff --git a/src/splice-test.c b/src/splice-test.c new file mode 100644 index 00000000..d3c12075 --- /dev/null +++ b/src/splice-test.c @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2019 ????????????????????????????? + * Author: + * + * Make sure that reading and writing to a pipe via splice. + */ +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <unistd.h> +#include <fcntl.h> +#include <err.h> + +#include <stdlib.h> +#include <stdio.h> +#include <stdbool.h> +#include <string.h> +#include <errno.h> + +#define SECTOR_SIZE 512 +#define BUFFER_SIZE (150 * SECTOR_SIZE) + +void read_from_pipe(int fd, const char *filename, size_t size) +{ + char buffer[SECTOR_SIZE]; + size_t sz; + ssize_t ret; + + while (size) { + sz = size; + if (sz > sizeof buffer) + sz = sizeof buffer; + ret = read(fd, buffer, sz); + if (ret < 0) + err(1, "read: %s", filename); + if (ret == 0) { + fprintf(stderr, "read: %s: unexpected EOF\n", filename); + exit(1); + } + size -= sz; + } +} + +void do_splice1(int fd, const char *filename, size_t size) +{ + bool retried = false; + int pipefd[2]; + + if (pipe(pipefd) == -1) + err(1, "pipe"); + while (size) { + ssize_t spliced; + + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); + if (spliced == -1) { + if (errno == EAGAIN && !retried) { + retried = true; + fprintf(stderr, "retrying splice\n"); + sleep(1); + continue; + } + err(1, "splice"); + } + read_from_pipe(pipefd[0], filename, spliced); + size -= spliced; + } + close(pipefd[0]); + close(pipefd[1]); +} + +void do_splice2(int fd, const char *filename, size_t size) +{ + bool retried = false; + int pipefd[2]; + int pid; + + if (pipe(pipefd) == -1) + err(1, "pipe"); + + pid = fork(); + if (pid == 0) { + close(pipefd[1]); + read_from_pipe(pipefd[0], filename, size); + exit(0); + } else { + close(pipefd[0]); + while (size) { + ssize_t spliced; + + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); + if (spliced == -1) { + if (errno == EAGAIN && !retried) { + retried = true; + fprintf(stderr, "retrying splice\n"); + sleep(1); + continue; + } + err(1, "splice"); + } + size -= spliced; + } + close(pipefd[1]); + waitpid(pid, NULL, 0); + } +} + +void usage(const char *argv0) +{ + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); + exit(2); +} + +int main(int argc, char *argv[]) +{ + void (*do_splice)(int fd, const char *filename, size_t size); + const char *filename; + char *buffer; + int opt, open_flags, fd; + ssize_t ret; + + do_splice = do_splice1; + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; + + while ((opt = getopt(argc, argv, "rd")) != -1) { + switch(opt) { + case 'r': + do_splice = do_splice2; + break; + case 'd': + open_flags &= ~O_DIRECT; + break; + default: /* '?' */ + usage(argv[0]); + } + } + + if (optind >= argc) + usage(argv[0]); + filename = argv[optind]; + + printf("%s reader %s O_DIRECT\n", + do_splice == do_splice1 ? "sequential" : "concurrent", + (open_flags & O_DIRECT) ? "with" : "without"); + + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); + if (buffer == NULL) + err(1, "aligned_alloc"); + + fd = open(filename, open_flags, 0666); + if (fd == -1) + err(1, "open: %s", filename); + + memset(buffer, 'x', BUFFER_SIZE); + ret = write(fd, buffer, BUFFER_SIZE); + if (ret < 0) + err(1, "write: %s", filename); + if (ret != BUFFER_SIZE) { + fprintf(stderr, "%s: short write\n", filename); + exit(1); + } + + ret = lseek(fd, 0, SEEK_SET); + if (ret != 0) + err(1, "lseek: %s", filename); + + do_splice(fd, filename, BUFFER_SIZE); + + if (unlink(filename) == -1) + err(1, "unlink: %s", filename); + + return 0; +} diff --git a/tests/generic/720 b/tests/generic/720 new file mode 100755 index 00000000..b7f09c40 --- /dev/null +++ b/tests/generic/720 @@ -0,0 +1,41 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. +# +# FS QA Test No. 720 +# +# Test using splice() to read from pipes. + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $TEST_DIR/a +} + +# get standard environment, filters and checks +. ./common/rc + +# real QA test starts here +_supported_os Linux +_supported_fs generic +_require_test + +rm -f $seqres.full + +src/splice-test -r $TEST_DIR/a +src/splice-test -rd $TEST_DIR/a +src/splice-test $TEST_DIR/a +src/splice-test -d $TEST_DIR/a + +# success, all done +status=0 +exit diff --git a/tests/generic/720.out b/tests/generic/720.out new file mode 100644 index 00000000..b0fc9935 --- /dev/null +++ b/tests/generic/720.out @@ -0,0 +1,7 @@ +QA output created by 720 +concurrent reader with O_DIRECT +concurrent reader with O_DIRECT +concurrent reader without O_DIRECT +concurrent reader without O_DIRECT +sequential reader with O_DIRECT +sequential reader without O_DIRECT diff --git a/tests/generic/group b/tests/generic/group index cd418106..f75d4e60 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -569,3 +569,4 @@ 564 auto quick copy_range 565 auto quick copy_range 719 auto quick quota metadata +720 auto quick rw pipe splice ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-08-30 0:44 ` [RFC PATCH] generic: test splice() with pipes Darrick J. Wong @ 2019-09-02 2:20 ` Zorro Lang 2019-09-02 16:56 ` Darrick J. Wong 2019-11-21 17:01 ` Darrick J. Wong 1 sibling, 1 reply; 15+ messages in thread From: Zorro Lang @ 2019-09-02 2:20 UTC (permalink / raw) To: Darrick J. Wong; +Cc: andreas.gruenbacher, xfs, viro, linux-fsdevel, fstests On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Andreas Grünbacher reports that on the two filesystems that support > iomap directio, it's possible for splice() to return -EAGAIN (instead of > a short splice) if the pipe being written to has less space available in > its pipe buffers than the length supplied by the calling process. > > This is a regression test to check for correct operation. > > XXX Andreas: Since you wrote the C reproducer, can you send me the > proper copyright and author attribution statement for the C program? > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Hi Darrick, I tried to add a splice_f operation into xfs_io long time ago: https://marc.info/?l=linux-xfs&m=155828702128047&w=2 For we can easily write more splice related test case later, I'd like to have a xfs_io to help that (if it can). Would you like to help to review that patch, if it can match your requirement to write this case? Or it can be improved to cover more testing conditions? I'm really appreciate that:) Thanks, Zorro > .gitignore | 1 > src/Makefile | 2 - > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/720 | 41 ++++++++++++ > tests/generic/720.out | 7 ++ > tests/generic/group | 1 > 6 files changed, 224 insertions(+), 1 deletion(-) > create mode 100644 src/splice-test.c > create mode 100755 tests/generic/720 > create mode 100644 tests/generic/720.out > > diff --git a/.gitignore b/.gitignore > index c8c815f9..26d4da11 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -112,6 +112,7 @@ > /src/runas > /src/seek_copy_test > /src/seek_sanity_test > +/src/splice-test > /src/stale_handle > /src/stat_test > /src/swapon > diff --git a/src/Makefile b/src/Makefile > index c4fcf370..2920dfb1 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > - fscrypt-crypt-util bulkstat_null_ocount > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > SUBDIRS = log-writes perf > > diff --git a/src/splice-test.c b/src/splice-test.c > new file mode 100644 > index 00000000..d3c12075 > --- /dev/null > +++ b/src/splice-test.c > @@ -0,0 +1,173 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 ????????????????????????????? > + * Author: > + * > + * Make sure that reading and writing to a pipe via splice. > + */ > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/wait.h> > +#include <unistd.h> > +#include <fcntl.h> > +#include <err.h> > + > +#include <stdlib.h> > +#include <stdio.h> > +#include <stdbool.h> > +#include <string.h> > +#include <errno.h> > + > +#define SECTOR_SIZE 512 > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > + > +void read_from_pipe(int fd, const char *filename, size_t size) > +{ > + char buffer[SECTOR_SIZE]; > + size_t sz; > + ssize_t ret; > + > + while (size) { > + sz = size; > + if (sz > sizeof buffer) > + sz = sizeof buffer; > + ret = read(fd, buffer, sz); > + if (ret < 0) > + err(1, "read: %s", filename); > + if (ret == 0) { > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > + exit(1); > + } > + size -= sz; > + } > +} > + > +void do_splice1(int fd, const char *filename, size_t size) > +{ > + bool retried = false; > + int pipefd[2]; > + > + if (pipe(pipefd) == -1) > + err(1, "pipe"); > + while (size) { > + ssize_t spliced; > + > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > + if (spliced == -1) { > + if (errno == EAGAIN && !retried) { > + retried = true; > + fprintf(stderr, "retrying splice\n"); > + sleep(1); > + continue; > + } > + err(1, "splice"); > + } > + read_from_pipe(pipefd[0], filename, spliced); > + size -= spliced; > + } > + close(pipefd[0]); > + close(pipefd[1]); > +} > + > +void do_splice2(int fd, const char *filename, size_t size) > +{ > + bool retried = false; > + int pipefd[2]; > + int pid; > + > + if (pipe(pipefd) == -1) > + err(1, "pipe"); > + > + pid = fork(); > + if (pid == 0) { > + close(pipefd[1]); > + read_from_pipe(pipefd[0], filename, size); > + exit(0); > + } else { > + close(pipefd[0]); > + while (size) { > + ssize_t spliced; > + > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > + if (spliced == -1) { > + if (errno == EAGAIN && !retried) { > + retried = true; > + fprintf(stderr, "retrying splice\n"); > + sleep(1); > + continue; > + } > + err(1, "splice"); > + } > + size -= spliced; > + } > + close(pipefd[1]); > + waitpid(pid, NULL, 0); > + } > +} > + > +void usage(const char *argv0) > +{ > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > + exit(2); > +} > + > +int main(int argc, char *argv[]) > +{ > + void (*do_splice)(int fd, const char *filename, size_t size); > + const char *filename; > + char *buffer; > + int opt, open_flags, fd; > + ssize_t ret; > + > + do_splice = do_splice1; > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > + > + while ((opt = getopt(argc, argv, "rd")) != -1) { > + switch(opt) { > + case 'r': > + do_splice = do_splice2; > + break; > + case 'd': > + open_flags &= ~O_DIRECT; > + break; > + default: /* '?' */ > + usage(argv[0]); > + } > + } > + > + if (optind >= argc) > + usage(argv[0]); > + filename = argv[optind]; > + > + printf("%s reader %s O_DIRECT\n", > + do_splice == do_splice1 ? "sequential" : "concurrent", > + (open_flags & O_DIRECT) ? "with" : "without"); > + > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > + if (buffer == NULL) > + err(1, "aligned_alloc"); > + > + fd = open(filename, open_flags, 0666); > + if (fd == -1) > + err(1, "open: %s", filename); > + > + memset(buffer, 'x', BUFFER_SIZE); > + ret = write(fd, buffer, BUFFER_SIZE); > + if (ret < 0) > + err(1, "write: %s", filename); > + if (ret != BUFFER_SIZE) { > + fprintf(stderr, "%s: short write\n", filename); > + exit(1); > + } > + > + ret = lseek(fd, 0, SEEK_SET); > + if (ret != 0) > + err(1, "lseek: %s", filename); > + > + do_splice(fd, filename, BUFFER_SIZE); > + > + if (unlink(filename) == -1) > + err(1, "unlink: %s", filename); > + > + return 0; > +} > diff --git a/tests/generic/720 b/tests/generic/720 > new file mode 100755 > index 00000000..b7f09c40 > --- /dev/null > +++ b/tests/generic/720 > @@ -0,0 +1,41 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > +# > +# FS QA Test No. 720 > +# > +# Test using splice() to read from pipes. > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $TEST_DIR/a > +} > + > +# get standard environment, filters and checks > +. ./common/rc > + > +# real QA test starts here > +_supported_os Linux > +_supported_fs generic > +_require_test > + > +rm -f $seqres.full > + > +src/splice-test -r $TEST_DIR/a > +src/splice-test -rd $TEST_DIR/a > +src/splice-test $TEST_DIR/a > +src/splice-test -d $TEST_DIR/a > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/720.out b/tests/generic/720.out > new file mode 100644 > index 00000000..b0fc9935 > --- /dev/null > +++ b/tests/generic/720.out > @@ -0,0 +1,7 @@ > +QA output created by 720 > +concurrent reader with O_DIRECT > +concurrent reader with O_DIRECT > +concurrent reader without O_DIRECT > +concurrent reader without O_DIRECT > +sequential reader with O_DIRECT > +sequential reader without O_DIRECT > diff --git a/tests/generic/group b/tests/generic/group > index cd418106..f75d4e60 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -569,3 +569,4 @@ > 564 auto quick copy_range > 565 auto quick copy_range > 719 auto quick quota metadata > +720 auto quick rw pipe splice ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-09-02 2:20 ` Zorro Lang @ 2019-09-02 16:56 ` Darrick J. Wong 2019-09-03 3:19 ` Zorro Lang 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-09-02 16:56 UTC (permalink / raw) To: Zorro Lang; +Cc: andreas.gruenbacher, xfs, viro, linux-fsdevel, fstests On Mon, Sep 02, 2019 at 10:20:52AM +0800, Zorro Lang wrote: > On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Andreas Grünbacher reports that on the two filesystems that support > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > a short splice) if the pipe being written to has less space available in > > its pipe buffers than the length supplied by the calling process. > > > > This is a regression test to check for correct operation. > > > > XXX Andreas: Since you wrote the C reproducer, can you send me the > > proper copyright and author attribution statement for the C program? > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > Hi Darrick, > > I tried to add a splice_f operation into xfs_io long time ago: > https://marc.info/?l=linux-xfs&m=155828702128047&w=2 > > For we can easily write more splice related test case later, I'd like to have > a xfs_io to help that (if it can). Would you like to help to review that patch, > if it can match your requirement to write this case? Or it can be improved to > cover more testing conditions? I'm really appreciate that:) Yeah, I'll try to have a look this week. (FWIW I think Eric is backed up with dozens of xfsprogs patches...) --D > Thanks, > Zorro > > > .gitignore | 1 > > src/Makefile | 2 - > > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/720 | 41 ++++++++++++ > > tests/generic/720.out | 7 ++ > > tests/generic/group | 1 > > 6 files changed, 224 insertions(+), 1 deletion(-) > > create mode 100644 src/splice-test.c > > create mode 100755 tests/generic/720 > > create mode 100644 tests/generic/720.out > > > > diff --git a/.gitignore b/.gitignore > > index c8c815f9..26d4da11 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -112,6 +112,7 @@ > > /src/runas > > /src/seek_copy_test > > /src/seek_sanity_test > > +/src/splice-test > > /src/stale_handle > > /src/stat_test > > /src/swapon > > diff --git a/src/Makefile b/src/Makefile > > index c4fcf370..2920dfb1 100644 > > --- a/src/Makefile > > +++ b/src/Makefile > > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > > - fscrypt-crypt-util bulkstat_null_ocount > > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > > > SUBDIRS = log-writes perf > > > > diff --git a/src/splice-test.c b/src/splice-test.c > > new file mode 100644 > > index 00000000..d3c12075 > > --- /dev/null > > +++ b/src/splice-test.c > > @@ -0,0 +1,173 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (C) 2019 ????????????????????????????? > > + * Author: > > + * > > + * Make sure that reading and writing to a pipe via splice. > > + */ > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <sys/wait.h> > > +#include <unistd.h> > > +#include <fcntl.h> > > +#include <err.h> > > + > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <stdbool.h> > > +#include <string.h> > > +#include <errno.h> > > + > > +#define SECTOR_SIZE 512 > > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > > + > > +void read_from_pipe(int fd, const char *filename, size_t size) > > +{ > > + char buffer[SECTOR_SIZE]; > > + size_t sz; > > + ssize_t ret; > > + > > + while (size) { > > + sz = size; > > + if (sz > sizeof buffer) > > + sz = sizeof buffer; > > + ret = read(fd, buffer, sz); > > + if (ret < 0) > > + err(1, "read: %s", filename); > > + if (ret == 0) { > > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > > + exit(1); > > + } > > + size -= sz; > > + } > > +} > > + > > +void do_splice1(int fd, const char *filename, size_t size) > > +{ > > + bool retried = false; > > + int pipefd[2]; > > + > > + if (pipe(pipefd) == -1) > > + err(1, "pipe"); > > + while (size) { > > + ssize_t spliced; > > + > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > + if (spliced == -1) { > > + if (errno == EAGAIN && !retried) { > > + retried = true; > > + fprintf(stderr, "retrying splice\n"); > > + sleep(1); > > + continue; > > + } > > + err(1, "splice"); > > + } > > + read_from_pipe(pipefd[0], filename, spliced); > > + size -= spliced; > > + } > > + close(pipefd[0]); > > + close(pipefd[1]); > > +} > > + > > +void do_splice2(int fd, const char *filename, size_t size) > > +{ > > + bool retried = false; > > + int pipefd[2]; > > + int pid; > > + > > + if (pipe(pipefd) == -1) > > + err(1, "pipe"); > > + > > + pid = fork(); > > + if (pid == 0) { > > + close(pipefd[1]); > > + read_from_pipe(pipefd[0], filename, size); > > + exit(0); > > + } else { > > + close(pipefd[0]); > > + while (size) { > > + ssize_t spliced; > > + > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > + if (spliced == -1) { > > + if (errno == EAGAIN && !retried) { > > + retried = true; > > + fprintf(stderr, "retrying splice\n"); > > + sleep(1); > > + continue; > > + } > > + err(1, "splice"); > > + } > > + size -= spliced; > > + } > > + close(pipefd[1]); > > + waitpid(pid, NULL, 0); > > + } > > +} > > + > > +void usage(const char *argv0) > > +{ > > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > > + exit(2); > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + void (*do_splice)(int fd, const char *filename, size_t size); > > + const char *filename; > > + char *buffer; > > + int opt, open_flags, fd; > > + ssize_t ret; > > + > > + do_splice = do_splice1; > > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > > + > > + while ((opt = getopt(argc, argv, "rd")) != -1) { > > + switch(opt) { > > + case 'r': > > + do_splice = do_splice2; > > + break; > > + case 'd': > > + open_flags &= ~O_DIRECT; > > + break; > > + default: /* '?' */ > > + usage(argv[0]); > > + } > > + } > > + > > + if (optind >= argc) > > + usage(argv[0]); > > + filename = argv[optind]; > > + > > + printf("%s reader %s O_DIRECT\n", > > + do_splice == do_splice1 ? "sequential" : "concurrent", > > + (open_flags & O_DIRECT) ? "with" : "without"); > > + > > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > > + if (buffer == NULL) > > + err(1, "aligned_alloc"); > > + > > + fd = open(filename, open_flags, 0666); > > + if (fd == -1) > > + err(1, "open: %s", filename); > > + > > + memset(buffer, 'x', BUFFER_SIZE); > > + ret = write(fd, buffer, BUFFER_SIZE); > > + if (ret < 0) > > + err(1, "write: %s", filename); > > + if (ret != BUFFER_SIZE) { > > + fprintf(stderr, "%s: short write\n", filename); > > + exit(1); > > + } > > + > > + ret = lseek(fd, 0, SEEK_SET); > > + if (ret != 0) > > + err(1, "lseek: %s", filename); > > + > > + do_splice(fd, filename, BUFFER_SIZE); > > + > > + if (unlink(filename) == -1) > > + err(1, "unlink: %s", filename); > > + > > + return 0; > > +} > > diff --git a/tests/generic/720 b/tests/generic/720 > > new file mode 100755 > > index 00000000..b7f09c40 > > --- /dev/null > > +++ b/tests/generic/720 > > @@ -0,0 +1,41 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > > +# > > +# FS QA Test No. 720 > > +# > > +# Test using splice() to read from pipes. > > + > > +seq=`basename $0` > > +seqres=$RESULT_DIR/$seq > > +echo "QA output created by $seq" > > + > > +here=`pwd` > > +tmp=/tmp/$$ > > +status=1 # failure is the default! > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > + > > +_cleanup() > > +{ > > + cd / > > + rm -f $TEST_DIR/a > > +} > > + > > +# get standard environment, filters and checks > > +. ./common/rc > > + > > +# real QA test starts here > > +_supported_os Linux > > +_supported_fs generic > > +_require_test > > + > > +rm -f $seqres.full > > + > > +src/splice-test -r $TEST_DIR/a > > +src/splice-test -rd $TEST_DIR/a > > +src/splice-test $TEST_DIR/a > > +src/splice-test -d $TEST_DIR/a > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/generic/720.out b/tests/generic/720.out > > new file mode 100644 > > index 00000000..b0fc9935 > > --- /dev/null > > +++ b/tests/generic/720.out > > @@ -0,0 +1,7 @@ > > +QA output created by 720 > > +concurrent reader with O_DIRECT > > +concurrent reader with O_DIRECT > > +concurrent reader without O_DIRECT > > +concurrent reader without O_DIRECT > > +sequential reader with O_DIRECT > > +sequential reader without O_DIRECT > > diff --git a/tests/generic/group b/tests/generic/group > > index cd418106..f75d4e60 100644 > > --- a/tests/generic/group > > +++ b/tests/generic/group > > @@ -569,3 +569,4 @@ > > 564 auto quick copy_range > > 565 auto quick copy_range > > 719 auto quick quota metadata > > +720 auto quick rw pipe splice ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-09-02 16:56 ` Darrick J. Wong @ 2019-09-03 3:19 ` Zorro Lang 0 siblings, 0 replies; 15+ messages in thread From: Zorro Lang @ 2019-09-03 3:19 UTC (permalink / raw) To: Darrick J. Wong; +Cc: andreas.gruenbacher, xfs, viro, linux-fsdevel, fstests On Mon, Sep 02, 2019 at 09:56:07AM -0700, Darrick J. Wong wrote: > On Mon, Sep 02, 2019 at 10:20:52AM +0800, Zorro Lang wrote: > > On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > Andreas Grünbacher reports that on the two filesystems that support > > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > > a short splice) if the pipe being written to has less space available in > > > its pipe buffers than the length supplied by the calling process. > > > > > > This is a regression test to check for correct operation. > > > > > > XXX Andreas: Since you wrote the C reproducer, can you send me the > > > proper copyright and author attribution statement for the C program? > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > > Hi Darrick, > > > > I tried to add a splice_f operation into xfs_io long time ago: > > https://marc.info/?l=linux-xfs&m=155828702128047&w=2 > > > > For we can easily write more splice related test case later, I'd like to have > > a xfs_io to help that (if it can). Would you like to help to review that patch, > > if it can match your requirement to write this case? Or it can be improved to > > cover more testing conditions? I'm really appreciate that:) > > Yeah, I'll try to have a look this week. Thanks so much, I wrote this patch with 'fsstress supports splice' together, when we found a XFS regression bug by splice operation. But this xfs_io command doesn't have a specified usage to reproduce a bug. So if it can be improved to be used to cover the case which you just wrote (about splice), that might be better I think. Thanks, Zorro > > (FWIW I think Eric is backed up with dozens of xfsprogs patches...) > > --D > > > Thanks, > > Zorro > > > > > .gitignore | 1 > > > src/Makefile | 2 - > > > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > tests/generic/720 | 41 ++++++++++++ > > > tests/generic/720.out | 7 ++ > > > tests/generic/group | 1 > > > 6 files changed, 224 insertions(+), 1 deletion(-) > > > create mode 100644 src/splice-test.c > > > create mode 100755 tests/generic/720 > > > create mode 100644 tests/generic/720.out > > > > > > diff --git a/.gitignore b/.gitignore > > > index c8c815f9..26d4da11 100644 > > > --- a/.gitignore > > > +++ b/.gitignore > > > @@ -112,6 +112,7 @@ > > > /src/runas > > > /src/seek_copy_test > > > /src/seek_sanity_test > > > +/src/splice-test > > > /src/stale_handle > > > /src/stat_test > > > /src/swapon > > > diff --git a/src/Makefile b/src/Makefile > > > index c4fcf370..2920dfb1 100644 > > > --- a/src/Makefile > > > +++ b/src/Makefile > > > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > > > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > > > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > > > - fscrypt-crypt-util bulkstat_null_ocount > > > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > > > > > SUBDIRS = log-writes perf > > > > > > diff --git a/src/splice-test.c b/src/splice-test.c > > > new file mode 100644 > > > index 00000000..d3c12075 > > > --- /dev/null > > > +++ b/src/splice-test.c > > > @@ -0,0 +1,173 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +/* > > > + * Copyright (C) 2019 ????????????????????????????? > > > + * Author: > > > + * > > > + * Make sure that reading and writing to a pipe via splice. > > > + */ > > > +#include <sys/types.h> > > > +#include <sys/stat.h> > > > +#include <sys/wait.h> > > > +#include <unistd.h> > > > +#include <fcntl.h> > > > +#include <err.h> > > > + > > > +#include <stdlib.h> > > > +#include <stdio.h> > > > +#include <stdbool.h> > > > +#include <string.h> > > > +#include <errno.h> > > > + > > > +#define SECTOR_SIZE 512 > > > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > > > + > > > +void read_from_pipe(int fd, const char *filename, size_t size) > > > +{ > > > + char buffer[SECTOR_SIZE]; > > > + size_t sz; > > > + ssize_t ret; > > > + > > > + while (size) { > > > + sz = size; > > > + if (sz > sizeof buffer) > > > + sz = sizeof buffer; > > > + ret = read(fd, buffer, sz); > > > + if (ret < 0) > > > + err(1, "read: %s", filename); > > > + if (ret == 0) { > > > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > > > + exit(1); > > > + } > > > + size -= sz; > > > + } > > > +} > > > + > > > +void do_splice1(int fd, const char *filename, size_t size) > > > +{ > > > + bool retried = false; > > > + int pipefd[2]; > > > + > > > + if (pipe(pipefd) == -1) > > > + err(1, "pipe"); > > > + while (size) { > > > + ssize_t spliced; > > > + > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > + if (spliced == -1) { > > > + if (errno == EAGAIN && !retried) { > > > + retried = true; > > > + fprintf(stderr, "retrying splice\n"); > > > + sleep(1); > > > + continue; > > > + } > > > + err(1, "splice"); > > > + } > > > + read_from_pipe(pipefd[0], filename, spliced); > > > + size -= spliced; > > > + } > > > + close(pipefd[0]); > > > + close(pipefd[1]); > > > +} > > > + > > > +void do_splice2(int fd, const char *filename, size_t size) > > > +{ > > > + bool retried = false; > > > + int pipefd[2]; > > > + int pid; > > > + > > > + if (pipe(pipefd) == -1) > > > + err(1, "pipe"); > > > + > > > + pid = fork(); > > > + if (pid == 0) { > > > + close(pipefd[1]); > > > + read_from_pipe(pipefd[0], filename, size); > > > + exit(0); > > > + } else { > > > + close(pipefd[0]); > > > + while (size) { > > > + ssize_t spliced; > > > + > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > + if (spliced == -1) { > > > + if (errno == EAGAIN && !retried) { > > > + retried = true; > > > + fprintf(stderr, "retrying splice\n"); > > > + sleep(1); > > > + continue; > > > + } > > > + err(1, "splice"); > > > + } > > > + size -= spliced; > > > + } > > > + close(pipefd[1]); > > > + waitpid(pid, NULL, 0); > > > + } > > > +} > > > + > > > +void usage(const char *argv0) > > > +{ > > > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > > > + exit(2); > > > +} > > > + > > > +int main(int argc, char *argv[]) > > > +{ > > > + void (*do_splice)(int fd, const char *filename, size_t size); > > > + const char *filename; > > > + char *buffer; > > > + int opt, open_flags, fd; > > > + ssize_t ret; > > > + > > > + do_splice = do_splice1; > > > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > > > + > > > + while ((opt = getopt(argc, argv, "rd")) != -1) { > > > + switch(opt) { > > > + case 'r': > > > + do_splice = do_splice2; > > > + break; > > > + case 'd': > > > + open_flags &= ~O_DIRECT; > > > + break; > > > + default: /* '?' */ > > > + usage(argv[0]); > > > + } > > > + } > > > + > > > + if (optind >= argc) > > > + usage(argv[0]); > > > + filename = argv[optind]; > > > + > > > + printf("%s reader %s O_DIRECT\n", > > > + do_splice == do_splice1 ? "sequential" : "concurrent", > > > + (open_flags & O_DIRECT) ? "with" : "without"); > > > + > > > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > > > + if (buffer == NULL) > > > + err(1, "aligned_alloc"); > > > + > > > + fd = open(filename, open_flags, 0666); > > > + if (fd == -1) > > > + err(1, "open: %s", filename); > > > + > > > + memset(buffer, 'x', BUFFER_SIZE); > > > + ret = write(fd, buffer, BUFFER_SIZE); > > > + if (ret < 0) > > > + err(1, "write: %s", filename); > > > + if (ret != BUFFER_SIZE) { > > > + fprintf(stderr, "%s: short write\n", filename); > > > + exit(1); > > > + } > > > + > > > + ret = lseek(fd, 0, SEEK_SET); > > > + if (ret != 0) > > > + err(1, "lseek: %s", filename); > > > + > > > + do_splice(fd, filename, BUFFER_SIZE); > > > + > > > + if (unlink(filename) == -1) > > > + err(1, "unlink: %s", filename); > > > + > > > + return 0; > > > +} > > > diff --git a/tests/generic/720 b/tests/generic/720 > > > new file mode 100755 > > > index 00000000..b7f09c40 > > > --- /dev/null > > > +++ b/tests/generic/720 > > > @@ -0,0 +1,41 @@ > > > +#! /bin/bash > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > > > +# > > > +# FS QA Test No. 720 > > > +# > > > +# Test using splice() to read from pipes. > > > + > > > +seq=`basename $0` > > > +seqres=$RESULT_DIR/$seq > > > +echo "QA output created by $seq" > > > + > > > +here=`pwd` > > > +tmp=/tmp/$$ > > > +status=1 # failure is the default! > > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > > + > > > +_cleanup() > > > +{ > > > + cd / > > > + rm -f $TEST_DIR/a > > > +} > > > + > > > +# get standard environment, filters and checks > > > +. ./common/rc > > > + > > > +# real QA test starts here > > > +_supported_os Linux > > > +_supported_fs generic > > > +_require_test > > > + > > > +rm -f $seqres.full > > > + > > > +src/splice-test -r $TEST_DIR/a > > > +src/splice-test -rd $TEST_DIR/a > > > +src/splice-test $TEST_DIR/a > > > +src/splice-test -d $TEST_DIR/a > > > + > > > +# success, all done > > > +status=0 > > > +exit > > > diff --git a/tests/generic/720.out b/tests/generic/720.out > > > new file mode 100644 > > > index 00000000..b0fc9935 > > > --- /dev/null > > > +++ b/tests/generic/720.out > > > @@ -0,0 +1,7 @@ > > > +QA output created by 720 > > > +concurrent reader with O_DIRECT > > > +concurrent reader with O_DIRECT > > > +concurrent reader without O_DIRECT > > > +concurrent reader without O_DIRECT > > > +sequential reader with O_DIRECT > > > +sequential reader without O_DIRECT > > > diff --git a/tests/generic/group b/tests/generic/group > > > index cd418106..f75d4e60 100644 > > > --- a/tests/generic/group > > > +++ b/tests/generic/group > > > @@ -569,3 +569,4 @@ > > > 564 auto quick copy_range > > > 565 auto quick copy_range > > > 719 auto quick quota metadata > > > +720 auto quick rw pipe splice ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-08-30 0:44 ` [RFC PATCH] generic: test splice() with pipes Darrick J. Wong 2019-09-02 2:20 ` Zorro Lang @ 2019-11-21 17:01 ` Darrick J. Wong 2019-11-21 18:48 ` Andreas Grünbacher 1 sibling, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-11-21 17:01 UTC (permalink / raw) To: andreas.gruenbacher; +Cc: xfs, viro, linux-fsdevel, fstests On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Andreas Grünbacher reports that on the two filesystems that support > iomap directio, it's possible for splice() to return -EAGAIN (instead of > a short splice) if the pipe being written to has less space available in > its pipe buffers than the length supplied by the calling process. > > This is a regression test to check for correct operation. > > XXX Andreas: Since you wrote the C reproducer, can you send me the > proper copyright and author attribution statement for the C program? Ping? Andreas, can I get the above info so I can get this moving again? --D > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > .gitignore | 1 > src/Makefile | 2 - > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/720 | 41 ++++++++++++ > tests/generic/720.out | 7 ++ > tests/generic/group | 1 > 6 files changed, 224 insertions(+), 1 deletion(-) > create mode 100644 src/splice-test.c > create mode 100755 tests/generic/720 > create mode 100644 tests/generic/720.out > > diff --git a/.gitignore b/.gitignore > index c8c815f9..26d4da11 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -112,6 +112,7 @@ > /src/runas > /src/seek_copy_test > /src/seek_sanity_test > +/src/splice-test > /src/stale_handle > /src/stat_test > /src/swapon > diff --git a/src/Makefile b/src/Makefile > index c4fcf370..2920dfb1 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > - fscrypt-crypt-util bulkstat_null_ocount > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > SUBDIRS = log-writes perf > > diff --git a/src/splice-test.c b/src/splice-test.c > new file mode 100644 > index 00000000..d3c12075 > --- /dev/null > +++ b/src/splice-test.c > @@ -0,0 +1,173 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 ????????????????????????????? > + * Author: > + * > + * Make sure that reading and writing to a pipe via splice. > + */ > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/wait.h> > +#include <unistd.h> > +#include <fcntl.h> > +#include <err.h> > + > +#include <stdlib.h> > +#include <stdio.h> > +#include <stdbool.h> > +#include <string.h> > +#include <errno.h> > + > +#define SECTOR_SIZE 512 > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > + > +void read_from_pipe(int fd, const char *filename, size_t size) > +{ > + char buffer[SECTOR_SIZE]; > + size_t sz; > + ssize_t ret; > + > + while (size) { > + sz = size; > + if (sz > sizeof buffer) > + sz = sizeof buffer; > + ret = read(fd, buffer, sz); > + if (ret < 0) > + err(1, "read: %s", filename); > + if (ret == 0) { > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > + exit(1); > + } > + size -= sz; > + } > +} > + > +void do_splice1(int fd, const char *filename, size_t size) > +{ > + bool retried = false; > + int pipefd[2]; > + > + if (pipe(pipefd) == -1) > + err(1, "pipe"); > + while (size) { > + ssize_t spliced; > + > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > + if (spliced == -1) { > + if (errno == EAGAIN && !retried) { > + retried = true; > + fprintf(stderr, "retrying splice\n"); > + sleep(1); > + continue; > + } > + err(1, "splice"); > + } > + read_from_pipe(pipefd[0], filename, spliced); > + size -= spliced; > + } > + close(pipefd[0]); > + close(pipefd[1]); > +} > + > +void do_splice2(int fd, const char *filename, size_t size) > +{ > + bool retried = false; > + int pipefd[2]; > + int pid; > + > + if (pipe(pipefd) == -1) > + err(1, "pipe"); > + > + pid = fork(); > + if (pid == 0) { > + close(pipefd[1]); > + read_from_pipe(pipefd[0], filename, size); > + exit(0); > + } else { > + close(pipefd[0]); > + while (size) { > + ssize_t spliced; > + > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > + if (spliced == -1) { > + if (errno == EAGAIN && !retried) { > + retried = true; > + fprintf(stderr, "retrying splice\n"); > + sleep(1); > + continue; > + } > + err(1, "splice"); > + } > + size -= spliced; > + } > + close(pipefd[1]); > + waitpid(pid, NULL, 0); > + } > +} > + > +void usage(const char *argv0) > +{ > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > + exit(2); > +} > + > +int main(int argc, char *argv[]) > +{ > + void (*do_splice)(int fd, const char *filename, size_t size); > + const char *filename; > + char *buffer; > + int opt, open_flags, fd; > + ssize_t ret; > + > + do_splice = do_splice1; > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > + > + while ((opt = getopt(argc, argv, "rd")) != -1) { > + switch(opt) { > + case 'r': > + do_splice = do_splice2; > + break; > + case 'd': > + open_flags &= ~O_DIRECT; > + break; > + default: /* '?' */ > + usage(argv[0]); > + } > + } > + > + if (optind >= argc) > + usage(argv[0]); > + filename = argv[optind]; > + > + printf("%s reader %s O_DIRECT\n", > + do_splice == do_splice1 ? "sequential" : "concurrent", > + (open_flags & O_DIRECT) ? "with" : "without"); > + > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > + if (buffer == NULL) > + err(1, "aligned_alloc"); > + > + fd = open(filename, open_flags, 0666); > + if (fd == -1) > + err(1, "open: %s", filename); > + > + memset(buffer, 'x', BUFFER_SIZE); > + ret = write(fd, buffer, BUFFER_SIZE); > + if (ret < 0) > + err(1, "write: %s", filename); > + if (ret != BUFFER_SIZE) { > + fprintf(stderr, "%s: short write\n", filename); > + exit(1); > + } > + > + ret = lseek(fd, 0, SEEK_SET); > + if (ret != 0) > + err(1, "lseek: %s", filename); > + > + do_splice(fd, filename, BUFFER_SIZE); > + > + if (unlink(filename) == -1) > + err(1, "unlink: %s", filename); > + > + return 0; > +} > diff --git a/tests/generic/720 b/tests/generic/720 > new file mode 100755 > index 00000000..b7f09c40 > --- /dev/null > +++ b/tests/generic/720 > @@ -0,0 +1,41 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > +# > +# FS QA Test No. 720 > +# > +# Test using splice() to read from pipes. > + > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $TEST_DIR/a > +} > + > +# get standard environment, filters and checks > +. ./common/rc > + > +# real QA test starts here > +_supported_os Linux > +_supported_fs generic > +_require_test > + > +rm -f $seqres.full > + > +src/splice-test -r $TEST_DIR/a > +src/splice-test -rd $TEST_DIR/a > +src/splice-test $TEST_DIR/a > +src/splice-test -d $TEST_DIR/a > + > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/720.out b/tests/generic/720.out > new file mode 100644 > index 00000000..b0fc9935 > --- /dev/null > +++ b/tests/generic/720.out > @@ -0,0 +1,7 @@ > +QA output created by 720 > +concurrent reader with O_DIRECT > +concurrent reader with O_DIRECT > +concurrent reader without O_DIRECT > +concurrent reader without O_DIRECT > +sequential reader with O_DIRECT > +sequential reader without O_DIRECT > diff --git a/tests/generic/group b/tests/generic/group > index cd418106..f75d4e60 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -569,3 +569,4 @@ > 564 auto quick copy_range > 565 auto quick copy_range > 719 auto quick quota metadata > +720 auto quick rw pipe splice ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-11-21 17:01 ` Darrick J. Wong @ 2019-11-21 18:48 ` Andreas Grünbacher 2019-11-21 19:14 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Andreas Grünbacher @ 2019-11-21 18:48 UTC (permalink / raw) To: Darrick J. Wong; +Cc: xfs, Al Viro, linux-fsdevel, fstests Am Do., 21. Nov. 2019 um 18:01 Uhr schrieb Darrick J. Wong <darrick.wong@oracle.com>: > On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Andreas Grünbacher reports that on the two filesystems that support > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > a short splice) if the pipe being written to has less space available in > > its pipe buffers than the length supplied by the calling process. > > > > This is a regression test to check for correct operation. > > > > XXX Andreas: Since you wrote the C reproducer, can you send me the > > proper copyright and author attribution statement for the C program? > > Ping? Andreas, can I get the above info so I can get this moving again? Oops, sure, this is: Copyright (c) 2019 RedHat Inc. All Rights Reserved. Author: Andreas Gruenbacher <agruenba@redhat.com> > --D > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > .gitignore | 1 > > src/Makefile | 2 - > > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > > tests/generic/720 | 41 ++++++++++++ > > tests/generic/720.out | 7 ++ > > tests/generic/group | 1 > > 6 files changed, 224 insertions(+), 1 deletion(-) > > create mode 100644 src/splice-test.c > > create mode 100755 tests/generic/720 > > create mode 100644 tests/generic/720.out > > > > diff --git a/.gitignore b/.gitignore > > index c8c815f9..26d4da11 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -112,6 +112,7 @@ > > /src/runas > > /src/seek_copy_test > > /src/seek_sanity_test > > +/src/splice-test > > /src/stale_handle > > /src/stat_test > > /src/swapon > > diff --git a/src/Makefile b/src/Makefile > > index c4fcf370..2920dfb1 100644 > > --- a/src/Makefile > > +++ b/src/Makefile > > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > > - fscrypt-crypt-util bulkstat_null_ocount > > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > > > SUBDIRS = log-writes perf > > > > diff --git a/src/splice-test.c b/src/splice-test.c > > new file mode 100644 > > index 00000000..d3c12075 > > --- /dev/null > > +++ b/src/splice-test.c > > @@ -0,0 +1,173 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (C) 2019 ???????????????????????????? > > + * Author: > > + * > > + * Make sure that reading and writing to a pipe via splice. > > + */ > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <sys/wait.h> > > +#include <unistd.h> > > +#include <fcntl.h> > > +#include <err.h> > > + > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <stdbool.h> > > +#include <string.h> > > +#include <errno.h> > > + > > +#define SECTOR_SIZE 512 > > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > > + > > +void read_from_pipe(int fd, const char *filename, size_t size) > > +{ > > + char buffer[SECTOR_SIZE]; > > + size_t sz; > > + ssize_t ret; > > + > > + while (size) { > > + sz = size; > > + if (sz > sizeof buffer) > > + sz = sizeof buffer; > > + ret = read(fd, buffer, sz); > > + if (ret < 0) > > + err(1, "read: %s", filename); > > + if (ret == 0) { > > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > > + exit(1); > > + } > > + size -= sz; > > + } > > +} > > + > > +void do_splice1(int fd, const char *filename, size_t size) > > +{ > > + bool retried = false; > > + int pipefd[2]; > > + > > + if (pipe(pipefd) == -1) > > + err(1, "pipe"); > > + while (size) { > > + ssize_t spliced; > > + > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > + if (spliced == -1) { > > + if (errno == EAGAIN && !retried) { > > + retried = true; > > + fprintf(stderr, "retrying splice\n"); > > + sleep(1); > > + continue; > > + } > > + err(1, "splice"); > > + } > > + read_from_pipe(pipefd[0], filename, spliced); > > + size -= spliced; > > + } > > + close(pipefd[0]); > > + close(pipefd[1]); > > +} > > + > > +void do_splice2(int fd, const char *filename, size_t size) > > +{ > > + bool retried = false; > > + int pipefd[2]; > > + int pid; > > + > > + if (pipe(pipefd) == -1) > > + err(1, "pipe"); > > + > > + pid = fork(); > > + if (pid == 0) { > > + close(pipefd[1]); > > + read_from_pipe(pipefd[0], filename, size); > > + exit(0); > > + } else { > > + close(pipefd[0]); > > + while (size) { > > + ssize_t spliced; > > + > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > + if (spliced == -1) { > > + if (errno == EAGAIN && !retried) { > > + retried = true; > > + fprintf(stderr, "retrying splice\n"); > > + sleep(1); > > + continue; > > + } > > + err(1, "splice"); > > + } > > + size -= spliced; > > + } > > + close(pipefd[1]); > > + waitpid(pid, NULL, 0); > > + } > > +} > > + > > +void usage(const char *argv0) > > +{ > > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > > + exit(2); > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + void (*do_splice)(int fd, const char *filename, size_t size); > > + const char *filename; > > + char *buffer; > > + int opt, open_flags, fd; > > + ssize_t ret; > > + > > + do_splice = do_splice1; > > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > > + > > + while ((opt = getopt(argc, argv, "rd")) != -1) { > > + switch(opt) { > > + case 'r': > > + do_splice = do_splice2; > > + break; > > + case 'd': > > + open_flags &= ~O_DIRECT; > > + break; > > + default: /* '?' */ > > + usage(argv[0]); > > + } > > + } > > + > > + if (optind >= argc) > > + usage(argv[0]); > > + filename = argv[optind]; > > + > > + printf("%s reader %s O_DIRECT\n", > > + do_splice == do_splice1 ? "sequential" : "concurrent", > > + (open_flags & O_DIRECT) ? "with" : "without"); > > + > > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > > + if (buffer == NULL) > > + err(1, "aligned_alloc"); > > + > > + fd = open(filename, open_flags, 0666); > > + if (fd == -1) > > + err(1, "open: %s", filename); > > + > > + memset(buffer, 'x', BUFFER_SIZE); > > + ret = write(fd, buffer, BUFFER_SIZE); > > + if (ret < 0) > > + err(1, "write: %s", filename); > > + if (ret != BUFFER_SIZE) { > > + fprintf(stderr, "%s: short write\n", filename); > > + exit(1); > > + } > > + > > + ret = lseek(fd, 0, SEEK_SET); > > + if (ret != 0) > > + err(1, "lseek: %s", filename); > > + > > + do_splice(fd, filename, BUFFER_SIZE); > > + > > + if (unlink(filename) == -1) > > + err(1, "unlink: %s", filename); > > + > > + return 0; > > +} > > diff --git a/tests/generic/720 b/tests/generic/720 > > new file mode 100755 > > index 00000000..b7f09c40 > > --- /dev/null > > +++ b/tests/generic/720 > > @@ -0,0 +1,41 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > > +# > > +# FS QA Test No. 720 > > +# > > +# Test using splice() to read from pipes. > > + > > +seq=`basename $0` > > +seqres=$RESULT_DIR/$seq > > +echo "QA output created by $seq" > > + > > +here=`pwd` > > +tmp=/tmp/$$ > > +status=1 # failure is the default! > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > + > > +_cleanup() > > +{ > > + cd / > > + rm -f $TEST_DIR/a > > +} > > + > > +# get standard environment, filters and checks > > +. ./common/rc > > + > > +# real QA test starts here > > +_supported_os Linux > > +_supported_fs generic > > +_require_test > > + > > +rm -f $seqres.full > > + > > +src/splice-test -r $TEST_DIR/a > > +src/splice-test -rd $TEST_DIR/a > > +src/splice-test $TEST_DIR/a > > +src/splice-test -d $TEST_DIR/a > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/generic/720.out b/tests/generic/720.out > > new file mode 100644 > > index 00000000..b0fc9935 > > --- /dev/null > > +++ b/tests/generic/720.out > > @@ -0,0 +1,7 @@ > > +QA output created by 720 > > +concurrent reader with O_DIRECT > > +concurrent reader with O_DIRECT > > +concurrent reader without O_DIRECT > > +concurrent reader without O_DIRECT > > +sequential reader with O_DIRECT > > +sequential reader without O_DIRECT > > diff --git a/tests/generic/group b/tests/generic/group > > index cd418106..f75d4e60 100644 > > --- a/tests/generic/group > > +++ b/tests/generic/group > > @@ -569,3 +569,4 @@ > > 564 auto quick copy_range > > 565 auto quick copy_range > > 719 auto quick quota metadata > > +720 auto quick rw pipe splice Thanks, Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-11-21 18:48 ` Andreas Grünbacher @ 2019-11-21 19:14 ` Darrick J. Wong 2019-11-22 1:27 ` Andreas Grünbacher 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-11-21 19:14 UTC (permalink / raw) To: Andreas Grünbacher; +Cc: xfs, Al Viro, linux-fsdevel, fstests On Thu, Nov 21, 2019 at 07:48:54PM +0100, Andreas Grünbacher wrote: > Am Do., 21. Nov. 2019 um 18:01 Uhr schrieb Darrick J. Wong > <darrick.wong@oracle.com>: > > On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > Andreas Grünbacher reports that on the two filesystems that support > > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > > a short splice) if the pipe being written to has less space available in > > > its pipe buffers than the length supplied by the calling process. > > > > > > This is a regression test to check for correct operation. > > > > > > XXX Andreas: Since you wrote the C reproducer, can you send me the > > > proper copyright and author attribution statement for the C program? > > > > Ping? Andreas, can I get the above info so I can get this moving again? > > Oops, sure, this is: > > Copyright (c) 2019 RedHat Inc. All Rights Reserved. > Author: Andreas Gruenbacher <agruenba@redhat.com> Ok thanks. It's appropriate to tag it as GPL v2 licensed, correct? --D > > --D > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > .gitignore | 1 > > > src/Makefile | 2 - > > > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > tests/generic/720 | 41 ++++++++++++ > > > tests/generic/720.out | 7 ++ > > > tests/generic/group | 1 > > > 6 files changed, 224 insertions(+), 1 deletion(-) > > > create mode 100644 src/splice-test.c > > > create mode 100755 tests/generic/720 > > > create mode 100644 tests/generic/720.out > > > > > > diff --git a/.gitignore b/.gitignore > > > index c8c815f9..26d4da11 100644 > > > --- a/.gitignore > > > +++ b/.gitignore > > > @@ -112,6 +112,7 @@ > > > /src/runas > > > /src/seek_copy_test > > > /src/seek_sanity_test > > > +/src/splice-test > > > /src/stale_handle > > > /src/stat_test > > > /src/swapon > > > diff --git a/src/Makefile b/src/Makefile > > > index c4fcf370..2920dfb1 100644 > > > --- a/src/Makefile > > > +++ b/src/Makefile > > > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > > > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > > > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > > > - fscrypt-crypt-util bulkstat_null_ocount > > > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > > > > > SUBDIRS = log-writes perf > > > > > > diff --git a/src/splice-test.c b/src/splice-test.c > > > new file mode 100644 > > > index 00000000..d3c12075 > > > --- /dev/null > > > +++ b/src/splice-test.c > > > @@ -0,0 +1,173 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +/* > > > + * Copyright (C) 2019 ???????????????????????????? > > > + * Author: > > > + * > > > + * Make sure that reading and writing to a pipe via splice. > > > + */ > > > +#include <sys/types.h> > > > +#include <sys/stat.h> > > > +#include <sys/wait.h> > > > +#include <unistd.h> > > > +#include <fcntl.h> > > > +#include <err.h> > > > + > > > +#include <stdlib.h> > > > +#include <stdio.h> > > > +#include <stdbool.h> > > > +#include <string.h> > > > +#include <errno.h> > > > + > > > +#define SECTOR_SIZE 512 > > > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > > > + > > > +void read_from_pipe(int fd, const char *filename, size_t size) > > > +{ > > > + char buffer[SECTOR_SIZE]; > > > + size_t sz; > > > + ssize_t ret; > > > + > > > + while (size) { > > > + sz = size; > > > + if (sz > sizeof buffer) > > > + sz = sizeof buffer; > > > + ret = read(fd, buffer, sz); > > > + if (ret < 0) > > > + err(1, "read: %s", filename); > > > + if (ret == 0) { > > > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > > > + exit(1); > > > + } > > > + size -= sz; > > > + } > > > +} > > > + > > > +void do_splice1(int fd, const char *filename, size_t size) > > > +{ > > > + bool retried = false; > > > + int pipefd[2]; > > > + > > > + if (pipe(pipefd) == -1) > > > + err(1, "pipe"); > > > + while (size) { > > > + ssize_t spliced; > > > + > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > + if (spliced == -1) { > > > + if (errno == EAGAIN && !retried) { > > > + retried = true; > > > + fprintf(stderr, "retrying splice\n"); > > > + sleep(1); > > > + continue; > > > + } > > > + err(1, "splice"); > > > + } > > > + read_from_pipe(pipefd[0], filename, spliced); > > > + size -= spliced; > > > + } > > > + close(pipefd[0]); > > > + close(pipefd[1]); > > > +} > > > + > > > +void do_splice2(int fd, const char *filename, size_t size) > > > +{ > > > + bool retried = false; > > > + int pipefd[2]; > > > + int pid; > > > + > > > + if (pipe(pipefd) == -1) > > > + err(1, "pipe"); > > > + > > > + pid = fork(); > > > + if (pid == 0) { > > > + close(pipefd[1]); > > > + read_from_pipe(pipefd[0], filename, size); > > > + exit(0); > > > + } else { > > > + close(pipefd[0]); > > > + while (size) { > > > + ssize_t spliced; > > > + > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > + if (spliced == -1) { > > > + if (errno == EAGAIN && !retried) { > > > + retried = true; > > > + fprintf(stderr, "retrying splice\n"); > > > + sleep(1); > > > + continue; > > > + } > > > + err(1, "splice"); > > > + } > > > + size -= spliced; > > > + } > > > + close(pipefd[1]); > > > + waitpid(pid, NULL, 0); > > > + } > > > +} > > > + > > > +void usage(const char *argv0) > > > +{ > > > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > > > + exit(2); > > > +} > > > + > > > +int main(int argc, char *argv[]) > > > +{ > > > + void (*do_splice)(int fd, const char *filename, size_t size); > > > + const char *filename; > > > + char *buffer; > > > + int opt, open_flags, fd; > > > + ssize_t ret; > > > + > > > + do_splice = do_splice1; > > > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > > > + > > > + while ((opt = getopt(argc, argv, "rd")) != -1) { > > > + switch(opt) { > > > + case 'r': > > > + do_splice = do_splice2; > > > + break; > > > + case 'd': > > > + open_flags &= ~O_DIRECT; > > > + break; > > > + default: /* '?' */ > > > + usage(argv[0]); > > > + } > > > + } > > > + > > > + if (optind >= argc) > > > + usage(argv[0]); > > > + filename = argv[optind]; > > > + > > > + printf("%s reader %s O_DIRECT\n", > > > + do_splice == do_splice1 ? "sequential" : "concurrent", > > > + (open_flags & O_DIRECT) ? "with" : "without"); > > > + > > > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > > > + if (buffer == NULL) > > > + err(1, "aligned_alloc"); > > > + > > > + fd = open(filename, open_flags, 0666); > > > + if (fd == -1) > > > + err(1, "open: %s", filename); > > > + > > > + memset(buffer, 'x', BUFFER_SIZE); > > > + ret = write(fd, buffer, BUFFER_SIZE); > > > + if (ret < 0) > > > + err(1, "write: %s", filename); > > > + if (ret != BUFFER_SIZE) { > > > + fprintf(stderr, "%s: short write\n", filename); > > > + exit(1); > > > + } > > > + > > > + ret = lseek(fd, 0, SEEK_SET); > > > + if (ret != 0) > > > + err(1, "lseek: %s", filename); > > > + > > > + do_splice(fd, filename, BUFFER_SIZE); > > > + > > > + if (unlink(filename) == -1) > > > + err(1, "unlink: %s", filename); > > > + > > > + return 0; > > > +} > > > diff --git a/tests/generic/720 b/tests/generic/720 > > > new file mode 100755 > > > index 00000000..b7f09c40 > > > --- /dev/null > > > +++ b/tests/generic/720 > > > @@ -0,0 +1,41 @@ > > > +#! /bin/bash > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > > > +# > > > +# FS QA Test No. 720 > > > +# > > > +# Test using splice() to read from pipes. > > > + > > > +seq=`basename $0` > > > +seqres=$RESULT_DIR/$seq > > > +echo "QA output created by $seq" > > > + > > > +here=`pwd` > > > +tmp=/tmp/$$ > > > +status=1 # failure is the default! > > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > > + > > > +_cleanup() > > > +{ > > > + cd / > > > + rm -f $TEST_DIR/a > > > +} > > > + > > > +# get standard environment, filters and checks > > > +. ./common/rc > > > + > > > +# real QA test starts here > > > +_supported_os Linux > > > +_supported_fs generic > > > +_require_test > > > + > > > +rm -f $seqres.full > > > + > > > +src/splice-test -r $TEST_DIR/a > > > +src/splice-test -rd $TEST_DIR/a > > > +src/splice-test $TEST_DIR/a > > > +src/splice-test -d $TEST_DIR/a > > > + > > > +# success, all done > > > +status=0 > > > +exit > > > diff --git a/tests/generic/720.out b/tests/generic/720.out > > > new file mode 100644 > > > index 00000000..b0fc9935 > > > --- /dev/null > > > +++ b/tests/generic/720.out > > > @@ -0,0 +1,7 @@ > > > +QA output created by 720 > > > +concurrent reader with O_DIRECT > > > +concurrent reader with O_DIRECT > > > +concurrent reader without O_DIRECT > > > +concurrent reader without O_DIRECT > > > +sequential reader with O_DIRECT > > > +sequential reader without O_DIRECT > > > diff --git a/tests/generic/group b/tests/generic/group > > > index cd418106..f75d4e60 100644 > > > --- a/tests/generic/group > > > +++ b/tests/generic/group > > > @@ -569,3 +569,4 @@ > > > 564 auto quick copy_range > > > 565 auto quick copy_range > > > 719 auto quick quota metadata > > > +720 auto quick rw pipe splice > > Thanks, > Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH] generic: test splice() with pipes 2019-11-21 19:14 ` Darrick J. Wong @ 2019-11-22 1:27 ` Andreas Grünbacher 0 siblings, 0 replies; 15+ messages in thread From: Andreas Grünbacher @ 2019-11-22 1:27 UTC (permalink / raw) To: Darrick J. Wong; +Cc: xfs, Al Viro, linux-fsdevel, fstests Am Do., 21. Nov. 2019 um 20:14 Uhr schrieb Darrick J. Wong <darrick.wong@oracle.com>: > On Thu, Nov 21, 2019 at 07:48:54PM +0100, Andreas Grünbacher wrote: > > Am Do., 21. Nov. 2019 um 18:01 Uhr schrieb Darrick J. Wong > > <darrick.wong@oracle.com>: > > > On Thu, Aug 29, 2019 at 05:44:07PM -0700, Darrick J. Wong wrote: > > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > > > Andreas Grünbacher reports that on the two filesystems that support > > > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > > > a short splice) if the pipe being written to has less space available in > > > > its pipe buffers than the length supplied by the calling process. > > > > > > > > This is a regression test to check for correct operation. > > > > > > > > XXX Andreas: Since you wrote the C reproducer, can you send me the > > > > proper copyright and author attribution statement for the C program? > > > > > > Ping? Andreas, can I get the above info so I can get this moving again? > > > > Oops, sure, this is: > > > > Copyright (c) 2019 RedHat Inc. All Rights Reserved. > > Author: Andreas Gruenbacher <agruenba@redhat.com> > > Ok thanks. It's appropriate to tag it as GPL v2 licensed, correct? My prevous reply didn't quite make it, so again, yes, that's correct. Thanks, Andreas > --D > > > > --D > > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > > --- > > > > .gitignore | 1 > > > > src/Makefile | 2 - > > > > src/splice-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > > tests/generic/720 | 41 ++++++++++++ > > > > tests/generic/720.out | 7 ++ > > > > tests/generic/group | 1 > > > > 6 files changed, 224 insertions(+), 1 deletion(-) > > > > create mode 100644 src/splice-test.c > > > > create mode 100755 tests/generic/720 > > > > create mode 100644 tests/generic/720.out > > > > > > > > diff --git a/.gitignore b/.gitignore > > > > index c8c815f9..26d4da11 100644 > > > > --- a/.gitignore > > > > +++ b/.gitignore > > > > @@ -112,6 +112,7 @@ > > > > /src/runas > > > > /src/seek_copy_test > > > > /src/seek_sanity_test > > > > +/src/splice-test > > > > /src/stale_handle > > > > /src/stat_test > > > > /src/swapon > > > > diff --git a/src/Makefile b/src/Makefile > > > > index c4fcf370..2920dfb1 100644 > > > > --- a/src/Makefile > > > > +++ b/src/Makefile > > > > @@ -28,7 +28,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \ > > > > attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \ > > > > dio-invalidate-cache stat_test t_encrypted_d_revalidate \ > > > > attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \ > > > > - fscrypt-crypt-util bulkstat_null_ocount > > > > + fscrypt-crypt-util bulkstat_null_ocount splice-test > > > > > > > > SUBDIRS = log-writes perf > > > > > > > > diff --git a/src/splice-test.c b/src/splice-test.c > > > > new file mode 100644 > > > > index 00000000..d3c12075 > > > > --- /dev/null > > > > +++ b/src/splice-test.c > > > > @@ -0,0 +1,173 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > > +/* > > > > + * Copyright (C) 2019 ???????????????????????????? > > > > + * Author: > > > > + * > > > > + * Make sure that reading and writing to a pipe via splice. > > > > + */ > > > > +#include <sys/types.h> > > > > +#include <sys/stat.h> > > > > +#include <sys/wait.h> > > > > +#include <unistd.h> > > > > +#include <fcntl.h> > > > > +#include <err.h> > > > > + > > > > +#include <stdlib.h> > > > > +#include <stdio.h> > > > > +#include <stdbool.h> > > > > +#include <string.h> > > > > +#include <errno.h> > > > > + > > > > +#define SECTOR_SIZE 512 > > > > +#define BUFFER_SIZE (150 * SECTOR_SIZE) > > > > + > > > > +void read_from_pipe(int fd, const char *filename, size_t size) > > > > +{ > > > > + char buffer[SECTOR_SIZE]; > > > > + size_t sz; > > > > + ssize_t ret; > > > > + > > > > + while (size) { > > > > + sz = size; > > > > + if (sz > sizeof buffer) > > > > + sz = sizeof buffer; > > > > + ret = read(fd, buffer, sz); > > > > + if (ret < 0) > > > > + err(1, "read: %s", filename); > > > > + if (ret == 0) { > > > > + fprintf(stderr, "read: %s: unexpected EOF\n", filename); > > > > + exit(1); > > > > + } > > > > + size -= sz; > > > > + } > > > > +} > > > > + > > > > +void do_splice1(int fd, const char *filename, size_t size) > > > > +{ > > > > + bool retried = false; > > > > + int pipefd[2]; > > > > + > > > > + if (pipe(pipefd) == -1) > > > > + err(1, "pipe"); > > > > + while (size) { > > > > + ssize_t spliced; > > > > + > > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > > + if (spliced == -1) { > > > > + if (errno == EAGAIN && !retried) { > > > > + retried = true; > > > > + fprintf(stderr, "retrying splice\n"); > > > > + sleep(1); > > > > + continue; > > > > + } > > > > + err(1, "splice"); > > > > + } > > > > + read_from_pipe(pipefd[0], filename, spliced); > > > > + size -= spliced; > > > > + } > > > > + close(pipefd[0]); > > > > + close(pipefd[1]); > > > > +} > > > > + > > > > +void do_splice2(int fd, const char *filename, size_t size) > > > > +{ > > > > + bool retried = false; > > > > + int pipefd[2]; > > > > + int pid; > > > > + > > > > + if (pipe(pipefd) == -1) > > > > + err(1, "pipe"); > > > > + > > > > + pid = fork(); > > > > + if (pid == 0) { > > > > + close(pipefd[1]); > > > > + read_from_pipe(pipefd[0], filename, size); > > > > + exit(0); > > > > + } else { > > > > + close(pipefd[0]); > > > > + while (size) { > > > > + ssize_t spliced; > > > > + > > > > + spliced = splice(fd, NULL, pipefd[1], NULL, size, SPLICE_F_MOVE); > > > > + if (spliced == -1) { > > > > + if (errno == EAGAIN && !retried) { > > > > + retried = true; > > > > + fprintf(stderr, "retrying splice\n"); > > > > + sleep(1); > > > > + continue; > > > > + } > > > > + err(1, "splice"); > > > > + } > > > > + size -= spliced; > > > > + } > > > > + close(pipefd[1]); > > > > + waitpid(pid, NULL, 0); > > > > + } > > > > +} > > > > + > > > > +void usage(const char *argv0) > > > > +{ > > > > + fprintf(stderr, "USAGE: %s [-rd] {filename}\n", basename(argv0)); > > > > + exit(2); > > > > +} > > > > + > > > > +int main(int argc, char *argv[]) > > > > +{ > > > > + void (*do_splice)(int fd, const char *filename, size_t size); > > > > + const char *filename; > > > > + char *buffer; > > > > + int opt, open_flags, fd; > > > > + ssize_t ret; > > > > + > > > > + do_splice = do_splice1; > > > > + open_flags = O_CREAT | O_TRUNC | O_RDWR | O_DIRECT; > > > > + > > > > + while ((opt = getopt(argc, argv, "rd")) != -1) { > > > > + switch(opt) { > > > > + case 'r': > > > > + do_splice = do_splice2; > > > > + break; > > > > + case 'd': > > > > + open_flags &= ~O_DIRECT; > > > > + break; > > > > + default: /* '?' */ > > > > + usage(argv[0]); > > > > + } > > > > + } > > > > + > > > > + if (optind >= argc) > > > > + usage(argv[0]); > > > > + filename = argv[optind]; > > > > + > > > > + printf("%s reader %s O_DIRECT\n", > > > > + do_splice == do_splice1 ? "sequential" : "concurrent", > > > > + (open_flags & O_DIRECT) ? "with" : "without"); > > > > + > > > > + buffer = aligned_alloc(SECTOR_SIZE, BUFFER_SIZE); > > > > + if (buffer == NULL) > > > > + err(1, "aligned_alloc"); > > > > + > > > > + fd = open(filename, open_flags, 0666); > > > > + if (fd == -1) > > > > + err(1, "open: %s", filename); > > > > + > > > > + memset(buffer, 'x', BUFFER_SIZE); > > > > + ret = write(fd, buffer, BUFFER_SIZE); > > > > + if (ret < 0) > > > > + err(1, "write: %s", filename); > > > > + if (ret != BUFFER_SIZE) { > > > > + fprintf(stderr, "%s: short write\n", filename); > > > > + exit(1); > > > > + } > > > > + > > > > + ret = lseek(fd, 0, SEEK_SET); > > > > + if (ret != 0) > > > > + err(1, "lseek: %s", filename); > > > > + > > > > + do_splice(fd, filename, BUFFER_SIZE); > > > > + > > > > + if (unlink(filename) == -1) > > > > + err(1, "unlink: %s", filename); > > > > + > > > > + return 0; > > > > +} > > > > diff --git a/tests/generic/720 b/tests/generic/720 > > > > new file mode 100755 > > > > index 00000000..b7f09c40 > > > > --- /dev/null > > > > +++ b/tests/generic/720 > > > > @@ -0,0 +1,41 @@ > > > > +#! /bin/bash > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > +# Copyright (c) 2019, Oracle and/or its affiliates. All Rights Reserved. > > > > +# > > > > +# FS QA Test No. 720 > > > > +# > > > > +# Test using splice() to read from pipes. > > > > + > > > > +seq=`basename $0` > > > > +seqres=$RESULT_DIR/$seq > > > > +echo "QA output created by $seq" > > > > + > > > > +here=`pwd` > > > > +tmp=/tmp/$$ > > > > +status=1 # failure is the default! > > > > +trap "_cleanup; exit \$status" 0 1 2 3 15 > > > > + > > > > +_cleanup() > > > > +{ > > > > + cd / > > > > + rm -f $TEST_DIR/a > > > > +} > > > > + > > > > +# get standard environment, filters and checks > > > > +. ./common/rc > > > > + > > > > +# real QA test starts here > > > > +_supported_os Linux > > > > +_supported_fs generic > > > > +_require_test > > > > + > > > > +rm -f $seqres.full > > > > + > > > > +src/splice-test -r $TEST_DIR/a > > > > +src/splice-test -rd $TEST_DIR/a > > > > +src/splice-test $TEST_DIR/a > > > > +src/splice-test -d $TEST_DIR/a > > > > + > > > > +# success, all done > > > > +status=0 > > > > +exit > > > > diff --git a/tests/generic/720.out b/tests/generic/720.out > > > > new file mode 100644 > > > > index 00000000..b0fc9935 > > > > --- /dev/null > > > > +++ b/tests/generic/720.out > > > > @@ -0,0 +1,7 @@ > > > > +QA output created by 720 > > > > +concurrent reader with O_DIRECT > > > > +concurrent reader with O_DIRECT > > > > +concurrent reader without O_DIRECT > > > > +concurrent reader without O_DIRECT > > > > +sequential reader with O_DIRECT > > > > +sequential reader without O_DIRECT > > > > diff --git a/tests/generic/group b/tests/generic/group > > > > index cd418106..f75d4e60 100644 > > > > --- a/tests/generic/group > > > > +++ b/tests/generic/group > > > > @@ -569,3 +569,4 @@ > > > > 564 auto quick copy_range > > > > 565 auto quick copy_range > > > > 719 auto quick quota metadata > > > > +720 auto quick rw pipe splice > > > > Thanks, > > Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-08-29 16:11 [PATCH] splice: only read in as much information as there is pipe buffer space Darrick J. Wong 2019-08-30 0:44 ` [RFC PATCH] generic: test splice() with pipes Darrick J. Wong @ 2019-08-30 21:06 ` Darrick J. Wong 2019-09-05 3:42 ` Darrick J. Wong 1 sibling, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-08-30 21:06 UTC (permalink / raw) To: viro, andreas.gruenbacher; +Cc: xfs, linux-fsdevel From: Darrick J. Wong <darrick.wong@oracle.com> Andreas Grünbacher reports that on the two filesystems that support iomap directio, it's possible for splice() to return -EAGAIN (instead of a short splice) if the pipe being written to has less space available in its pipe buffers than the length supplied by the calling process. Months ago we fixed splice_direct_to_actor to clamp the length of the read request to the size of the splice pipe. Do the same to do_splice. Fixes: 17614445576b6 ("splice: don't read more than available pipe space") Reported-by: Andreas Grünbacher <andreas.gruenbacher@gmail.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- v2: tidy up the other call site per Andreas' request --- fs/splice.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/fs/splice.c b/fs/splice.c index 98412721f056..2ddbace9129f 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -945,12 +945,13 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, WARN_ON_ONCE(pipe->nrbufs != 0); while (len) { + unsigned int pipe_pages; size_t read_len; loff_t pos = sd->pos, prev_pos = pos; /* Don't try to read more the pipe has space for. */ - read_len = min_t(size_t, len, - (pipe->buffers - pipe->nrbufs) << PAGE_SHIFT); + pipe_pages = pipe->buffers - pipe->nrbufs; + read_len = min(len, (size_t)pipe_pages << PAGE_SHIFT); ret = do_splice_to(in, &pos, pipe, read_len, flags); if (unlikely(ret <= 0)) goto out_release; @@ -1101,6 +1102,7 @@ static long do_splice(struct file *in, loff_t __user *off_in, struct pipe_inode_info *ipipe; struct pipe_inode_info *opipe; loff_t offset; + unsigned int pipe_pages; long ret; ipipe = get_pipe_info(in); @@ -1123,6 +1125,10 @@ static long do_splice(struct file *in, loff_t __user *off_in, if ((in->f_flags | out->f_flags) & O_NONBLOCK) flags |= SPLICE_F_NONBLOCK; + /* Don't try to read more the pipe has space for. */ + pipe_pages = opipe->buffers - opipe->nrbufs; + len = min(len, (size_t)pipe_pages << PAGE_SHIFT); + return splice_pipe_to_pipe(ipipe, opipe, len, flags); } @@ -1180,8 +1186,13 @@ static long do_splice(struct file *in, loff_t __user *off_in, pipe_lock(opipe); ret = wait_for_space(opipe, flags); - if (!ret) + if (!ret) { + /* Don't try to read more the pipe has space for. */ + pipe_pages = opipe->buffers - opipe->nrbufs; + len = min(len, (size_t)pipe_pages << PAGE_SHIFT); + ret = do_splice_to(in, &offset, opipe, len, flags); + } pipe_unlock(opipe); if (ret > 0) wakeup_pipe_readers(opipe); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-08-30 21:06 ` [PATCH v2] splice: only read in as much information as there is pipe buffer space Darrick J. Wong @ 2019-09-05 3:42 ` Darrick J. Wong 2019-09-17 13:17 ` Andreas Grünbacher 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-09-05 3:42 UTC (permalink / raw) To: viro, andreas.gruenbacher; +Cc: xfs, linux-fsdevel, Dave Chinner On Fri, Aug 30, 2019 at 02:06:03PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Andreas Grünbacher reports that on the two filesystems that support > iomap directio, it's possible for splice() to return -EAGAIN (instead of > a short splice) if the pipe being written to has less space available in > its pipe buffers than the length supplied by the calling process. > > Months ago we fixed splice_direct_to_actor to clamp the length of the > read request to the size of the splice pipe. Do the same to do_splice. > > Fixes: 17614445576b6 ("splice: don't read more than available pipe space") > Reported-by: Andreas Grünbacher <andreas.gruenbacher@gmail.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > v2: tidy up the other call site per Andreas' request Ping? Anyone want to add a RVB to this? --D > --- > fs/splice.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/fs/splice.c b/fs/splice.c > index 98412721f056..2ddbace9129f 100644 > --- a/fs/splice.c > +++ b/fs/splice.c > @@ -945,12 +945,13 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, > WARN_ON_ONCE(pipe->nrbufs != 0); > > while (len) { > + unsigned int pipe_pages; > size_t read_len; > loff_t pos = sd->pos, prev_pos = pos; > > /* Don't try to read more the pipe has space for. */ > - read_len = min_t(size_t, len, > - (pipe->buffers - pipe->nrbufs) << PAGE_SHIFT); > + pipe_pages = pipe->buffers - pipe->nrbufs; > + read_len = min(len, (size_t)pipe_pages << PAGE_SHIFT); > ret = do_splice_to(in, &pos, pipe, read_len, flags); > if (unlikely(ret <= 0)) > goto out_release; > @@ -1101,6 +1102,7 @@ static long do_splice(struct file *in, loff_t __user *off_in, > struct pipe_inode_info *ipipe; > struct pipe_inode_info *opipe; > loff_t offset; > + unsigned int pipe_pages; > long ret; > > ipipe = get_pipe_info(in); > @@ -1123,6 +1125,10 @@ static long do_splice(struct file *in, loff_t __user *off_in, > if ((in->f_flags | out->f_flags) & O_NONBLOCK) > flags |= SPLICE_F_NONBLOCK; > > + /* Don't try to read more the pipe has space for. */ > + pipe_pages = opipe->buffers - opipe->nrbufs; > + len = min(len, (size_t)pipe_pages << PAGE_SHIFT); > + > return splice_pipe_to_pipe(ipipe, opipe, len, flags); > } > > @@ -1180,8 +1186,13 @@ static long do_splice(struct file *in, loff_t __user *off_in, > > pipe_lock(opipe); > ret = wait_for_space(opipe, flags); > - if (!ret) > + if (!ret) { > + /* Don't try to read more the pipe has space for. */ > + pipe_pages = opipe->buffers - opipe->nrbufs; > + len = min(len, (size_t)pipe_pages << PAGE_SHIFT); > + > ret = do_splice_to(in, &offset, opipe, len, flags); > + } > pipe_unlock(opipe); > if (ret > 0) > wakeup_pipe_readers(opipe); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-09-05 3:42 ` Darrick J. Wong @ 2019-09-17 13:17 ` Andreas Grünbacher 2019-09-17 16:46 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Andreas Grünbacher @ 2019-09-17 13:17 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Al Viro, xfs, linux-fsdevel, Dave Chinner Am Do., 5. Sept. 2019 um 05:42 Uhr schrieb Darrick J. Wong <darrick.wong@oracle.com>: > On Fri, Aug 30, 2019 at 02:06:03PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Andreas Grünbacher reports that on the two filesystems that support > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > a short splice) if the pipe being written to has less space available in > > its pipe buffers than the length supplied by the calling process. > > > > Months ago we fixed splice_direct_to_actor to clamp the length of the > > read request to the size of the splice pipe. Do the same to do_splice. > > > > Fixes: 17614445576b6 ("splice: don't read more than available pipe space") > > Reported-by: Andreas Grünbacher <andreas.gruenbacher@gmail.com> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > v2: tidy up the other call site per Andreas' request > > Ping? Anyone want to add a RVB to this? You can add the following: Reviewed-by: Andreas Grünbacher <agruenba@redhat.com> Tested-by: Andreas Grünbacher <agruenba@redhat.com> And could you please update the email address in the reported-by tag as well? Is this going to go in via the xfs tree? Thanks, Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-09-17 13:17 ` Andreas Grünbacher @ 2019-09-17 16:46 ` Darrick J. Wong 2019-09-17 17:00 ` Andreas Grünbacher 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2019-09-17 16:46 UTC (permalink / raw) To: Andreas Grünbacher; +Cc: Al Viro, xfs, linux-fsdevel, Dave Chinner On Tue, Sep 17, 2019 at 03:17:22PM +0200, Andreas Grünbacher wrote: > Am Do., 5. Sept. 2019 um 05:42 Uhr schrieb Darrick J. Wong > <darrick.wong@oracle.com>: > > On Fri, Aug 30, 2019 at 02:06:03PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > Andreas Grünbacher reports that on the two filesystems that support > > > iomap directio, it's possible for splice() to return -EAGAIN (instead of > > > a short splice) if the pipe being written to has less space available in > > > its pipe buffers than the length supplied by the calling process. > > > > > > Months ago we fixed splice_direct_to_actor to clamp the length of the > > > read request to the size of the splice pipe. Do the same to do_splice. > > > > > > Fixes: 17614445576b6 ("splice: don't read more than available pipe space") > > > Reported-by: Andreas Grünbacher <andreas.gruenbacher@gmail.com> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > v2: tidy up the other call site per Andreas' request > > > > Ping? Anyone want to add a RVB to this? > > You can add the following: > > Reviewed-by: Andreas Grünbacher <agruenba@redhat.com> > Tested-by: Andreas Grünbacher <agruenba@redhat.com> > > And could you please update the email address in the reported-by tag as well? Done. > Is this going to go in via the xfs tree? I'll let it soak in -next for a few days and send a single-patch pull request for it. (I'm sending out pull requests today for the things that have been ready to go for the last couple of weeks.) --D > Thanks, > Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-09-17 16:46 ` Darrick J. Wong @ 2019-09-17 17:00 ` Andreas Grünbacher 2019-09-19 22:39 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Andreas Grünbacher @ 2019-09-17 17:00 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Al Viro, xfs, linux-fsdevel, Dave Chinner Am Di., 17. Sept. 2019 um 18:46 Uhr schrieb Darrick J. Wong <darrick.wong@oracle.com>: > > Is this going to go in via the xfs tree? > > I'll let it soak in -next for a few days and send a single-patch pull > request for it. > > (I'm sending out pull requests today for the things that have been > ready to go for the last couple of weeks.) Okay, works for me. Thanks, Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] splice: only read in as much information as there is pipe buffer space 2019-09-17 17:00 ` Andreas Grünbacher @ 2019-09-19 22:39 ` Darrick J. Wong 0 siblings, 0 replies; 15+ messages in thread From: Darrick J. Wong @ 2019-09-19 22:39 UTC (permalink / raw) To: Andreas Grünbacher; +Cc: Al Viro, xfs, linux-fsdevel, Dave Chinner On Tue, Sep 17, 2019 at 07:00:15PM +0200, Andreas Grünbacher wrote: > Am Di., 17. Sept. 2019 um 18:46 Uhr schrieb Darrick J. Wong > <darrick.wong@oracle.com>: > > > Is this going to go in via the xfs tree? > > > > I'll let it soak in -next for a few days and send a single-patch pull > > request for it. > > > > (I'm sending out pull requests today for the things that have been > > ready to go for the last couple of weeks.) > > Okay, works for me. Heh, syzbot found a bug[1] in this patch, so I'm withdrawing it for now. --D [1] https://lore.kernel.org/linux-fsdevel/20190919211013.GN5340@magnolia/T/ > Thanks, > Andreas ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-11-22 1:28 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-29 16:11 [PATCH] splice: only read in as much information as there is pipe buffer space Darrick J. Wong 2019-08-30 0:44 ` [RFC PATCH] generic: test splice() with pipes Darrick J. Wong 2019-09-02 2:20 ` Zorro Lang 2019-09-02 16:56 ` Darrick J. Wong 2019-09-03 3:19 ` Zorro Lang 2019-11-21 17:01 ` Darrick J. Wong 2019-11-21 18:48 ` Andreas Grünbacher 2019-11-21 19:14 ` Darrick J. Wong 2019-11-22 1:27 ` Andreas Grünbacher 2019-08-30 21:06 ` [PATCH v2] splice: only read in as much information as there is pipe buffer space Darrick J. Wong 2019-09-05 3:42 ` Darrick J. Wong 2019-09-17 13:17 ` Andreas Grünbacher 2019-09-17 16:46 ` Darrick J. Wong 2019-09-17 17:00 ` Andreas Grünbacher 2019-09-19 22:39 ` Darrick J. Wong
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).