[LTP] [LTP RFC PATCH v3] inotify13: new test for fs/splice.c functions vs pipes vs inotify
Amir Goldstein
amir73il@gmail.com
Wed Jun 28 07:30:15 CEST 2023
On Wed, Jun 28, 2023 at 3:21 AM Ahelenia Ziemiańska
<nabijaczleweli@nabijaczleweli.xyz> wrote:
>
> The only one that passes on 6.1.27-1 is sendfile_file_to_pipe.
>
> Link: https://lore.kernel.org/linux-fsdevel/jbyihkyk5dtaohdwjyivambb2gffyjs3dodpofafnkkunxq7bu@jngkdxx65pux/t/#u
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
> Sorry, I missed second part of Amir's comments somehow.
> cleanup is only run at the end by default:
> run it manually to not leak fds between tests.
>
> I've parameterised the tests from the driver, instead of with macros,
> and removed the tst_tag data.
>
> Added the * [Description] tag and full commit subject to the header
> comment; leaving the lore.k.o link for now, to be turned into a SHA
> when the kernel behaviour this tests starts having a SHA.
>
> Error checking has been lifted out as well.
> Formatted in kernel style accd'g to clang-format and check-inotify13.
>
> I used the wrong address for ltp@ the first time; I've since bounced the
> patchset, and am sending this, to the correct address. They were all
> held for moderation for now.
>
> testcases/kernel/syscalls/inotify/.gitignore | 1 +
> testcases/kernel/syscalls/inotify/inotify13.c | 282 ++++++++++++++++++
> 2 files changed, 283 insertions(+)
> create mode 100644 testcases/kernel/syscalls/inotify/inotify13.c
>
> diff --git a/testcases/kernel/syscalls/inotify/.gitignore b/testcases/kernel/syscalls/inotify/.gitignore
> index f6e5c546a..b597ea63f 100644
> --- a/testcases/kernel/syscalls/inotify/.gitignore
> +++ b/testcases/kernel/syscalls/inotify/.gitignore
> @@ -10,3 +10,4 @@
> /inotify10
> /inotify11
> /inotify12
> +/inotify13
> diff --git a/testcases/kernel/syscalls/inotify/inotify13.c b/testcases/kernel/syscalls/inotify/inotify13.c
> new file mode 100644
> index 000000000..97f88053e
> --- /dev/null
> +++ b/testcases/kernel/syscalls/inotify/inotify13.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*\
> + * [Description]
> + * Verify splice-family functions (and sendfile) generate IN_ACCESS
> + * for what they read and IN_MODIFY for what they write.
> + *
> + * Regression test for 983652c69199 ("splice: report related fsnotify events") and
> + * https://lore.kernel.org/linux-fsdevel/jbyihkyk5dtaohdwjyivambb2gffyjs3dodpofafnkkunxq7bu@jngkdxx65pux/t/#u
The process of posting a test for the fix that was not yet merged
is indeed a chicken and egg situation.
What I usually do is post a draft test (like this) and link
to the post of the LTP test (and maybe a branch on github)
when posting the fix, to say how I tested the fix.
I would then put it in my TODO to re-post the LTP
test once the kernel fix has been merged.
> + */
> +
> +#define _GNU_SOURCE
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include <inttypes.h>
> +#include <signal.h>
> +#include <sys/mman.h>
> +#include <sys/sendfile.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "inotify.h"
> +
> +#if defined(HAVE_SYS_INOTIFY_H)
> +#include <sys/inotify.h>
> +
> +static int pipes[2] = { -1, -1 };
> +static int inotify = -1;
> +static int memfd = -1;
> +static int data_pipes[2] = { -1, -1 };
> +
> +static void watch_rw(int fd)
> +{
> + char buf[64];
> +
> + sprintf(buf, "/proc/self/fd/%d", fd);
> + SAFE_MYINOTIFY_ADD_WATCH(inotify, buf, IN_ACCESS | IN_MODIFY);
> +}
> +
> +static int compar(const void *l, const void *r)
> +{
> + const struct inotify_event *lie = l;
> + const struct inotify_event *rie = r;
> +
> + return lie->wd - rie->wd;
> +}
> +
> +static void get_events(size_t evcnt, struct inotify_event evs[static evcnt])
> +{
> + struct inotify_event tail, *itr = evs;
> +
> + for (size_t left = evcnt; left; --left)
> + SAFE_READ(true, inotify, itr++, sizeof(struct inotify_event));
> +
> + TEST(read(inotify, &tail, sizeof(struct inotify_event)));
> + if (TST_RET != -1)
> + tst_brk(TFAIL, ">%zu events", evcnt);
> + if (TST_ERR != EAGAIN)
> + tst_brk(TFAIL | TTERRNO, "expected EAGAIN");
> +
> + qsort(evs, evcnt, sizeof(struct inotify_event), compar);
> +}
> +
> +static void expect_transfer(const char *name, size_t size)
> +{
> + if (TST_RET == -1)
> + tst_brk(TBROK | TERRNO, "%s", name);
> + if ((size_t)TST_RET != size)
> + tst_brk(TBROK, "%s: %ld != %zu", name, TST_RET, size);
> +}
> +
> +static void expect_event(struct inotify_event *ev, int wd, uint32_t mask)
> +{
> + if (ev->wd != wd)
> + tst_brk(TFAIL, "expect event for wd %d got %d", wd, ev->wd);
> + if (ev->mask != mask)
> + tst_brk(TFAIL,
> + "expect event with mask %" PRIu32 " got %" PRIu32 "",
> + mask, ev->mask);
> +}
> +
> +// write to file, rewind, transfer accd'g to f2p, read from pipe
> +// expecting: IN_ACCESS memfd, IN_MODIFY pipes[0]
> +static void file_to_pipe(const char *name, ssize_t (*f2p)(void))
> +{
> + struct inotify_event events[2];
> + char buf[strlen(name)];
> +
> + SAFE_WRITE(SAFE_WRITE_RETRY, memfd, name, strlen(name));
> + SAFE_LSEEK(memfd, 0, SEEK_SET);
> + watch_rw(memfd);
> + watch_rw(pipes[0]);
> + TEST(f2p());
> + expect_transfer(name, strlen(name));
> +
> + get_events(ARRAY_SIZE(events), events);
> + expect_event(events + 0, 1, IN_ACCESS);
> + expect_event(events + 1, 2, IN_MODIFY);
So what I meant to say is that if there are double events that
usually get merged (unless reader was fast enough to read the
first event), this is something that I could live with, but encoding
an expectation for a double event, that's not at all what I meant.
But anyway, I see that you've found a way to work around
this problem, so at least the test can expect and get a single event.
I think you are missing expect_no_more_events() here to
verify that you won't get double events.
See test inotify12 as an example for a test that encodes
expect_events per test case and also verifies there are no
unexpected extra events.
That's also an example of a more generic test template,
but your test cases are all a bit different from each other is
subtle ways, so I trust you will find the best balance between
putting generic parameterized code in the run_test() template
and putting code in the test case subroutine.
> +
> + SAFE_READ(true, pipes[0], buf, strlen(name));
> + if (memcmp(buf, name, strlen(name)))
> + tst_brk(TFAIL, "buf contents bad");
> +}
> +static ssize_t splice_file_to_pipe(void)
> +{
> + return splice(memfd, NULL, pipes[1], NULL, 128 * 1024 * 1024, 0);
> +}
> +static ssize_t sendfile_file_to_pipe(void)
> +{
> + return sendfile(pipes[1], memfd, NULL, 128 * 1024 * 1024);
> +}
> +
> +// write to pipe, transfer with splice, rewind file, read from file
> +// expecting: IN_ACCESS pipes[0], IN_MODIFY memfd
> +static void splice_pipe_to_file(const char *name, ssize_t (*param)(void))
> +{
> + (void)name;
> + (void)param;
> + struct inotify_event events[2];
> + char buf[sizeof(__func__)];
> +
> + SAFE_WRITE(SAFE_WRITE_RETRY, pipes[1], __func__, sizeof(__func__));
> + watch_rw(pipes[0]);
> + watch_rw(memfd);
> + TEST(splice(pipes[0], NULL, memfd, NULL, 128 * 1024 * 1024, 0));
> + expect_transfer(__func__, sizeof(__func__));
> +
> + get_events(ARRAY_SIZE(events), events);
> + expect_event(events + 0, 1, IN_ACCESS);
> + expect_event(events + 1, 2, IN_MODIFY);
> +
> + SAFE_LSEEK(memfd, 0, SEEK_SET);
> + SAFE_READ(true, memfd, buf, sizeof(__func__));
> + if (memcmp(buf, __func__, sizeof(__func__)))
> + tst_brk(TFAIL, "buf contents bad");
> +}
> +
> +// write to data_pipe, transfer accd'g to p2p, read from pipe
> +// expecting: IN_ACCESS data_pipes[0], IN_MODIFY pipes[1]
> +static void pipe_to_pipe(const char *name, ssize_t (*p2p)(void))
> +{
> + struct inotify_event events[2];
> + char buf[strlen(name)];
> +
> + SAFE_WRITE(SAFE_WRITE_RETRY, data_pipes[1], name, strlen(name));
> + watch_rw(data_pipes[0]);
> + watch_rw(pipes[1]);
> + TEST(p2p());
> + expect_transfer(name, strlen(name));
> +
> + get_events(ARRAY_SIZE(events), events);
> + expect_event(events + 0, 1, IN_ACCESS);
> + expect_event(events + 1, 2, IN_MODIFY);
> +
> + SAFE_READ(true, pipes[0], buf, strlen(name));
> + if (memcmp(buf, name, strlen(name)))
> + tst_brk(TFAIL, "buf contents bad");
> +}
> +static ssize_t splice_pipe_to_pipe(void)
> +{
> + return splice(data_pipes[0], NULL, pipes[1], NULL, 128 * 1024 * 1024,
> + 0);
> +}
> +static ssize_t tee_pipe_to_pipe(void)
> +{
> + return tee(data_pipes[0], pipes[1], 128 * 1024 * 1024, 0);
> +}
> +
> +// vmsplice to pipe, read from pipe
> +// expecting: IN_MODIFY pipes[0]
> +static char vmsplice_pipe_to_mem_dt[32 * 1024];
> +static void vmsplice_pipe_to_mem(const char *name, ssize_t (*param)(void))
> +{
> + (void)name;
> + (void)param;
> + struct inotify_event event;
> + char buf[sizeof(__func__)];
> +
> + memcpy(vmsplice_pipe_to_mem_dt, __func__, sizeof(__func__));
> + watch_rw(pipes[0]);
> + TEST(vmsplice(
> + pipes[1],
> + &(struct iovec){ .iov_base = vmsplice_pipe_to_mem_dt,
> + .iov_len = sizeof(vmsplice_pipe_to_mem_dt) },
> + 1, SPLICE_F_GIFT));
> + expect_transfer(__func__, sizeof(vmsplice_pipe_to_mem_dt));
> +
> + get_events(1, &event);
> + expect_event(&event, 1, IN_MODIFY);
> +
> + SAFE_READ(true, pipes[0], buf, sizeof(__func__));
> + if (memcmp(buf, __func__, sizeof(__func__)))
> + tst_brk(TFAIL, "buf contents bad");
> +}
> +
> +// write to pipe, vmsplice from pipe
> +// expecting: IN_ACCESS pipes[1]
> +static void vmsplice_mem_to_pipe(const char *name, ssize_t (*param)(void))
> +{
> + (void)name;
> + (void)param;
> + char buf[sizeof(__func__)];
> + struct inotify_event event;
> +
> + SAFE_WRITE(SAFE_WRITE_RETRY, pipes[1], __func__, sizeof(__func__));
> + watch_rw(pipes[1]);
> + TEST(vmsplice(pipes[0],
> + &(struct iovec){ .iov_base = buf,
> + .iov_len = sizeof(buf) },
> + 1, 0));
> + expect_transfer(__func__, sizeof(buf));
> +
> + get_events(1, &event);
> + expect_event(&event, 1, IN_ACCESS);
> +
> + if (memcmp(buf, __func__, sizeof(__func__)))
> + tst_brk(TFAIL, "buf contents bad");
> +}
> +
> +#define TEST_F(f, param) \
> + { \
> + #f, f, param, \
> + }
> +static const struct {
> + const char *n;
> + void (*f)(const char *name, ssize_t (*param)(void));
> + ssize_t (*param)(void);
> +} tests[] = {
> + TEST_F(file_to_pipe, splice_file_to_pipe),
> + TEST_F(file_to_pipe, sendfile_file_to_pipe),
> + TEST_F(splice_pipe_to_file, NULL),
> + TEST_F(pipe_to_pipe, splice_pipe_to_pipe),
> + TEST_F(pipe_to_pipe, tee_pipe_to_pipe),
> + TEST_F(vmsplice_pipe_to_mem, NULL),
> + TEST_F(vmsplice_mem_to_pipe, NULL),
> +};
> +
> +static void cleanup(void)
> +{
> + if (memfd != -1)
> + SAFE_CLOSE(memfd);
> + if (inotify != -1)
> + SAFE_CLOSE(inotify);
> + if (pipes[0] != -1)
> + SAFE_CLOSE(pipes[0]);
> + if (pipes[1] != -1)
> + SAFE_CLOSE(pipes[1]);
> + if (data_pipes[0] != -1)
> + SAFE_CLOSE(data_pipes[0]);
> + if (data_pipes[1] != -1)
> + SAFE_CLOSE(data_pipes[1]);
> +}
> +
> +static void run_test(unsigned int n)
> +{
> + tst_res(TINFO, "%s", tests[n].n);
> +
> + SAFE_PIPE2(pipes, O_CLOEXEC);
> + SAFE_PIPE2(data_pipes, O_CLOEXEC);
> + inotify = SAFE_MYINOTIFY_INIT1(IN_NONBLOCK | IN_CLOEXEC);
> + memfd = memfd_create(__func__, MFD_CLOEXEC);
> + if (memfd == -1)
> + tst_brk(TCONF | TERRNO, "memfd");
> + tests[n].f(tests[n].n, tests[n].param);
> + tst_res(TPASS, "ок");
> + cleanup();
> +}
> +
> +static struct tst_test test = {
> + .cleanup = cleanup,
> + .test = run_test,
> + .tcnt = ARRAY_SIZE(tests),
> + .tags = (const struct tst_tag[]){ {} },
I don't think this is needed for the draft...
Thanks,
Amir.
More information about the ltp
mailing list