[LTP] [PATCH 0/2] Add splice tests fro /dev/{zero,null,full}

[LTP] [PATCH 0/2] Add splice tests fro /dev/{zero,null,full}

[Cyril Hrubis]

We came around this when we were doing pre-release testing for the
splice07 test, the support for splicing to/from some dev files was
likely unintentionaly removed and added back later on. This patchset
implements first two basic splice tests for these device files.

Cyril Hrubis (2):
  syscalls: Add test for splicing from /dev/zero and /dev/full
  syscalls: Add test for splicing to /dev/zero and /dev/null

 runtest/syscalls                            |  2 +
 testcases/kernel/syscalls/splice/.gitignore |  2 +
 testcases/kernel/syscalls/splice/splice08.c | 88 +++++++++++++++++++++
 testcases/kernel/syscalls/splice/splice09.c | 55 +++++++++++++
 4 files changed, 147 insertions(+)
 create mode 100644 testcases/kernel/syscalls/splice/splice08.c
 create mode 100644 testcases/kernel/syscalls/splice/splice09.c

-- 
2.43.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Cyril Hrubis]

Both of these devices produce zeroes when read.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/splice/.gitignore |  1 +
 testcases/kernel/syscalls/splice/splice08.c | 88 +++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 testcases/kernel/syscalls/splice/splice08.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 4ed2b5602..0889f58a1 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1517,6 +1517,7 @@ splice04 splice04
 splice05 splice05
 splice06 splice06
 splice07 splice07
+splice08 splice08
 
 tee01 tee01
 tee02 tee02
diff --git a/testcases/kernel/syscalls/splice/.gitignore b/testcases/kernel/syscalls/splice/.gitignore
index 88a8dff78..9453cf93a 100644
--- a/testcases/kernel/syscalls/splice/.gitignore
+++ b/testcases/kernel/syscalls/splice/.gitignore
@@ -5,3 +5,4 @@
 /splice05
 /splice06
 /splice07
+/splice08
diff --git a/testcases/kernel/syscalls/splice/splice08.c b/testcases/kernel/syscalls/splice/splice08.c
new file mode 100644
index 000000000..cdd51b66c
--- /dev/null
+++ b/testcases/kernel/syscalls/splice/splice08.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test for splicing from /dev/zero and /dev/full.
+ *
+ * The support for splicing from /dev/zero and /dev/full was removed in:
+ * c6585011bc1d ("splice: Remove generic_file_splice_read()")
+ *
+ * And added back in:
+ * 1b057bd800c3 ("drivers/char/mem: implement splice() for /dev/zero, /dev/full")
+ */
+
+#define _GNU_SOURCE
+#include "tst_test.h"
+
+static int fd_zero;
+static int fd_full;
+
+static void test_splice(unsigned int bytes, int dev_fd)
+{
+	int pipefd[2];
+	char buf[bytes];
+	size_t i;
+	int fail = 0;
+
+	memset(buf, 0xff, sizeof(buf));
+
+	SAFE_PIPE(pipefd);
+
+	TST_EXP_POSITIVE(splice(dev_fd, NULL, pipefd[1], NULL, sizeof(buf), 0));
+
+	if (!TST_PASS)
+		goto ret;
+
+	SAFE_READ(1, pipefd[0], buf, sizeof(buf));
+
+	for (i = 0; i < sizeof(buf); i++) {
+		if (buf[i])
+			fail++;
+	}
+
+	if (fail)
+		tst_res(TFAIL, "Non-zero bytes spliced from /dev/zero");
+	else
+		tst_res(TPASS, "All bytes spliced from /dev/zero are zeroed");
+
+ret:
+	SAFE_CLOSE(pipefd[0]);
+	SAFE_CLOSE(pipefd[1]);
+}
+
+static void verify_splice(unsigned int n)
+{
+	unsigned int bytes = 1009 * n;
+
+	tst_res(TINFO, "Splicing %u bytes from /dev/zero", bytes);
+	test_splice(bytes, fd_zero);
+	tst_res(TINFO, "Splicing %u bytes from /dev/full", bytes);
+	test_splice(bytes, fd_full);
+}
+
+static void setup(void)
+{
+	fd_zero = SAFE_OPEN("/dev/zero", O_RDONLY);
+	fd_full = SAFE_OPEN("/dev/full", O_RDONLY);
+}
+
+static void cleanup(void)
+{
+	if (fd_zero > 0)
+		SAFE_CLOSE(fd_zero);
+
+	if (fd_full > 0)
+		SAFE_CLOSE(fd_full);
+}
+
+static struct tst_test test = {
+	.test = verify_splice,
+	.tcnt = 9,
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "6.7",
+};
-- 
2.43.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Jan Stancek]

