| Message ID | 20221212135524.1333-3-rpalethorpe@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/3] lib/tst_rand_data: Add statically defined data pattern | expand |
Hi! > +enum tst_fill_access_pattern { > + TST_FILL_FLAT_VEC, Not sure if this is a good name, maybe TST_FILL_BLOCKS or TST_FILL_PAGES. > + TST_FILL_RANDOM > +}; Otherwise the whole patchset looks good to me. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> +enum tst_fill_access_pattern { >> + TST_FILL_FLAT_VEC, > > Not sure if this is a good name, maybe TST_FILL_BLOCKS or > TST_FILL_PAGES. hhmm, OK blocks then. > >> + TST_FILL_RANDOM >> +}; > > Otherwise the whole patchset looks good to me. > > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Thanks
Hi Richie, > Depending on the workload, the way in which a file system is filled > will be different. The system calls will also be different. > This adds a fill mode which uses writev with uniform batches of > data. This simulates when the FS is filled by a program which batches > writes. ... FYI this broke CI on Alpine. > +++ b/lib/tst_fill_fs.c ... > +void fill_flat_vec(const char *path, int verbose) > +{ > + int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); Alpine has problem with O_FSYNC: In file included from ../include/tst_test.h:110, from tst_fill_fs.c:13: tst_fill_fs.c: In function 'fill_flat_vec': tst_fill_fs.c:76:58: error: 'O_FSYNC' undeclared (first use in this function); did you mean 'O_ASYNC'? 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); | ^~~~~~~ ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ | ^~~~~~ tst_fill_fs.c:76:58: note: each undeclared identifier is reported only once for each function it appears in 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); | ^~~~~~~ ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ | ^~~~~~ make: *** [../include/mk/rules.mk:15: tst_fill_fs.o] Error 1 It actually does not have it in fcntl.h (and nowhere in the sources). Kind regards, Petr
Hi! > > Depending on the workload, the way in which a file system is filled > > will be different. The system calls will also be different. > > > This adds a fill mode which uses writev with uniform batches of > > data. This simulates when the FS is filled by a program which batches > > writes. > ... > > FYI this broke CI on Alpine. > > > +++ b/lib/tst_fill_fs.c > ... > > +void fill_flat_vec(const char *path, int verbose) > > +{ > > + int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); > > Alpine has problem with O_FSYNC: > > In file included from ../include/tst_test.h:110, > from tst_fill_fs.c:13: > tst_fill_fs.c: In function 'fill_flat_vec': > tst_fill_fs.c:76:58: error: 'O_FSYNC' undeclared (first use in this function); did you mean 'O_ASYNC'? > 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); > | ^~~~~~~ > ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' > 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ > | ^~~~~~ > tst_fill_fs.c:76:58: note: each undeclared identifier is reported only once for each function it appears in > 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); > | ^~~~~~~ > ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' > 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ > | ^~~~~~ > make: *** [../include/mk/rules.mk:15: tst_fill_fs.o] Error 1 > > It actually does not have it in fcntl.h (and nowhere in the sources). O_FSYNC originates from BSD it should be O_SYNC on Linux, but given that this is only file descriptor passed to later openat() we can just remove it instead since it does not make sense at all.
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> > Depending on the workload, the way in which a file system is filled >> > will be different. The system calls will also be different. >> >> > This adds a fill mode which uses writev with uniform batches of >> > data. This simulates when the FS is filled by a program which batches >> > writes. >> ... >> >> FYI this broke CI on Alpine. >> >> > +++ b/lib/tst_fill_fs.c >> ... >> > +void fill_flat_vec(const char *path, int verbose) >> > +{ >> > + int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); >> >> Alpine has problem with O_FSYNC: >> >> In file included from ../include/tst_test.h:110, >> from tst_fill_fs.c:13: >> tst_fill_fs.c: In function 'fill_flat_vec': >> tst_fill_fs.c:76:58: error: 'O_FSYNC' undeclared (first use in this function); did you mean 'O_ASYNC'? >> 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); >> | ^~~~~~~ >> ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' >> 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ >> | ^~~~~~ >> tst_fill_fs.c:76:58: note: each undeclared identifier is reported only once for each function it appears in >> 76 | int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); >> | ^~~~~~~ >> ../include/tst_safe_macros.h:90:58: note: in definition of macro 'SAFE_OPEN' >> 90 | safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ >> | ^~~~~~ >> make: *** [../include/mk/rules.mk:15: tst_fill_fs.o] Error 1 >> >> It actually does not have it in fcntl.h (and nowhere in the sources). > > O_FSYNC originates from BSD it should be O_SYNC on Linux, but given that > this is only file descriptor passed to later openat() we can just remove > it instead since it does not make sense at all. Ah, that was supposed to be added to the next open. However I guess it can just be removed.
diff --git a/include/tst_fs.h b/include/tst_fs.h index cc6d9b547..43ead32a2 100644 --- a/include/tst_fs.h +++ b/include/tst_fs.h @@ -34,6 +34,11 @@ #define TST_VFAT_MAGIC 0x4d44 /* AKA MSDOS */ #define TST_EXFAT_MAGIC 0x2011BAB0UL +enum tst_fill_access_pattern { + TST_FILL_FLAT_VEC, + TST_FILL_RANDOM +}; + enum { TST_BYTES = 1, TST_KB = 1024, @@ -201,7 +206,7 @@ int tst_fs_in_skiplist(const char *fs_type, const char *const *skiplist); /* * Creates and writes to files on given path until write fails with ENOSPC */ -void tst_fill_fs(const char *path, int verbose); +void tst_fill_fs(const char *path, int verbose, enum tst_fill_access_pattern pattern); /* * test if FIBMAP ioctl is supported diff --git a/lib/tst_fill_fs.c b/lib/tst_fill_fs.c index 1d6d76abd..e7d5d73b7 100644 --- a/lib/tst_fill_fs.c +++ b/lib/tst_fill_fs.c @@ -7,13 +7,16 @@ #include <stdio.h> #include <stdlib.h> #include <sys/statvfs.h> +#include <sys/uio.h> #define TST_NO_DEFAULT_MAIN #include "tst_test.h" +#include "lapi/fcntl.h" #include "tst_fs.h" #include "tst_rand_data.h" +#include "tst_safe_file_at.h" -void tst_fill_fs(const char *path, int verbose) +void fill_random(const char *path, int verbose) { int i = 0; char file[PATH_MAX]; @@ -67,3 +70,57 @@ void tst_fill_fs(const char *path, int verbose) SAFE_CLOSE(fd); } } + +void fill_flat_vec(const char *path, int verbose) +{ + int dir = SAFE_OPEN(path, O_PATH | O_DIRECTORY | O_FSYNC); + int fd = SAFE_OPENAT(dir, "AOF", O_WRONLY | O_CREAT, 0600); + struct iovec iov[512]; + int iovcnt = ARRAY_SIZE(iov); + int retries = 3; + + SAFE_CLOSE(dir); + + for (int i = 0; i < iovcnt; i++) { + iov[i] = (struct iovec) { + (void *)tst_rand_data, + tst_rand_data_len + }; + } + + while (retries) { + const int ret = writev(fd, iov, iovcnt); + + if (!ret) + tst_res(TWARN | TERRNO, "writev returned 0; not sure what this means"); + + if (ret > -1) { + if (verbose && retries < 3) + tst_res(TINFO, "writev(\"%s/AOF\", iov, %d) = %d", path, iovcnt, ret); + + retries = 3; + continue; + } + + if (errno != ENOSPC) + tst_brk(TBROK | TERRNO, "writev(\"%s/AOF\", iov, %d)", path, iovcnt); + + if (verbose) + tst_res(TINFO, "writev(\"%s/AOF\", iov, %d): ENOSPC", path, iovcnt); + + retries--; + } + + SAFE_CLOSE(fd); +} + +void tst_fill_fs(const char *path, int verbose, enum tst_fill_access_pattern pattern) +{ + + switch (pattern) { + case TST_FILL_FLAT_VEC: + return fill_flat_vec(path, verbose); + case TST_FILL_RANDOM: + return fill_random(path, verbose); + } +} diff --git a/testcases/kernel/fs/fs_fill/fs_fill.c b/testcases/kernel/fs/fs_fill/fs_fill.c index 95dfc2cb6..2027b6df1 100644 --- a/testcases/kernel/fs/fs_fill/fs_fill.c +++ b/testcases/kernel/fs/fs_fill/fs_fill.c @@ -25,6 +25,7 @@ static int enospc_cnt; static struct worker *workers; struct worker { + enum tst_fill_access_pattern pattern; char dir[PATH_MAX]; }; @@ -36,7 +37,7 @@ static void *worker(void *p) char file[PATH_MAX]; while (run) { - tst_fill_fs(w->dir, 0); + tst_fill_fs(w->dir, 1, w->pattern); tst_atomic_inc(&enospc_cnt); @@ -61,22 +62,26 @@ static void *worker(void *p) return NULL; } -static void testrun(void) +static void testrun(unsigned int n) { pthread_t threads[nthreads]; unsigned int i, ms; + tst_atomic_store(0, &enospc_cnt); + run = 1; - for (i = 0; i < nthreads; i++) + for (i = 0; i < nthreads; i++) { + workers[i].pattern = n; SAFE_PTHREAD_CREATE(&threads[i], NULL, worker, &workers[i]); + } for (ms = 0; ; ms++) { usleep(1000); - if (ms >= 1000 && enospc_cnt) + if (ms >= 1000 && tst_atomic_load(&enospc_cnt)) break; - if (enospc_cnt > 100) + if (tst_atomic_load(&enospc_cnt) > 100) break; } @@ -116,5 +121,6 @@ static struct tst_test test = { .all_filesystems = 1, .setup = setup, .cleanup = cleanup, - .test_all = testrun, + .test = testrun, + .tcnt = 2 }; diff --git a/testcases/kernel/syscalls/fallocate/fallocate05.c b/testcases/kernel/syscalls/fallocate/fallocate05.c index d67c6cf5b..af6bf9e8c 100644 --- a/testcases/kernel/syscalls/fallocate/fallocate05.c +++ b/testcases/kernel/syscalls/fallocate/fallocate05.c @@ -68,7 +68,7 @@ static void run(void) tst_brk(TBROK | TTERRNO, "fallocate(fd, 0, 0, %ld)", bufsize); } - tst_fill_fs(MNTPOINT, 1); + tst_fill_fs(MNTPOINT, 1, TST_FILL_RANDOM); TEST(write(fd, buf, bufsize)); diff --git a/testcases/kernel/syscalls/fallocate/fallocate06.c b/testcases/kernel/syscalls/fallocate/fallocate06.c index 16f9db066..124fb7eae 100644 --- a/testcases/kernel/syscalls/fallocate/fallocate06.c +++ b/testcases/kernel/syscalls/fallocate/fallocate06.c @@ -202,7 +202,7 @@ static void run(unsigned int n) } if (tc->fill_fs) - tst_fill_fs(MNTPOINT, 1); + tst_fill_fs(MNTPOINT, 1, TST_FILL_RANDOM); SAFE_LSEEK(fd, offset, SEEK_SET); TEST(write(fd, wbuf, size));
Depending on the workload, the way in which a file system is filled will be different. The system calls will also be different. This adds a fill mode which uses writev with uniform batches of data. This simulates when the FS is filled by a program which batches writes. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- include/tst_fs.h | 7 ++- lib/tst_fill_fs.c | 59 ++++++++++++++++++- testcases/kernel/fs/fs_fill/fs_fill.c | 18 ++++-- .../kernel/syscalls/fallocate/fallocate05.c | 2 +- .../kernel/syscalls/fallocate/fallocate06.c | 2 +- 5 files changed, 78 insertions(+), 10 deletions(-)