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

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

[Rafael David Tinoco]

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 <rafael.tinoco@linaro.org>
---
 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

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

[Cyrill Gorcunov]

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 <rafael.tinoco@linaro.org>
> ---
>  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 <gorcunov@gmail.com>

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

[Alexey Dobriyan]

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 <rafael.tinoco@linaro.org>
> ---
>  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.

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

[Cyrill Gorcunov]

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?

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

[Alexey Dobriyan]

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.

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

[Cyrill Gorcunov]

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.

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

[Rafael David Tinoco]

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

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

[Cyrill Gorcunov]

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.

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

[Rafael David Tinoco]

On Mon, Oct 15, 2018 at 2:21 PM, Cyrill Gorcunov <gorcunov@gmail.com> 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 ?

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

[Cyrill Gorcunov]

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

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

[Rafael David Tinoco]

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

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;
-}
-- 
2.19.1

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

[Cyrill Gorcunov]

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.

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

[Rafael David Tinoco]

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!

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

[Cyrill Gorcunov]

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.

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

[Rafael David Tinoco]

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)

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

[Cyrill Gorcunov]

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

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

[Rafael David Tinoco]

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

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

[Cyrill Gorcunov]

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

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

[Alexey Dobriyan]

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?

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

[Rafael David Tinoco]

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

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

[Alexey Dobriyan]

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.

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

[Rafael David Tinoco]

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