On Wed, Mar 20, 2024 at 11:01 AM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Both of these devices produce zeroes when read.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/splice/.gitignore |  1 +
>  testcases/kernel/syscalls/splice/splice08.c | 88 +++++++++++++++++++++
>  3 files changed, 90 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/splice/splice08.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 4ed2b5602..0889f58a1 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1517,6 +1517,7 @@ splice04 splice04
>  splice05 splice05
>  splice06 splice06
>  splice07 splice07
> +splice08 splice08
>
>  tee01 tee01
>  tee02 tee02
> diff --git a/testcases/kernel/syscalls/splice/.gitignore b/testcases/kernel/syscalls/splice/.gitignore
> index 88a8dff78..9453cf93a 100644
> --- a/testcases/kernel/syscalls/splice/.gitignore
> +++ b/testcases/kernel/syscalls/splice/.gitignore
> @@ -5,3 +5,4 @@
>  /splice05
>  /splice06
>  /splice07
> +/splice08
> diff --git a/testcases/kernel/syscalls/splice/splice08.c b/testcases/kernel/syscalls/splice/splice08.c
> new file mode 100644
> index 000000000..cdd51b66c
> --- /dev/null
> +++ b/testcases/kernel/syscalls/splice/splice08.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Test for splicing from /dev/zero and /dev/full.
> + *
> + * The support for splicing from /dev/zero and /dev/full was removed in:
> + * c6585011bc1d ("splice: Remove generic_file_splice_read()")
> + *
> + * And added back in:
> + * 1b057bd800c3 ("drivers/char/mem: implement splice() for /dev/zero, /dev/full")
> + */
> +
> +#define _GNU_SOURCE
> +#include "tst_test.h"
> +
> +static int fd_zero;
> +static int fd_full;
> +
> +static void test_splice(unsigned int bytes, int dev_fd)
> +{
> +       int pipefd[2];
> +       char buf[bytes];
> +       size_t i;
> +       int fail = 0;
> +
> +       memset(buf, 0xff, sizeof(buf));
> +
> +       SAFE_PIPE(pipefd);
> +
> +       TST_EXP_POSITIVE(splice(dev_fd, NULL, pipefd[1], NULL, sizeof(buf), 0));

Both look good to me, just curious if you meant to check TST_RET
against sizeof(buf) here
as you did in splice09.

> +
> +       if (!TST_PASS)
> +               goto ret;
> +
> +       SAFE_READ(1, pipefd[0], buf, sizeof(buf));
> +
> +       for (i = 0; i < sizeof(buf); i++) {
> +               if (buf[i])
> +                       fail++;
> +       }
> +
> +       if (fail)
> +               tst_res(TFAIL, "Non-zero bytes spliced from /dev/zero");
> +       else
> +               tst_res(TPASS, "All bytes spliced from /dev/zero are zeroed");
> +
> +ret:
> +       SAFE_CLOSE(pipefd[0]);
> +       SAFE_CLOSE(pipefd[1]);
> +}
> +
> +static void verify_splice(unsigned int n)
> +{
> +       unsigned int bytes = 1009 * n;
> +
> +       tst_res(TINFO, "Splicing %u bytes from /dev/zero", bytes);
> +       test_splice(bytes, fd_zero);
> +       tst_res(TINFO, "Splicing %u bytes from /dev/full", bytes);
> +       test_splice(bytes, fd_full);
> +}
> +
> +static void setup(void)
> +{
> +       fd_zero = SAFE_OPEN("/dev/zero", O_RDONLY);
> +       fd_full = SAFE_OPEN("/dev/full", O_RDONLY);
> +}
> +
> +static void cleanup(void)
> +{
> +       if (fd_zero > 0)
> +               SAFE_CLOSE(fd_zero);
> +
> +       if (fd_full > 0)
> +               SAFE_CLOSE(fd_full);
> +}
> +
> +static struct tst_test test = {
> +       .test = verify_splice,
> +       .tcnt = 9,
> +       .setup = setup,
> +       .cleanup = cleanup,
> +       .min_kver = "6.7",
> +};
> --
> 2.43.2
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Cyril Hrubis]

Hi!
> Both look good to me, just curious if you meant to check TST_RET

Is that Acked-by or Reviewed-by?

> against sizeof(buf) here
> as you did in splice09.

Will fix.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Jan Stancek]

On Thu, Apr 11, 2024 at 12:44 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Both look good to me, just curious if you meant to check TST_RET
>
> Is that Acked-by or Reviewed-by?
>
> > against sizeof(buf) here
> > as you did in splice09.
>
> Will fix.

Feel free to add to next version:
Reviewed-by: Jan Stancek <jstancek@redhat.com>

