2018-10-11 19:37:55

by Rafael Tinoco

[permalink] [raw]
Subject: [PATCH] proc: fix proc-self-map-files selftest for arm

MAP_FIXED is important for this test but, unfortunately, lowest virtual
address for user space mapping on arm is (PAGE_SIZE * 2) and NULL hint
does not seem to guarantee that when MAP_FIXED is given. This patch sets
the virtual address that will hold the mapping for the test, fixing the
issue.

Link: https://bugs.linaro.org/show_bug.cgi?id=3782
Signed-off-by: Rafael David Tinoco <[email protected]>
---
tools/testing/selftests/proc/proc-self-map-files-002.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/proc/proc-self-map-files-002.c b/tools/testing/selftests/proc/proc-self-map-files-002.c
index 6f1f4a6e1ecb..0a47eaca732a 100644
--- a/tools/testing/selftests/proc/proc-self-map-files-002.c
+++ b/tools/testing/selftests/proc/proc-self-map-files-002.c
@@ -55,7 +55,9 @@ int main(void)
if (fd == -1)
return 1;

- p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);
+ p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE,
+ MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);
+
if (p == MAP_FAILED) {
if (errno == EPERM)
return 2;
--
2.19.1



2018-10-11 19:42:51

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Thu, Oct 11, 2018 at 03:43:59PM -0300, Rafael David Tinoco wrote:
> MAP_FIXED is important for this test but, unfortunately, lowest virtual
> address for user space mapping on arm is (PAGE_SIZE * 2) and NULL hint
> does not seem to guarantee that when MAP_FIXED is given. This patch sets
> the virtual address that will hold the mapping for the test, fixing the
> issue.
>
> Link: https://bugs.linaro.org/show_bug.cgi?id=3782
> Signed-off-by: Rafael David Tinoco <[email protected]>
> ---
> tools/testing/selftests/proc/proc-self-map-files-002.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/proc/proc-self-map-files-002.c b/tools/testing/selftests/proc/proc-self-map-files-002.c
> index 6f1f4a6e1ecb..0a47eaca732a 100644
> --- a/tools/testing/selftests/proc/proc-self-map-files-002.c
> +++ b/tools/testing/selftests/proc/proc-self-map-files-002.c
> @@ -55,7 +55,9 @@ int main(void)
> if (fd == -1)
> return 1;
>
> - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);
> + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE,
> + MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);
> +
> if (p == MAP_FAILED) {
> if (errno == EPERM)
> return 2;

As far as I remember nil virtual address has been there only to be sure the
vma allocated won't be merged with another vmas. Fore sure most of x86
standart application won't be using 8K address as well, so should do the
trick I think.

(Strictlly speaking the test should be rather parsing own maps first and
find unused address instead but whatever :)

Reviewed-by: Cyrill Gorcunov <[email protected]>

2018-10-11 20:57:08

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Thu, Oct 11, 2018 at 03:43:59PM -0300, Rafael David Tinoco wrote:
> MAP_FIXED is important for this test but, unfortunately, lowest virtual
> address for user space mapping on arm is (PAGE_SIZE * 2) and NULL hint
> does not seem to guarantee that when MAP_FIXED is given. This patch sets
> the virtual address that will hold the mapping for the test, fixing the
> issue.
>
> Link: https://bugs.linaro.org/show_bug.cgi?id=3782
> Signed-off-by: Rafael David Tinoco <[email protected]>
> ---
> tools/testing/selftests/proc/proc-self-map-files-002.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/proc/proc-self-map-files-002.c b/tools/testing/selftests/proc/proc-self-map-files-002.c
> index 6f1f4a6e1ecb..0a47eaca732a 100644
> --- a/tools/testing/selftests/proc/proc-self-map-files-002.c
> +++ b/tools/testing/selftests/proc/proc-self-map-files-002.c
> @@ -55,7 +55,9 @@ int main(void)
> if (fd == -1)
> return 1;
>
> - p = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);
> + p = mmap((void *) (2 * PAGE_SIZE), PAGE_SIZE, PROT_NONE,
> + MAP_PRIVATE|MAP_FILE|MAP_FIXED, fd, 0);

As the comment in the beginning says this test is specifically for addresss 0.
Maybe it should be ifdeffed with __arm__ then.

2018-10-11 21:03:54

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Thu, Oct 11, 2018 at 11:56:01PM +0300, Alexey Dobriyan wrote:
>
> As the comment in the beginning says this test is specifically for addresss 0.
> Maybe it should be ifdeffed with __arm__ then.

Is there some other reason than allocating non-mergable VMA?

2018-10-11 21:30:49

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Fri, Oct 12, 2018 at 12:02:56AM +0300, Cyrill Gorcunov wrote:
> On Thu, Oct 11, 2018 at 11:56:01PM +0300, Alexey Dobriyan wrote:
> >
> > As the comment in the beginning says this test is specifically for addresss 0.
> > Maybe it should be ifdeffed with __arm__ then.
>
> Is there some other reason than allocating non-mergable VMA?

IIRC the reason is to test address 0 as it is effectively banned
for userspace so if it will be broken, it will be broken silently
for a long time.

As for "unmergeable" libc here doesn't map /dev/zero. I know how to
avoid even theoretical breakage by creating binaries by hand but it
will be probably too much.

2018-10-11 22:01:17

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Fri, Oct 12, 2018 at 12:30:06AM +0300, Alexey Dobriyan wrote:
> On Fri, Oct 12, 2018 at 12:02:56AM +0300, Cyrill Gorcunov wrote:
> > On Thu, Oct 11, 2018 at 11:56:01PM +0300, Alexey Dobriyan wrote:
> > >
> > > As the comment in the beginning says this test is specifically for addresss 0.
> > > Maybe it should be ifdeffed with __arm__ then.
> >
> > Is there some other reason than allocating non-mergable VMA?
>
> IIRC the reason is to test address 0 as it is effectively banned
> for userspace so if it will be broken, it will be broken silently
> for a long time.

