Message ID | 1528228739-27731-1-git-send-email-adrian@lisas.de |
---|---|
State | Accepted |
Series | "zdtm/static/maps06: removed hardcoded page-size" |
Commit | 0bb02253e67c75bb3829d993b6622e9742d1fe22 |
Headers | show |
diff --git a/test/zdtm/static/maps06.c b/test/zdtm/static/maps06.c index 14dd90f..7480d6b 100644 --- a/test/zdtm/static/maps06.c +++ b/test/zdtm/static/maps06.c @@ -10,12 +10,12 @@ const char *test_author = "Andrei Vagin <avagin@openvz.org>"; char *filename; TEST_OPTION(filename, string, "file name", 1); -#define TEST_SIZE 10240 - int main(int argc, char ** argv) { void *start; int fd, i; + int ps = sysconf(_SC_PAGESIZE); + int test_size; test_init(argc, argv); @@ -23,21 +23,26 @@ int main(int argc, char ** argv) if (fd < 0) return 1; - ftruncate(fd, 4096); + ftruncate(fd, ps); + + if (ps == 0x1000) + test_size = 10240; + else + test_size = 512; - start = mmap(0, 4096 * TEST_SIZE * 4, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); + start = mmap(0, ps * test_size * 4, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); if (start == MAP_FAILED) return 1; - for (i = 0; i < TEST_SIZE; i++) { + for (i = 0; i < test_size; i++) { int *addr; - addr = mmap(start + i * 3 * 4096, 4096, + addr = mmap(start + i * 3 * ps, ps, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FILE | MAP_FIXED, fd, 0); if (addr == MAP_FAILED) return 1; addr[0] = i * 2; - addr = mmap(start + (i * 3 + 1) * 4096, 4096, + addr = mmap(start + (i * 3 + 1) * ps, ps, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); if (addr == MAP_FAILED) @@ -49,12 +54,12 @@ int main(int argc, char ** argv) test_waitsig(); - for (i = 0; i < TEST_SIZE; i++) { + for (i = 0; i < test_size; i++) { int *addr; - addr = start + i * 3 * 4096; + addr = start + i * 3 * ps; if (addr[0] != i * 2) fail(); - addr = start + (i * 3 + 1) * 4096; + addr = start + (i * 3 + 1) * ps; if (addr[0] != i) fail(); }
Hi Adrian, Thanks for doing this! 2018-06-05 20:58 GMT+01:00 Adrian Reber <adrian@lisas.de>: [..] > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) > if (fd < 0) > return 1; > > - ftruncate(fd, 4096); > + ftruncate(fd, ps); > + > + if (ps == 0x1000) > + test_size = 10240; > + else > + test_size = 512; Is it worth to calculate test_size dynamically based on ps? Like will the test work on both 16k and 64k pages? Other than that LGTM, Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
On Wed, Jun 06, 2018 at 03:44:32PM +0100, Dmitry Safonov wrote: > 2018-06-05 20:58 GMT+01:00 Adrian Reber <adrian@lisas.de>: > [..] > > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) > > if (fd < 0) > > return 1; > > > > - ftruncate(fd, 4096); > > + ftruncate(fd, ps); > > + > > + if (ps == 0x1000) > > + test_size = 10240; > > + else > > + test_size = 512; > > Is it worth to calculate test_size dynamically based on ps? > Like will the test work on both 16k and 64k pages? Without that the test failed as the result (ps * test_size *4) for mmap()'s length overflowed. I have not checked if it works when changing ps or test_size to a 64bit value. Adrian
2018-06-06 15:52 GMT+01:00 Adrian Reber <areber@redhat.com>: > On Wed, Jun 06, 2018 at 03:44:32PM +0100, Dmitry Safonov wrote: >> 2018-06-05 20:58 GMT+01:00 Adrian Reber <adrian@lisas.de>: >> [..] >> > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) >> > if (fd < 0) >> > return 1; >> > >> > - ftruncate(fd, 4096); >> > + ftruncate(fd, ps); >> > + >> > + if (ps == 0x1000) >> > + test_size = 10240; >> > + else >> > + test_size = 512; >> >> Is it worth to calculate test_size dynamically based on ps? >> Like will the test work on both 16k and 64k pages? > > Without that the test failed as the result (ps * test_size *4) for mmap()'s > length overflowed. I have not checked if it works when changing ps or > test_size to a 64bit value. No, what I meant is that you've chosen 512 static value. Will the test work both for 16k and 64k pages? Or test_size will differ between 16k/64k - then probably it would be worth to calculate it with a division. Thanks, Dmitry
On Wed, Jun 06, 2018 at 04:12:51PM +0100, Dmitry Safonov wrote: > 2018-06-06 15:52 GMT+01:00 Adrian Reber <areber@redhat.com>: > > On Wed, Jun 06, 2018 at 03:44:32PM +0100, Dmitry Safonov wrote: > >> 2018-06-05 20:58 GMT+01:00 Adrian Reber <adrian@lisas.de>: > >> [..] > >> > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) > >> > if (fd < 0) > >> > return 1; > >> > > >> > - ftruncate(fd, 4096); > >> > + ftruncate(fd, ps); > >> > + > >> > + if (ps == 0x1000) > >> > + test_size = 10240; > >> > + else > >> > + test_size = 512; > >> > >> Is it worth to calculate test_size dynamically based on ps? > >> Like will the test work on both 16k and 64k pages? > > > > Without that the test failed as the result (ps * test_size *4) for mmap()'s > > length overflowed. I have not checked if it works when changing ps or > > test_size to a 64bit value. > > No, what I meant is that you've chosen 512 static value. > Will the test work both for 16k and 64k pages? > Or test_size will differ between 16k/64k - then probably > it would be worth to calculate it with a division. I first thought that was what you were asking, but then I was not so sure any more. Yes, for 16K pages it is not perfect. That is correct. I only tested with 64K a 4K pages. I thought about a division, but wanted to keep it simple. I can do a new version with a division. Adrian
2018-06-06 16:47 GMT+01:00 Adrian Reber <areber@redhat.com>: > On Wed, Jun 06, 2018 at 04:12:51PM +0100, Dmitry Safonov wrote: >> 2018-06-06 15:52 GMT+01:00 Adrian Reber <areber@redhat.com>: >> > On Wed, Jun 06, 2018 at 03:44:32PM +0100, Dmitry Safonov wrote: >> >> 2018-06-05 20:58 GMT+01:00 Adrian Reber <adrian@lisas.de>: >> >> [..] >> >> > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) >> >> > if (fd < 0) >> >> > return 1; >> >> > >> >> > - ftruncate(fd, 4096); >> >> > + ftruncate(fd, ps); >> >> > + >> >> > + if (ps == 0x1000) >> >> > + test_size = 10240; >> >> > + else >> >> > + test_size = 512; >> >> >> >> Is it worth to calculate test_size dynamically based on ps? >> >> Like will the test work on both 16k and 64k pages? >> > >> > Without that the test failed as the result (ps * test_size *4) for mmap()'s >> > length overflowed. I have not checked if it works when changing ps or >> > test_size to a 64bit value. >> >> No, what I meant is that you've chosen 512 static value. >> Will the test work both for 16k and 64k pages? >> Or test_size will differ between 16k/64k - then probably >> it would be worth to calculate it with a division. > > I first thought that was what you were asking, but then I was not so > sure any more. Yes, for 16K pages it is not perfect. That is correct. > > I only tested with 64K a 4K pages. > > I thought about a division, but wanted to keep it simple. > > I can do a new version with a division. I'm not really sure if it's worth to create a new version with a division. If it you expect it to work with the current code on 16k, it's fine for me. I haven't carefully looked at code to catch if it will or will not work with 512 value. Thanks, Dmitry
Applied, thanks! On Tue, Jun 05, 2018 at 07:58:59PM +0000, Adrian Reber wrote: > From: Adrian Reber <areber@redhat.com> > > zdtm/static/maps06 failed on systems with different page-size than 4096. > This changes maps06 to use sysconf(_SC_PAGESIZE) instead. > > Signed-off-by: Adrian Reber <areber@redhat.com> > --- > test/zdtm/static/maps06.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/test/zdtm/static/maps06.c b/test/zdtm/static/maps06.c > index 14dd90f..7480d6b 100644 > --- a/test/zdtm/static/maps06.c > +++ b/test/zdtm/static/maps06.c > @@ -10,12 +10,12 @@ const char *test_author = "Andrei Vagin <avagin@openvz.org>"; > char *filename; > TEST_OPTION(filename, string, "file name", 1); > > -#define TEST_SIZE 10240 > - > int main(int argc, char ** argv) > { > void *start; > int fd, i; > + int ps = sysconf(_SC_PAGESIZE); > + int test_size; > > test_init(argc, argv); > > @@ -23,21 +23,26 @@ int main(int argc, char ** argv) > if (fd < 0) > return 1; > > - ftruncate(fd, 4096); > + ftruncate(fd, ps); > + > + if (ps == 0x1000) > + test_size = 10240; > + else > + test_size = 512; > > - start = mmap(0, 4096 * TEST_SIZE * 4, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); > + start = mmap(0, ps * test_size * 4, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0); > if (start == MAP_FAILED) > return 1; > > - for (i = 0; i < TEST_SIZE; i++) { > + for (i = 0; i < test_size; i++) { > int *addr; > - addr = mmap(start + i * 3 * 4096, 4096, > + addr = mmap(start + i * 3 * ps, ps, > PROT_READ | PROT_WRITE, > MAP_PRIVATE | MAP_FILE | MAP_FIXED, fd, 0); > if (addr == MAP_FAILED) > return 1; > addr[0] = i * 2; > - addr = mmap(start + (i * 3 + 1) * 4096, 4096, > + addr = mmap(start + (i * 3 + 1) * ps, ps, > PROT_READ | PROT_WRITE, > MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); > if (addr == MAP_FAILED) > @@ -49,12 +54,12 @@ int main(int argc, char ** argv) > > test_waitsig(); > > - for (i = 0; i < TEST_SIZE; i++) { > + for (i = 0; i < test_size; i++) { > int *addr; > - addr = start + i * 3 * 4096; > + addr = start + i * 3 * ps; > if (addr[0] != i * 2) > fail(); > - addr = start + (i * 3 + 1) * 4096; > + addr = start + (i * 3 + 1) * ps; > if (addr[0] != i) > fail(); > } > -- > 1.8.3.1 >