On Wed, Jun 28, 2023 at 3:21 AM Ahelenia Ziemiańska <nabijaczleweli@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> > --- > 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.
