| Message ID | 20220121100604.1072-1-pvorel@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] aiodio: Skip tests on tmpfs | expand |
Hi all,
[Cc Li ]
> tmpfs does not support it.
Verifying with .all_filesystems, that only tmpfs is problematic
- tested on exfat, ntfs over FUSE, vfat (common linux filesystems obviously work)
That's why I avoided opening file with O_DIRECT in setup().
BTW I wonder what is the reason of duplicate entries in ltp-aiodio.part4?
i.e. dio_sparse, aiodio_append, dio_append, dio_truncate...
To create bigger load?
IMHO it'd make sense to run at least one test on .all_filesystems to check more
filesystems (dio_append seems to be quick), but I wouldn't done it when there
are duplicate entries.
Kind regards,
Petr
Hi! On 1/21/22 11:06, Petr Vorel wrote: > tmpfs does not support it. > > Signed-off-by: Petr Vorel<pvorel@suse.cz> > --- > testcases/kernel/io/ltp-aiodio/aiodio_append.c | 10 ++++++++-- > testcases/kernel/io/ltp-aiodio/dio_append.c | 4 ++++ > testcases/kernel/io/ltp-aiodio/dio_read.c | 4 ++++ > testcases/kernel/io/ltp-aiodio/dio_sparse.c | 10 ++++++++-- > testcases/kernel/io/ltp-aiodio/dio_truncate.c | 10 ++++++++-- > 5 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_append.c b/testcases/kernel/io/ltp-aiodio/aiodio_append.c > index cb04b04a57..46cc74ee4e 100644 > --- a/testcases/kernel/io/ltp-aiodio/aiodio_append.c > +++ b/testcases/kernel/io/ltp-aiodio/aiodio_append.c > @@ -131,8 +131,10 @@ static void setup(void) > > static void cleanup(void) > { > - *run_child = 0; > - SAFE_MUNMAP(run_child, sizeof(int)); > + if (run_child) { > + *run_child = 0; > + SAFE_MUNMAP(run_child, sizeof(int)); SAFE_MUNMAP should be called always, even if *run_child == 0 (which happens if run() completed successfully). > + } > } > > static void run(void) > @@ -177,6 +179,10 @@ static struct tst_test test = { > {"b:", &str_numaio, "Number of async IO blocks (default 16)"}, > {} > }, > + .skip_filesystems = (const char *[]) { > + "tmpfs", > + NULL > + }, > }; > #else > TST_TEST_TCONF("test requires libaio and its development packages"); > diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c > index 59fd710e70..c099793f6c 100644 > --- a/testcases/kernel/io/ltp-aiodio/dio_append.c > +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c > @@ -93,4 +93,8 @@ static struct tst_test test = { > {"c:", &str_appends, "Number of appends (default 1000)"}, > {} > }, > + .skip_filesystems = (const char *[]) { > + "tmpfs", > + NULL > + }, > }; > diff --git a/testcases/kernel/io/ltp-aiodio/dio_read.c b/testcases/kernel/io/ltp-aiodio/dio_read.c > index 2c2ec4bce0..67a28147fd 100644 > --- a/testcases/kernel/io/ltp-aiodio/dio_read.c > +++ b/testcases/kernel/io/ltp-aiodio/dio_read.c > @@ -177,4 +177,8 @@ static struct tst_test test = { > {"s:", &str_filesize, "File size (default 128M)"}, > {} > }, > + .skip_filesystems = (const char *[]) { > + "tmpfs", > + NULL > + }, > }; > diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c > index 4ee2fbab18..39fc895d65 100644 > --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c > +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c > @@ -83,8 +83,10 @@ static void setup(void) > > static void cleanup(void) > { > - *run_child = 0; > - SAFE_MUNMAP(run_child, sizeof(int)); > + if (run_child) { > + *run_child = 0; > + SAFE_MUNMAP(run_child, sizeof(int)); > + } > } > > static void run(void) > @@ -129,4 +131,8 @@ static struct tst_test test = { > {"o:", &str_offset, "File offset (default 0)"}, > {} > }, > + .skip_filesystems = (const char *[]) { > + "tmpfs", > + NULL > + }, > }; > diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c > index 4bf11c9588..1fbf83de06 100644 > --- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c > +++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c > @@ -107,8 +107,10 @@ static void setup(void) > > static void cleanup(void) > { > - *run_child = 0; > - SAFE_MUNMAP(run_child, sizeof(int)); > + if (run_child) { > + *run_child = 0; > + SAFE_MUNMAP(run_child, sizeof(int)); > + } > } > > static void run(void) > @@ -163,4 +165,8 @@ static struct tst_test test = { > {"c:", &str_numwrites, "Number of append & truncate (default 100)"}, > {} > }, > + .skip_filesystems = (const char *[]) { > + "tmpfs", > + NULL > + }, > }; Kind regards, Andrea
Hi!
> SAFE_MUNMAP should be called always, even if *run_child == 0 (which happens if run() completed successfully).
This is actually if (run_child) not if (*run_child) and it fixes the
case where the test is skipped even before the run_child has been
mmaped.
Hi! > > tmpfs does not support it. > Verifying with .all_filesystems, that only tmpfs is problematic > - tested on exfat, ntfs over FUSE, vfat (common linux filesystems obviously work) > > That's why I avoided opening file with O_DIRECT in setup(). > > BTW I wonder what is the reason of duplicate entries in ltp-aiodio.part4? > i.e. dio_sparse, aiodio_append, dio_append, dio_truncate... > To create bigger load? > > IMHO it'd make sense to run at least one test on .all_filesystems to check more > filesystems (dio_append seems to be quick), but I wouldn't done it when there > are duplicate entries. Historicall mess I guess. We will clean that up in the next development cycle.
> Hi! > > SAFE_MUNMAP should be called always, even if *run_child == 0 (which happens if run() completed successfully). > This is actually if (run_child) not if (*run_child) and it fixes the > case where the test is skipped even before the run_child has been > mmaped. Yes, that's needed for tmpfs. Kind regards, Petr
Hi!
Looks good to me, but since we are right before release I would be more
happier if this gets more than one review before it gets pushed.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Hi! > Looks good to me, but since we are right before release I would be more > happier if this gets more than one review before it gets pushed. Definitely. Li, Andrea, Martin, could you please have a look? Kind regards, Petr > Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hi! On 1/21/22 11:16, Petr Vorel wrote: > Hi all, > > [Cc Li ] > >> tmpfs does not support it. > Verifying with .all_filesystems, that only tmpfs is problematic > - tested on exfat, ntfs over FUSE, vfat (common linux filesystems obviously work) > > That's why I avoided opening file with O_DIRECT in setup(). > > BTW I wonder what is the reason of duplicate entries in ltp-aiodio.part4? > i.e. dio_sparse, aiodio_append, dio_append, dio_truncate... > To create bigger load? To duplicate entries in ltp-aiodio.part4 doesn't make much sense and I perfectly agree with you. Once the aiodio testing suite is finished, we do need to change runtest file in order to have multiple scenarios with different files/buffers size with a different number of sub-processes. > > IMHO it'd make sense to run at least one test on .all_filesystems to check more > filesystems (dio_append seems to be quick), but I wouldn't done it when there > are duplicate entries. > > Kind regards, > Petr >
It makes sense to avoid running tests using O_DIRECT on tmpfs, so patch looks good to me. On 1/21/22 11:43, Petr Vorel wrote: >> Hi! >> Looks good to me, but since we are right before release I would be more >> happier if this gets more than one review before it gets pushed. > Definitely. Li, Andrea, Martin, could you please have a look? > > Kind regards, > Petr > >> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Kind regards, Andrea
Hi, one tiny nit below, otherwise: Reviewed-by: Martin Doucha <mdoucha@suse.cz> On 21. 01. 22 11:06, Petr Vorel wrote: > tmpfs does not support it. What's "it"? ^ > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/kernel/io/ltp-aiodio/aiodio_append.c | 10 ++++++++-- > testcases/kernel/io/ltp-aiodio/dio_append.c | 4 ++++ > testcases/kernel/io/ltp-aiodio/dio_read.c | 4 ++++ > testcases/kernel/io/ltp-aiodio/dio_sparse.c | 10 ++++++++-- > testcases/kernel/io/ltp-aiodio/dio_truncate.c | 10 ++++++++-- > 5 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_append.c b/testcases/kernel/io/ltp-aiodio/aiodio_append.c
Hi! > > tmpfs does not support it. > > What's "it"? ^ Should be replaced by O_DIRECT I guess.
Hi Martin, > Hi, > one tiny nit below, otherwise: > Reviewed-by: Martin Doucha <mdoucha@suse.cz> Thanks! > On 21. 01. 22 11:06, Petr Vorel wrote: > > tmpfs does not support it. > What's "it"? ^ +1, I'll add O_DIRECT. Kind regards, Petr
diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_append.c b/testcases/kernel/io/ltp-aiodio/aiodio_append.c index cb04b04a57..46cc74ee4e 100644 --- a/testcases/kernel/io/ltp-aiodio/aiodio_append.c +++ b/testcases/kernel/io/ltp-aiodio/aiodio_append.c @@ -131,8 +131,10 @@ static void setup(void) static void cleanup(void) { - *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + if (run_child) { + *run_child = 0; + SAFE_MUNMAP(run_child, sizeof(int)); + } } static void run(void) @@ -177,6 +179,10 @@ static struct tst_test test = { {"b:", &str_numaio, "Number of async IO blocks (default 16)"}, {} }, + .skip_filesystems = (const char *[]) { + "tmpfs", + NULL + }, }; #else TST_TEST_TCONF("test requires libaio and its development packages"); diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c index 59fd710e70..c099793f6c 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_append.c +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c @@ -93,4 +93,8 @@ static struct tst_test test = { {"c:", &str_appends, "Number of appends (default 1000)"}, {} }, + .skip_filesystems = (const char *[]) { + "tmpfs", + NULL + }, }; diff --git a/testcases/kernel/io/ltp-aiodio/dio_read.c b/testcases/kernel/io/ltp-aiodio/dio_read.c index 2c2ec4bce0..67a28147fd 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_read.c +++ b/testcases/kernel/io/ltp-aiodio/dio_read.c @@ -177,4 +177,8 @@ static struct tst_test test = { {"s:", &str_filesize, "File size (default 128M)"}, {} }, + .skip_filesystems = (const char *[]) { + "tmpfs", + NULL + }, }; diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c index 4ee2fbab18..39fc895d65 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c @@ -83,8 +83,10 @@ static void setup(void) static void cleanup(void) { - *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + if (run_child) { + *run_child = 0; + SAFE_MUNMAP(run_child, sizeof(int)); + } } static void run(void) @@ -129,4 +131,8 @@ static struct tst_test test = { {"o:", &str_offset, "File offset (default 0)"}, {} }, + .skip_filesystems = (const char *[]) { + "tmpfs", + NULL + }, }; diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c index 4bf11c9588..1fbf83de06 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c +++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c @@ -107,8 +107,10 @@ static void setup(void) static void cleanup(void) { - *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + if (run_child) { + *run_child = 0; + SAFE_MUNMAP(run_child, sizeof(int)); + } } static void run(void) @@ -163,4 +165,8 @@ static struct tst_test test = { {"c:", &str_numwrites, "Number of append & truncate (default 100)"}, {} }, + .skip_filesystems = (const char *[]) { + "tmpfs", + NULL + }, };
tmpfs does not support it. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testcases/kernel/io/ltp-aiodio/aiodio_append.c | 10 ++++++++-- testcases/kernel/io/ltp-aiodio/dio_append.c | 4 ++++ testcases/kernel/io/ltp-aiodio/dio_read.c | 4 ++++ testcases/kernel/io/ltp-aiodio/dio_sparse.c | 10 ++++++++-- testcases/kernel/io/ltp-aiodio/dio_truncate.c | 10 ++++++++-- 5 files changed, 32 insertions(+), 6 deletions(-)