>
> --
> Cyril Hrubis
> chrubis@suse.cz
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Cyril Hrubis]

Hi!
> Feel free to add to next version:
> Reviewed-by: Jan Stancek <jstancek@redhat.com>

I've fixed all the problems pointed out by you and peter and pushed,
thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Petr Vorel]

Hi Cyril,

> +static void test_splice(unsigned int bytes, int dev_fd)
> +{
> +	int pipefd[2];
> +	char buf[bytes];
> +	size_t i;
> +	int fail = 0;
> +
> +	memset(buf, 0xff, sizeof(buf));
> +
> +	SAFE_PIPE(pipefd);
> +
> +	TST_EXP_POSITIVE(splice(dev_fd, NULL, pipefd[1], NULL, sizeof(buf), 0));
> +
> +	if (!TST_PASS)
> +		goto ret;
> +
> +	SAFE_READ(1, pipefd[0], buf, sizeof(buf));
> +
> +	for (i = 0; i < sizeof(buf); i++) {
> +		if (buf[i])
> +			fail++;
> +	}
> +
> +	if (fail)
> +		tst_res(TFAIL, "Non-zero bytes spliced from /dev/zero");
Maybe write how many fail we have?

I also agree with Jan's comment about missing TST_RET, which is in splice09.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> +	else
> +		tst_res(TPASS, "All bytes spliced from /dev/zero are zeroed");
> +
> +ret:
> +	SAFE_CLOSE(pipefd[0]);
> +	SAFE_CLOSE(pipefd[1]);
> +}
> +
> +static void verify_splice(unsigned int n)
> +{
> +	unsigned int bytes = 1009 * n;

Out of curiosity, why 1009 and not 1000?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] syscalls: Add test for splicing from /dev/zero and /dev/full

[Cyril Hrubis]

Hi!
> > +{
> > +	int pipefd[2];
> > +	char buf[bytes];
> > +	size_t i;
> > +	int fail = 0;
> > +
> > +	memset(buf, 0xff, sizeof(buf));
> > +
> > +	SAFE_PIPE(pipefd);
> > +
> > +	TST_EXP_POSITIVE(splice(dev_fd, NULL, pipefd[1], NULL, sizeof(buf), 0));
> > +
> > +	if (!TST_PASS)
> > +		goto ret;
> > +
> > +	SAFE_READ(1, pipefd[0], buf, sizeof(buf));
> > +
> > +	for (i = 0; i < sizeof(buf); i++) {
> > +		if (buf[i])
> > +			fail++;
> > +	}
> > +
> > +	if (fail)
> > +		tst_res(TFAIL, "Non-zero bytes spliced from /dev/zero");
> Maybe write how many fail we have?
> 
> I also agree with Jan's comment about missing TST_RET, which is in splice09.

Will fix both.

> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> > +	else
> > +		tst_res(TPASS, "All bytes spliced from /dev/zero are zeroed");
> > +
> > +ret:
> > +	SAFE_CLOSE(pipefd[0]);
> > +	SAFE_CLOSE(pipefd[1]);
> > +}
> > +
> > +static void verify_splice(unsigned int n)
> > +{
> > +	unsigned int bytes = 1009 * n;
> 
> Out of curiosity, why 1009 and not 1000?

Because people tend to use buffers that are either power of two or
multiples of 100 or 1000. The 1009 is a prime number which means that
multiples of it will have "unexpected" sizes.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] syscalls: Add test for splicing to /dev/zero and /dev/null

[Cyril Hrubis]

Both of these devices discard written data.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/splice/.gitignore |  1 +
 testcases/kernel/syscalls/splice/splice09.c | 55 +++++++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 testcases/kernel/syscalls/splice/splice09.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 0889f58a1..50bc350f0 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1518,6 +1518,7 @@ splice05 splice05
 splice06 splice06
 splice07 splice07
 splice08 splice08
+splice09 splice09
 
 tee01 tee01
 tee02 tee02
diff --git a/testcases/kernel/syscalls/splice/.gitignore b/testcases/kernel/syscalls/splice/.gitignore
index 9453cf93a..96b1727a1 100644
--- a/testcases/kernel/syscalls/splice/.gitignore
+++ b/testcases/kernel/syscalls/splice/.gitignore
@@ -6,3 +6,4 @@
 /splice06
 /splice07
 /splice08
