| Message ID | 20230908103921.511595-1-shirisha@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] Migrating the libhugetlbfs/testcases/truncate.c test | expand |
Hi! > +/*\ > + * [Description] > + * > + * Test Name: truncate > + * Test case is used to verify the correct functionality > + * and compatibility of the library with the "truncate" system call when > + * operating on files residing in a mounted huge page filesystem. > + */ > + > +#include "hugetlb.h" > + > +#define RANDOM_CONSTANT 0x1234ABCD ^ THis is not used at all. > +#define MNTPOINT "hugetlbfs/" > +long hpage_size; > +int fd; These two should be static. > + > + > +static void sigbus_handler(int signum LTP_ATTRIBUTE_UNUSED) > +{ > + tst_res(TPASS, "Test Passed"); > + exit(0); It's not safe to call exit(0) from a signal handler. What should be done instead is to: - add global static volatile int variable - reset it before we attempt to access the truncated memory - set it in the signal handler - print TPASS/TFAIL based on the value of the variable in the run_test() function > +} > + > +static void run_test(void) > +{ > + void *p; > + volatile unsigned int *q; ^ I do not think that this has to be volatile. All in all this can be just: unsigned int *p; ... p = SAFE_MMAP(); ... *p = 0; > + struct sigaction my_sigaction; > + my_sigaction.sa_handler = sigbus_handler; > + p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, > + fd, 0); > + if (p == MAP_FAILED) > + tst_res(TFAIL, "mmap failed..!!"); SAFE_MMAP() cannot fail, it does exit the test with a failure if the it fails to map the memory. > + q = p; > + *q = 0; > + SAFE_SIGACTION(SIGBUS, &my_sigaction, NULL); I guess that we can set up the handler in the setup instead. > + SAFE_FTRUNCATE(fd, 0); > + *q; > + tst_res(TFAIL, "Didn't SIGBUS"); And we should SAFE_UNMAP() the memory here. Also does the test work with -i 2 ? > +} > + > + > +void setup(void) > +{ > + hpage_size = tst_get_hugepage_size(); > + fd = tst_creat_unlinked(MNTPOINT, 0); ^ Wrong indentation, please make sure to run 'make check' and fix all the reported problems. > +} > + > +void cleanup(void) > +{ > + if (fd > 0) > + SAFE_CLOSE(fd); > +} > + > +static struct tst_test test = { > + .needs_root = 1, > + .mntpoint = MNTPOINT, > + .needs_hugetlbfs = 1, > + .needs_tmpdir = 1, > + .setup = setup, > + .cleanup = cleanup, > + .test_all = run_test, > + .hugepages = {1, TST_NEEDS}, > +}; > -- > 2.39.3 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
On Fri, 2023-09-08 at 14:15 +0200, Cyril Hrubis wrote: > Hi! > > +/*\ > > + * [Description] > > + * > > + * Test Name: truncate > > + * Test case is used to verify the correct functionality > > + * and compatibility of the library with the "truncate" system > > call when > > + * operating on files residing in a mounted huge page filesystem. > > + */ > > + > > +#include "hugetlb.h" > > + > > +#define RANDOM_CONSTANT 0x1234ABCD > ^ > THis is not used at all. Will take care of this in v3. > > > +#define MNTPOINT "hugetlbfs/" > > +long hpage_size; > > +int fd; > > These two should be static. Will take care of this in v3. > > > + > > + > > +static void sigbus_handler(int signum LTP_ATTRIBUTE_UNUSED) > > +{ > > + tst_res(TPASS, "Test Passed"); > > + exit(0); > > It's not safe to call exit(0) from a signal handler. > > What should be done instead is to: > > - add global static volatile int variable > - reset it before we attempt to access the truncated memory > - set it in the signal handler > - print TPASS/TFAIL based on the value of the variable in the > run_test() > function > Will take care of this in v3. > > +} > > + > > +static void run_test(void) > > +{ > > + void *p; > > + volatile unsigned int *q; > ^ > I do not think that this has to be volatile. > > All in all this can be just: > > unsigned int *p; > > ... > > p = SAFE_MMAP(); > > ... > > *p = 0; This has to be volatile. The test logic is hunting for a bug that changes the value of q. > > > + struct sigaction my_sigaction; > > + my_sigaction.sa_handler = sigbus_handler; > > + p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, > > MAP_SHARED, > > + fd, 0); > > + if (p == MAP_FAILED) > > + tst_res(TFAIL, "mmap failed..!!"); > > SAFE_MMAP() cannot fail, it does exit the test with a failure if the > it > fails to map the memory. Will take care of this in v3. > > > + q = p; > > + *q = 0; > > + SAFE_SIGACTION(SIGBUS, &my_sigaction, NULL); > > I guess that we can set up the handler in the setup instead. Will take care of this in v3. > > > + SAFE_FTRUNCATE(fd, 0); > > + *q; > > + tst_res(TFAIL, "Didn't SIGBUS"); > > And we should SAFE_UNMAP() the memory here. Sure will take care of this in v3. > > Also does the test work with -i 2 ? The new Version ie., v3 works > > > +} > > + > > + > > +void setup(void) > > +{ > > + hpage_size = tst_get_hugepage_size(); > > + fd = tst_creat_unlinked(MNTPOINT, 0); > ^ > Wrong indentation, please make sure to run 'make check' and fix > all > the reported problems. Done in v3. > > > +} > > + > > +void cleanup(void) > > +{ > > + if (fd > 0) > > + SAFE_CLOSE(fd); > > +} > > + > > +static struct tst_test test = { > > + .needs_root = 1, > > + .mntpoint = MNTPOINT, > > + .needs_hugetlbfs = 1, > > + .needs_tmpdir = 1, > > + .setup = setup, > > + .cleanup = cleanup, > > + .test_all = run_test, > > + .hugepages = {1, TST_NEEDS}, > > +}; > > -- > > 2.39.3 > > > > > > -- > > Mailing list info: https://lists.linux.it/listinfo/ltp
diff --git a/runtest/hugetlb b/runtest/hugetlb index 299c07ac9..1300e80fb 100644 --- a/runtest/hugetlb +++ b/runtest/hugetlb @@ -35,6 +35,7 @@ hugemmap29 hugemmap29 hugemmap30 hugemmap30 hugemmap31 hugemmap31 hugemmap32 hugemmap32 +hugemmap33 hugemmap33 hugemmap05_1 hugemmap05 -m hugemmap05_2 hugemmap05 -s hugemmap05_3 hugemmap05 -s -m diff --git a/testcases/kernel/mem/.gitignore b/testcases/kernel/mem/.gitignore index 7258489ed..d130d4dcd 100644 --- a/testcases/kernel/mem/.gitignore +++ b/testcases/kernel/mem/.gitignore @@ -34,6 +34,7 @@ /hugetlb/hugemmap/hugemmap30 /hugetlb/hugemmap/hugemmap31 /hugetlb/hugemmap/hugemmap32 +/hugetlb/hugemmap/hugemmap33 /hugetlb/hugeshmat/hugeshmat01 /hugetlb/hugeshmat/hugeshmat02 /hugetlb/hugeshmat/hugeshmat03 diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap33.c b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap33.c new file mode 100644 index 000000000..a4a071b53 --- /dev/null +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap33.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2005-2006 IBM Corporation. + * Author: David Gibson & Adam Litke + */ + +/*\ + * [Description] + * + * Test Name: truncate + * + * Test case is used to verify the correct functionality + * and compatibility of the library with the "truncate" system call when + * operating on files residing in a mounted huge page filesystem. + */ + +#include "hugetlb.h" + +#define RANDOM_CONSTANT 0x1234ABCD +#define MNTPOINT "hugetlbfs/" +long hpage_size; +int fd; + + + +static void sigbus_handler(int signum LTP_ATTRIBUTE_UNUSED) +{ + tst_res(TPASS, "Test Passed"); + exit(0); +} + +static void run_test(void) +{ + void *p; + volatile unsigned int *q; + struct sigaction my_sigaction; + my_sigaction.sa_handler = sigbus_handler; + p = SAFE_MMAP(NULL, hpage_size, PROT_READ|PROT_WRITE, MAP_SHARED, + fd, 0); + if (p == MAP_FAILED) + tst_res(TFAIL, "mmap failed..!!"); + q = p; + *q = 0; + SAFE_SIGACTION(SIGBUS, &my_sigaction, NULL); + SAFE_FTRUNCATE(fd, 0); + *q; + tst_res(TFAIL, "Didn't SIGBUS"); +} + + +void setup(void) +{ + hpage_size = tst_get_hugepage_size(); + fd = tst_creat_unlinked(MNTPOINT, 0); +} + +void cleanup(void) +{ + if (fd > 0) + SAFE_CLOSE(fd); +} + +static struct tst_test test = { + .needs_root = 1, + .mntpoint = MNTPOINT, + .needs_hugetlbfs = 1, + .needs_tmpdir = 1, + .setup = setup, + .cleanup = cleanup, + .test_all = run_test, + .hugepages = {1, TST_NEEDS}, +};
Test Description: Test is used to verify the correct functionality and compatibility of the library with the "truncate" system call when operating on files residing in a mounted huge page filesystem. Signed-off-by: Shirisha G <shirisha@linux.ibm.com> --- v2: -Corrected typo --- runtest/hugetlb | 1 + testcases/kernel/mem/.gitignore | 1 + .../kernel/mem/hugetlb/hugemmap/hugemmap33.c | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 testcases/kernel/mem/hugetlb/hugemmap/hugemmap33.c