| Message ID | 20181109113036.21512-1-rafael.tinoco@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | proc: fix and merge proc-self-map-file tests | expand |
On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > Merge proc-self-map-files tests into one since this test should focus in > testing readlink in /proc/self/map_files/* only, and not trying to test > mapping virtual address 0. > > Lowest virtual address for user space mapping in other architectures, > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > guarantee that when MAP_FIXED flag, important to this test, is given. > This patch also fixes this issue in remaining test. > > Link: https://bugs.linaro.org/show_bug.cgi?id=3782 > Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org> Wait, Rafael. But we will loose the test of mapping virtual address 0 then. I though you would move testing of virtual address 0 into a separate testcase. I mean testing of first page was a positive side effect of the former Alexey's patch, so we definitely should keep it on x86 at least. Gimme some time I'll try to address it today evening or tomorrow. I think this way everybody will be happy: procfs get passed on arm32 and x86 will still have first page testing.
On Fri, Nov 9, 2018, at 9:41 AM, Cyrill Gorcunov wrote: > On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > > Merge proc-self-map-files tests into one since this test should focus in > > testing readlink in /proc/self/map_files/* only, and not trying to test > > mapping virtual address 0. > > > > Lowest virtual address for user space mapping in other architectures, > > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > > guarantee that when MAP_FIXED flag, important to this test, is given. > > This patch also fixes this issue in remaining test. > > > > Link: https://bugs.linaro.org/show_bug.cgi?id=3782 > > Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org> > > Wait, Rafael. But we will loose the test of mapping virtual address 0 > then. I though you would move testing of virtual address 0 into > a separate testcase. I mean testing of first page was a positive > side effect of the former Alexey's patch, so we definitely should > keep it on x86 at least. Gimme some time I'll try to address it > today evening or tomorrow. I think this way everybody will be > happy: procfs get passed on arm32 and x86 will still have first > page testing. Ohh, my understanding was that this was going to be addressed in some other test, like what you said.. I did not understand you wanted me to create a test for it altogether, my bad. I can do it if you want, let me know, pls. Thanks!
On Fri, Nov 09, 2018 at 09:45:36AM -0200, Rafael David Tinoco wrote: ... > > today evening or tomorrow. I think this way everybody will be > > happy: procfs get passed on arm32 and x86 will still have first > > page testing. > > Ohh, my understanding was that this was going to be addressed in some other test, > like what you said.. I did not understand you wanted me to create a test for > it altogether, my bad. I can do it if you want, let me know, pls. :-) I'll be able to create patch and test in about 10 hours, so feel free to beat me if you have time meanwhile.
On Fri, Nov 9, 2018, at 9:48 AM, Cyrill Gorcunov wrote: > On Fri, Nov 09, 2018 at 09:45:36AM -0200, Rafael David Tinoco wrote: > ... > > > today evening or tomorrow. I think this way everybody will be > > > happy: procfs get passed on arm32 and x86 will still have first > > > page testing. > > > > Ohh, my understanding was that this was going to be addressed in some other test, > > like what you said.. I did not understand you wanted me to create a test for > > it altogether, my bad. I can do it if you want, let me know, pls. > > :-) I'll be able to create patch and test in about 10 hours, so > feel free to beat me if you have time meanwhile. Alright, I'm fixing membarrier_test before, so.. I guess we have a competition.. =o)
On Fri, Nov 09, 2018 at 10:01:13AM -0200, Rafael David Tinoco wrote: > > Alright, I'm fixing membarrier_test before, so.. I guess we have a competition.. =o) Rafael, Alexey, what about simply wrap the test code with x86 and extend later with all archs which support zero address mapping? --- tools/testing/selftests/proc/proc-self-map-files-002.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) Index: linux-ml.git/tools/testing/selftests/proc/proc-self-map-files-002.c =================================================================== --- linux-ml.git.orig/tools/testing/selftests/proc/proc-self-map-files-002.c +++ linux-ml.git/tools/testing/selftests/proc/proc-self-map-files-002.c @@ -23,6 +23,11 @@ #include <sys/mman.h> #include <stdlib.h> +/* + * Should run on archs which support zero address mapping. + */ +#if defined(__i386) || defined(__x86_64) + static void pass(const char *fmt, unsigned long a, unsigned long b) { char name[64]; @@ -83,3 +88,12 @@ int main(void) return 0; } + +#else + +int main(void) +{ + return 0; +} + +#endif
On Fri, Nov 9, 2018, at 4:04 PM, Cyrill Gorcunov wrote: > On Fri, Nov 09, 2018 at 10:01:13AM -0200, Rafael David Tinoco wrote: > > > > Alright, I'm fixing membarrier_test before, so.. I guess we have a competition.. =o) > > Rafael, Alexey, what about simply wrap the test code with x86 and extend later > with all archs which support zero address mapping? > --- > tools/testing/selftests/proc/proc-self-map-files-002.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > Index: linux-ml.git/tools/testing/selftests/proc/proc-self-map-files-002.c > =================================================================== > --- linux-ml.git.orig/tools/testing/selftests/proc/proc-self-map-files-002.c > +++ linux-ml.git/tools/testing/selftests/proc/proc-self-map-files-002.c > @@ -23,6 +23,11 @@ > #include <sys/mman.h> > #include <stdlib.h> > > +/* > + * Should run on archs which support zero address mapping. > + */ > +#if defined(__i386) || defined(__x86_64) > + > static void pass(const char *fmt, unsigned long a, unsigned long b) > { > char name[64]; > @@ -83,3 +88,12 @@ int main(void) > > return 0; > } > + > +#else > + > +int main(void) > +{ > + return 0; > +} > + > +#endif let me see if I got this right.. the premise for this test is to have *at least* 2 vmas, so we can check if the symlink for the mem range, describing the mapped area, is correct in procfs files, correct ? if yes, then why to have a totally duplicated test... just to check if mmap(0, ... MAP_FIXED ...) would work ? Wouldn't exist a better place to have such test ? like in tools/testing/selftests/vm/mmap-null.c or something like it ? genuine curiosity.. thinking i'm missing something about this test...
On Fri, Nov 09, 2018 at 04:48:49PM -0200, Rafael David Tinoco wrote: > > let me see if I got this right.. the premise for this test is to have *at least* > 2 vmas, so we can check if the symlink for the mem range, describing the mapped > area, is correct in procfs files, correct ? if yes, then why to have a totally > duplicated test... just to check if mmap(0, ... MAP_FIXED ...) would work ? > > Wouldn't exist a better place to have such test ? like in > tools/testing/selftests/vm/mmap-null.c or something like it ? genuine > curiosity.. thinking i'm missing something about this test... Ah, I happen to miss that they are identical except nil address. Then true, vm/ looks like more suitable place for that. Do you happen to know which exactly archs reserve first page (together with x86)?
On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > Merge proc-self-map-files tests into one since this test should focus in > testing readlink in /proc/self/map_files/* only, and not trying to test > mapping virtual address 0. > > Lowest virtual address for user space mapping in other architectures, > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > guarantee that when MAP_FIXED flag, important to this test, is given. > This patch also fixes this issue in remaining test. > - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE, fd, 0); > + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE, I don't know ARM. Is this 2 page limitation a limitation of hardware or kernel's?
On Sat, Nov 10, 2018, at 3:47 PM, Alexey Dobriyan wrote: > On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > > Merge proc-self-map-files tests into one since this test should focus in > > testing readlink in /proc/self/map_files/* only, and not trying to test > > mapping virtual address 0. > > > > Lowest virtual address for user space mapping in other architectures, > > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > > guarantee that when MAP_FIXED flag, important to this test, is given. > > This patch also fixes this issue in remaining test. > > > - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE, fd, 0); > > + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE, > > I don't know ARM. Is this 2 page limitation a limitation of hardware or > kernel's? Kernel: https://bugs.linaro.org/show_bug.cgi?id=3782#c7
On Sat, Nov 10, 2018 at 03:56:03PM -0200, Rafael David Tinoco wrote: > On Sat, Nov 10, 2018, at 3:47 PM, Alexey Dobriyan wrote: > > On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > > > Merge proc-self-map-files tests into one since this test should focus in > > > testing readlink in /proc/self/map_files/* only, and not trying to test > > > mapping virtual address 0. > > > > > > Lowest virtual address for user space mapping in other architectures, > > > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > > > guarantee that when MAP_FIXED flag, important to this test, is given. > > > This patch also fixes this issue in remaining test. > > > > > - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE, fd, 0); > > > + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE, > > > > I don't know ARM. Is this 2 page limitation a limitation of hardware or > > kernel's? > > Kernel: > https://bugs.linaro.org/show_bug.cgi?id=3782#c7 Ahh. please test the path I've sent, I don't have arm install readily available.
On Sat, Nov 10, 2018, at 4:49 PM, Alexey Dobriyan wrote: > On Sat, Nov 10, 2018 at 03:56:03PM -0200, Rafael David Tinoco wrote: > > On Sat, Nov 10, 2018, at 3:47 PM, Alexey Dobriyan wrote: > > > On Fri, Nov 09, 2018 at 09:30:36AM -0200, Rafael David Tinoco wrote: > > > > Merge proc-self-map-files tests into one since this test should focus in > > > > testing readlink in /proc/self/map_files/* only, and not trying to test > > > > mapping virtual address 0. > > > > > > > > Lowest virtual address for user space mapping in other architectures, > > > > like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not > > > > guarantee that when MAP_FIXED flag, important to this test, is given. > > > > This patch also fixes this issue in remaining test. > > > > > > > - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE, fd, 0); > > > > + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE, > > > > > > I don't know ARM. Is this 2 page limitation a limitation of hardware or > > > kernel's? > > > > Kernel: > > https://bugs.linaro.org/show_bug.cgi?id=3782#c7 > > Ahh. please test the path I've sent, I don't have arm install readily > available. I replied to your patch based on some of the discussion we had in this thread. Thanks Rafael - Rafael D. Tinoco Linaro Kernel Validation Team
diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore index 82121a81681f..d44ec8755879 100644 --- a/tools/testing/selftests/proc/.gitignore +++ b/tools/testing/selftests/proc/.gitignore @@ -3,7 +3,6 @@ /fd-003-kthread /proc-loadavg-001 /proc-self-map-files-001 -/proc-self-map-files-002 /proc-self-syscall /proc-self-wchan /proc-uptime-001 diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile index 1c12c34cf85d..6c17557c2f9a 100644 --- a/tools/testing/selftests/proc/Makefile +++ b/tools/testing/selftests/proc/Makefile @@ -7,7 +7,6 @@ TEST_GEN_PROGS += fd-002-posix-eq TEST_GEN_PROGS += fd-003-kthread TEST_GEN_PROGS += proc-loadavg-001 TEST_GEN_PROGS += proc-self-map-files-001 -TEST_GEN_PROGS += proc-self-map-files-002 TEST_GEN_PROGS += proc-self-syscall TEST_GEN_PROGS += proc-self-wchan TEST_GEN_PROGS += proc-uptime-001 diff --git a/tools/testing/selftests/proc/proc-self-map-files-001.c b/tools/testing/selftests/proc/proc-self-map-files-001.c index 4209c64283d6..646d8d3fba3a 100644 --- a/tools/testing/selftests/proc/proc-self-map-files-001.c +++ b/tools/testing/selftests/proc/proc-self-map-files-001.c @@ -46,16 +46,17 @@ static void fail(const char *fmt, unsigned long a, unsigned long b) int main(void) { - const unsigned int PAGE_SIZE = sysconf(_SC_PAGESIZE); void *p; int fd; unsigned long a, b; + const long PAGE_SIZE = sysconf(_SC_PAGESIZE); fd = open("/dev/zero", O_RDONLY); if (fd == -1) return 1; - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE, fd, 0); + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE, + MAP_PRIVATE|MAP_FILE, fd, 0); if (p == MAP_FAILED) return 1; diff --git a/tools/testing/selftests/proc/proc-self-map-files-002.c b/tools/testing/selftests/proc/proc-self-map-files-002.c deleted file mode 100644 index 6f1f4a6e1ecb..000000000000 --- a/tools/testing/selftests/proc/proc-self-map-files-002.c +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright © 2018 Alexey Dobriyan <adobriyan@gmail.com> - * - * Permission to use, copy, modify, and distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. - */ -/* Test readlink /proc/self/map_files/... with address 0. */ -#include <errno.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> -#include <stdio.h> -#include <unistd.h> -#include <sys/mman.h> -#include <stdlib.h> - -static void pass(const char *fmt, unsigned long a, unsigned long b) -{ - char name[64]; - char buf[64]; - - snprintf(name, sizeof(name), fmt, a, b); - if (readlink(name, buf, sizeof(buf)) == -1) - exit(1); -} - -static void fail(const char *fmt, unsigned long a, unsigned long b) -{ - char name[64]; - char buf[64]; - - snprintf(name, sizeof(name), fmt, a, b); - if (readlink(name, buf, sizeof(buf)) == -1 && errno == ENOENT) - return; - exit(1); -} - -int main(void) -{ - const unsigned int PAGE_SIZE = sysconf(_SC_PAGESIZE); - void *p; - int fd; - unsigned long a, b; - - fd = open("/dev/zero", O_RDONLY); - if (fd == -1) - return 1; - - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0); - if (p == MAP_FAILED) { - if (errno == EPERM) - return 2; - return 1; - } - - a = (unsigned long)p; - b = (unsigned long)p + PAGE_SIZE; - - pass("/proc/self/map_files/%lx-%lx", a, b); - fail("/proc/self/map_files/ %lx-%lx", a, b); - fail("/proc/self/map_files/%lx -%lx", a, b); - fail("/proc/self/map_files/%lx- %lx", a, b); - fail("/proc/self/map_files/%lx-%lx ", a, b); - fail("/proc/self/map_files/0%lx-%lx", a, b); - fail("/proc/self/map_files/%lx-0%lx", a, b); - if (sizeof(long) == 4) { - fail("/proc/self/map_files/100000000%lx-%lx", a, b); - fail("/proc/self/map_files/%lx-100000000%lx", a, b); - } else if (sizeof(long) == 8) { - fail("/proc/self/map_files/10000000000000000%lx-%lx", a, b); - fail("/proc/self/map_files/%lx-10000000000000000%lx", a, b); - } else - return 1; - - return 0; -}
Merge proc-self-map-files tests into one since this test should focus in testing readlink in /proc/self/map_files/* only, and not trying to test mapping virtual address 0. Lowest virtual address for user space mapping in other architectures, like arm, is *at least* *(PAGE_SIZE * 2) and NULL hint does not guarantee that when MAP_FIXED flag, important to this test, is given. This patch also fixes this issue in remaining test. Link: https://bugs.linaro.org/show_bug.cgi?id=3782 Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org> --- tools/testing/selftests/proc/.gitignore | 1 - tools/testing/selftests/proc/Makefile | 1 - .../selftests/proc/proc-self-map-files-001.c | 5 +- .../selftests/proc/proc-self-map-files-002.c | 85 ------------------- 4 files changed, 3 insertions(+), 89 deletions(-) delete mode 100644 tools/testing/selftests/proc/proc-self-map-files-002.c -- 2.19.1