This is rather a side effect of the test because the primary reason
was to check procfs numbers conversion, right? Don't get me wrong,
I don't mind about __arm__ define or similar, this is fine for
one architecture, but if there comes more we will get a number
of #ifdefs which is unrelated to procfs numeric routines at all.

> As for "unmergeable" libc here doesn't map /dev/zero. I know how to
> avoid even theoretical breakage by creating binaries by hand but it
> will be probably too much.

Sure.

2018-10-15 16:57:26

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On 10/11/18 7:00 PM, Cyrill Gorcunov wrote:
> On Fri, Oct 12, 2018 at 12:30:06AM +0300, Alexey Dobriyan wrote:
>> On Fri, Oct 12, 2018 at 12:02:56AM +0300, Cyrill Gorcunov wrote:
>>> On Thu, Oct 11, 2018 at 11:56:01PM +0300, Alexey Dobriyan wrote:
>>>>
>>>> As the comment in the beginning says this test is specifically for addresss 0.
>>>> Maybe it should be ifdeffed with __arm__ then.
>>>
>>> Is there some other reason than allocating non-mergable VMA?
>>
>> IIRC the reason is to test address 0 as it is effectively banned
>> for userspace so if it will be broken, it will be broken silently
>> for a long time.
>
> This is rather a side effect of the test because the primary reason
> was to check procfs numbers conversion, right? Don't get me wrong,
> I don't mind about __arm__ define or similar, this is fine for
> one architecture, but if there comes more we will get a number
> of #ifdefs which is unrelated to procfs numeric routines at all.

That is what I also had in mind, thus the patch. I just realized we had
another issue on LKFT (our functional tests tool) for
proc-self-map-files-001.c. Test 001 does pretty much the same as 002,
but without the MAP_FIXED mmap flag.

Is it okay to consolidate both tests into just 1, and focus in checking
procfs numbers conversion only, rather than if mapping 0 is allowed or
not ? Can I send a v2 with that in mind ?

>
>> As for "unmergeable" libc here doesn't map /dev/zero. I know how to
>> avoid even theoretical breakage by creating binaries by hand but it
>> will be probably too much.
>
> Sure.
>


2018-10-15 17:22:26

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Mon, Oct 15, 2018 at 01:55:14PM -0300, Rafael David Tinoco wrote:
> That is what I also had in mind, thus the patch. I just realized we had
> another issue on LKFT (our functional tests tool) for
> proc-self-map-files-001.c. Test 001 does pretty much the same as 002, but
> without the MAP_FIXED mmap flag.
>
> Is it okay to consolidate both tests into just 1, and focus in checking
> procfs numbers conversion only, rather than if mapping 0 is allowed or not ?
> Can I send a v2 with that in mind ?

As to me -- yes, I would move zero page testing to a separate memory testcase.
But since Alexey is the former author of the tests better wait for his opinion.

2018-11-08 10:43:02

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Mon, Oct 15, 2018 at 2:21 PM, Cyrill Gorcunov <[email protected]> wrote:
> On Mon, Oct 15, 2018 at 01:55:14PM -0300, Rafael David Tinoco wrote:
>> That is what I also had in mind, thus the patch. I just realized we had
>> another issue on LKFT (our functional tests tool) for
>> proc-self-map-files-001.c. Test 001 does pretty much the same as 002, but
>> without the MAP_FIXED mmap flag.
>>
>> Is it okay to consolidate both tests into just 1, and focus in checking
>> procfs numbers conversion only, rather than if mapping 0 is allowed or not ?
>> Can I send a v2 with that in mind ?
>
> As to me -- yes, I would move zero page testing to a separate memory testcase.
> But since Alexey is the former author of the tests better wait for his opinion.

Alexey,

would you care if we turn those 2 tests into 1, taking care of the
zero page testing elsewhere ? Would you mind if I send out a patch for
that ?

2018-11-08 11:13:13

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix proc-self-map-files selftest for arm

On Thu, Nov 08, 2018 at 08:41:46AM -0200, Rafael David Tinoco wrote:
> >
> > As to me -- yes, I would move zero page testing to a separate memory testcase.
> > But since Alexey is the former author of the tests better wait for his opinion.
>
> Alexey,
>
> would you care if we turn those 2 tests into 1, taking care of the
> zero page testing elsewhere ? Would you mind if I send out a patch for
> that ?

Rafael, just make it so. While we preserve old functionality
I think it is fine.

Cyrill

2018-11-09 11:31:27

by Rafael Tinoco

[permalink] [raw]
Subject: [PATCH] proc: fix and merge proc-self-map-file tests

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 <[email protected]>
---
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

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 <[email protected]>
- *
- * 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;
-}
--
2.19.1


2018-11-09 11:42:25

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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 <[email protected]>

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.

2018-11-09 11:46:20

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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 <[email protected]>
>
> 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!

2018-11-09 11:49:38

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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.

2018-11-09 12:02:01

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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)

2018-11-09 18:05:03

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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

2018-11-09 18:50:33

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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...

2018-11-09 19:40:18

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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)?

2018-11-10 17:48:06

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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?

2018-11-10 17:58:06

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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

2018-11-10 18:50:39

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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.

2018-11-11 02:52:45

by Rafael Tinoco

[permalink] [raw]
Subject: Re: [PATCH] proc: fix and merge proc-self-map-file tests

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