+/splice09
diff --git a/testcases/kernel/syscalls/splice/splice09.c b/testcases/kernel/syscalls/splice/splice09.c
new file mode 100644
index 000000000..46f755b01
--- /dev/null
+++ b/testcases/kernel/syscalls/splice/splice09.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test for splicing to /dev/zero and /dev/null these two devices discard all
+ * data written to them.
+ *
+ * The support for splicing to /dev/zero was added in:
+ * 1b057bd800c3 ("drivers/char/mem: implement splice() for /dev/zero, /dev/full")
+ */
+
+#define _GNU_SOURCE
+#include "tst_test.h"
+
+static const char *test_devices[] = {
+	"/dev/null",
+	"/dev/zero",
+};
+
+static void verify_splice(unsigned int n)
+{
+	char buf[1024];
+	char dev_fd;
+	int pipefd[2];
+
+	memset(buf, 0xff, sizeof(buf));
+
+	tst_res(TINFO, "Testing %s", test_devices[n]);
+
+	dev_fd = SAFE_OPEN(test_devices[n], O_WRONLY);
+
+	SAFE_PIPE(pipefd);
+	SAFE_WRITE(1, pipefd[1], buf, sizeof(buf));
+
+	TST_EXP_POSITIVE(splice(pipefd[0], NULL, dev_fd, NULL, sizeof(buf), 0));
+
+	if (TST_PASS && TST_RET != sizeof(buf))
+		tst_res(TFAIL, "Wrote only part of the pipe buffer");
+	else
+		tst_res(TPASS, "Wrote whole pipe buffer");
+
+	SAFE_CLOSE(pipefd[0]);
+	SAFE_CLOSE(pipefd[1]);
+	SAFE_CLOSE(dev_fd);
+}
+
+static struct tst_test test = {
+	.test = verify_splice,
+	.tcnt = ARRAY_SIZE(test_devices),
+	.min_kver = "6.7",
+};
-- 
2.43.2


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] syscalls: Add test for splicing to /dev/zero and /dev/null

[Petr Vorel]

Hi Cyril,

...
> +#define _GNU_SOURCE
> +#include "tst_test.h"
> +
> +static const char *test_devices[] = {
checkpatch.pl suggests:
static const char * const test_devices[] = {

> +	"/dev/null",
> +	"/dev/zero",
> +};
> +
> +static void verify_splice(unsigned int n)
> +{
> +	char buf[1024];
> +	char dev_fd;
> +	int pipefd[2];
> +
> +	memset(buf, 0xff, sizeof(buf));
> +
> +	tst_res(TINFO, "Testing %s", test_devices[n]);
> +
> +	dev_fd = SAFE_OPEN(test_devices[n], O_WRONLY);
> +
> +	SAFE_PIPE(pipefd);
> +	SAFE_WRITE(1, pipefd[1], buf, sizeof(buf));
I wonder if write() fails, we don't get SAFE_CLOSE() calls, right?

Otherwise LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +
> +	TST_EXP_POSITIVE(splice(pipefd[0], NULL, dev_fd, NULL, sizeof(buf), 0));
> +
> +	if (TST_PASS && TST_RET != sizeof(buf))
> +		tst_res(TFAIL, "Wrote only part of the pipe buffer");
> +	else
> +		tst_res(TPASS, "Wrote whole pipe buffer");
> +
> +	SAFE_CLOSE(pipefd[0]);
> +	SAFE_CLOSE(pipefd[1]);
> +	SAFE_CLOSE(dev_fd);
> +}
> +
> +static struct tst_test test = {
> +	.test = verify_splice,
> +	.tcnt = ARRAY_SIZE(test_devices),
> +	.min_kver = "6.7",
> +};

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] syscalls: Add test for splicing to /dev/zero and /dev/null

[Cyril Hrubis]

Hi!
> > +#define _GNU_SOURCE
> > +#include "tst_test.h"
> > +
> > +static const char *test_devices[] = {
> checkpatch.pl suggests:
> static const char * const test_devices[] = {

Will fix and push.

> > +	"/dev/null",
> > +	"/dev/zero",
> > +};
> > +
> > +static void verify_splice(unsigned int n)
> > +{
> > +	char buf[1024];
> > +	char dev_fd;
> > +	int pipefd[2];
> > +
> > +	memset(buf, 0xff, sizeof(buf));
> > +
> > +	tst_res(TINFO, "Testing %s", test_devices[n]);
> > +
> > +	dev_fd = SAFE_OPEN(test_devices[n], O_WRONLY);
> > +
> > +	SAFE_PIPE(pipefd);
> > +	SAFE_WRITE(1, pipefd[1], buf, sizeof(buf));
> I wonder if write() fails, we don't get SAFE_CLOSE() calls, right?

We do not care here since these are just pipes, the test will fail with
TBROK and kernel will destroy the pipes once the process exits.

We have to close real files in test temporary directory, because if we
do not do so, tst_rmdir() may fail on nfs. E.g. if you create a file on
nfs, open it and unlink it, the directory is not empty as one would
expect and cannot be removed.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp