| Message ID | 20220519174151.14114-1-andrea.cervesato@suse.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1] Fix ltp-aiodio tests failing on s390 | expand |
Hi Andrea, > --- a/testcases/kernel/io/ltp-aiodio/dio_append.c > +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c > @@ -19,7 +19,7 @@ > #include "tst_test.h" > #include "common.h" > -static int *run_child; > +static volatile int *run_child; > static char *str_numchildren; > static char *str_writesize; > @@ -49,7 +49,10 @@ static void setup(void) > static void cleanup(void) > { > - SAFE_MUNMAP(run_child, sizeof(int)); > + if (run_child) { > + *run_child = 0; > + SAFE_MUNMAP((void *)run_child, sizeof(int)); > + } nit: This looks like unrelated, right? If yes it could be in separate commit. But I'm ok to merge it in single commit. Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi Petr, that was missing from the previous implementation indeed. Something that was checked in the other tests, but not this one. Probably because it's also the older test we refactor in the aiodio testing suite. It seems related to the bug and the volatile variable, so I added it as well. Andrea On 5/20/22 09:30, Petr Vorel wrote: > Hi Andrea, > >> --- a/testcases/kernel/io/ltp-aiodio/dio_append.c >> +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c >> @@ -19,7 +19,7 @@ >> #include "tst_test.h" >> #include "common.h" >> -static int *run_child; >> +static volatile int *run_child; >> static char *str_numchildren; >> static char *str_writesize; >> @@ -49,7 +49,10 @@ static void setup(void) >> static void cleanup(void) >> { >> - SAFE_MUNMAP(run_child, sizeof(int)); >> + if (run_child) { >> + *run_child = 0; >> + SAFE_MUNMAP((void *)run_child, sizeof(int)); >> + } > nit: This looks like unrelated, right? If yes it could be in separate commit. > But I'm ok to merge it in single commit. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Kind regards, > Petr >
Hi! > run_child flag is commonly used by aiodio tests in order to check if > tests are passed or not and compiler seems to optimize run_child flag ^ test has finished? > because it's not defined as volatile. With this patch we ensure that the > flag is set as volatile so tests will work as expected on s390. > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de> The change itself looks fine and should go in before the release. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hi all, > Hi! > > run_child flag is commonly used by aiodio tests in order to check if > > tests are passed or not and compiler seems to optimize run_child flag > ^ > test has finished? > > because it's not defined as volatile. With this patch we ensure that the > > flag is set as volatile so tests will work as expected on s390. > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de> > The change itself looks fine and should go in before the release. Merged with amended commit message. Thanks Andrea! Kind regards, Petr
diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_append.c b/testcases/kernel/io/ltp-aiodio/aiodio_append.c index 46cc74ee4..e42c577cd 100644 --- a/testcases/kernel/io/ltp-aiodio/aiodio_append.c +++ b/testcases/kernel/io/ltp-aiodio/aiodio_append.c @@ -24,7 +24,7 @@ #include <libaio.h> #include "common.h" -static int *run_child; +static volatile int *run_child; static char *str_numchildren; static char *str_writesize; @@ -133,7 +133,7 @@ static void cleanup(void) { if (run_child) { *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + SAFE_MUNMAP((void *)run_child, sizeof(int)); } } diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c b/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c index 2aa5662bb..9aa9b8d54 100644 --- a/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c +++ b/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c @@ -28,7 +28,7 @@ #include <libaio.h> #include "common.h" -static int *run_child; +static volatile int *run_child; static char *str_numchildren; static char *str_writesize; @@ -181,7 +181,7 @@ static void cleanup(void) { if (run_child) { *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + SAFE_MUNMAP((void *)run_child, sizeof(int)); } } diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c index c099793f6..3a7e7c836 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_append.c +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c @@ -19,7 +19,7 @@ #include "tst_test.h" #include "common.h" -static int *run_child; +static volatile int *run_child; static char *str_numchildren; static char *str_writesize; @@ -49,7 +49,10 @@ static void setup(void) static void cleanup(void) { - SAFE_MUNMAP(run_child, sizeof(int)); + if (run_child) { + *run_child = 0; + SAFE_MUNMAP((void *)run_child, sizeof(int)); + } } static void run(void) diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c index 0039daa8d..661afde4c 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c @@ -26,7 +26,7 @@ #include "tst_test.h" #include "common.h" -static int *run_child; +static volatile int *run_child; static char *str_numchildren; static char *str_writesize; @@ -85,7 +85,7 @@ static void cleanup(void) { if (run_child) { *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + SAFE_MUNMAP((void *)run_child, sizeof(int)); } } diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c index 1fbf83de0..0fe6b87ac 100644 --- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c +++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c @@ -33,7 +33,7 @@ #include "tst_test.h" #include "common.h" -static int *run_child; +static volatile int *run_child; static char *str_numchildren; static char *str_filesize; @@ -109,7 +109,7 @@ static void cleanup(void) { if (run_child) { *run_child = 0; - SAFE_MUNMAP(run_child, sizeof(int)); + SAFE_MUNMAP((void *)run_child, sizeof(int)); } }
run_child flag is commonly used by aiodio tests in order to check if tests are passed or not and compiler seems to optimize run_child flag because it's not defined as volatile. With this patch we ensure that the flag is set as volatile so tests will work as expected on s390. Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de> --- testcases/kernel/io/ltp-aiodio/aiodio_append.c | 4 ++-- testcases/kernel/io/ltp-aiodio/aiodio_sparse.c | 4 ++-- testcases/kernel/io/ltp-aiodio/dio_append.c | 7 +++++-- testcases/kernel/io/ltp-aiodio/dio_sparse.c | 4 ++-- testcases/kernel/io/ltp-aiodio/dio_truncate.c | 4 ++-- 5 files changed, 13 insertions(+), 10 deletions(-)