[PATCH v2 26/39] tests/tcg: make test-mmap a little less aggressive

Alex Bennée posted 39 patches 4 years, 7 months ago
Maintainers: Brad Smith <brad@comstyle.com>, Reinoud Zandijk <reinoud@netbsd.org>, Kamil Rytarowski <kamil@netbsd.org>, Ryo ONODERA <ryoon@netbsd.org>
[PATCH v2 26/39] tests/tcg: make test-mmap a little less aggressive
Posted by Alex Bennée 4 years, 7 months ago
The check_aligned_anonymous_unfixed_mmaps and
check_aligned_anonymous_unfixed_colliding_mmaps do a lot of mmap's and
copying of data. This is especially unfriendly to targets like hexagon
which have quite large pages and need to do sanity checks on each
memory access.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/multiarch/test-mmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
index 11d0e777b1..b77deee37e 100644
--- a/tests/tcg/multiarch/test-mmap.c
+++ b/tests/tcg/multiarch/test-mmap.c
@@ -58,12 +58,12 @@ void check_aligned_anonymous_unfixed_mmaps(void)
 	int i;
 
 	fprintf(stdout, "%s", __func__);
-	for (i = 0; i < 0x1fff; i++)
+	for (i = 0; i < 0x1ff; i++)
 	{
 		size_t len;
 
 		len = pagesize + (pagesize * i & 7);
-		p1 = mmap(NULL, len, PROT_READ, 
+		p1 = mmap(NULL, len, PROT_READ,
 			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 		p2 = mmap(NULL, len, PROT_READ, 
 			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
@@ -142,7 +142,7 @@ void check_aligned_anonymous_unfixed_colliding_mmaps(void)
 	int i;
 
 	fprintf(stdout, "%s", __func__);
-	for (i = 0; i < 0x2fff; i++)
+	for (i = 0; i < 0x2ff; i++)
 	{
 		int nlen;
 		p1 = mmap(NULL, pagesize, PROT_READ, 
-- 
2.20.1


Re: [PATCH v2 26/39] tests/tcg: make test-mmap a little less aggressive
Posted by Richard Henderson 4 years, 7 months ago
On 7/8/21 12:09 PM, Alex Bennée wrote:
> -	for (i = 0; i < 0x1fff; i++)
> +	for (i = 0; i < 0x1ff; i++)
>   	{
>   		size_t len;
>   
>   		len = pagesize + (pagesize * i & 7);

There's really no point in i >= 8.

We release all of the memory at the end of the loop; we'll probably get back the same 
pages on the 8'th iteration.


> -	for (i = 0; i < 0x2fff; i++)
> +	for (i = 0; i < 0x2ff; i++)

I'm not sure why this one is iterating more than twice?


r~

Re: [PATCH v2 26/39] tests/tcg: make test-mmap a little less aggressive
Posted by Alex Bennée 4 years, 7 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 7/8/21 12:09 PM, Alex Bennée wrote:
>> -	for (i = 0; i < 0x1fff; i++)
>> +	for (i = 0; i < 0x1ff; i++)
>>   	{
>>   		size_t len;
>>     		len = pagesize + (pagesize * i & 7);
>
> There's really no point in i >= 8.
>
> We release all of the memory at the end of the loop; we'll probably
> get back the same pages on the 8'th iteration.
>
>
>> -	for (i = 0; i < 0x2fff; i++)
>> +	for (i = 0; i < 0x2ff; i++)
>
> I'm not sure why this one is iterating more than twice?

OK cutting it down to 8 and 2 with white space cleanup and removing the
timeout hack gives:

--8<---------------cut here---------------start------------->8---
tests/tcg: make test-mmap a little less aggressive

The check_aligned_anonymous_unfixed_mmaps and
check_aligned_anonymous_unfixed_colliding_mmaps do a lot of mmap's and
copying of data. This is especially unfriendly to targets like hexagon
which have quite large pages and need to do sanity checks on each
memory access.

While we are at it clean-up the white space and style issues from the
legacy code. As we no longer do quite so much needless memory access
we can also remove the hexagon timeout hack.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - style and whitespace cleanups, reduce loop even further.
  - remove hexagon timeout hack

2 files changed, 102 insertions(+), 113 deletions(-)
tests/tcg/multiarch/test-mmap.c   | 206 +++++++++++++++++++-------------------
tests/tcg/hexagon/Makefile.target |   9 --

modified   tests/tcg/multiarch/test-mmap.c
@@ -49,64 +49,62 @@ size_t test_fsize;
 
 void check_aligned_anonymous_unfixed_mmaps(void)
 {
-	void *p1;
-	void *p2;
-	void *p3;
-	void *p4;
-	void *p5;
-	uintptr_t p;
-	int i;
-
-	fprintf(stdout, "%s", __func__);
-	for (i = 0; i < 0x1fff; i++)
-	{
-		size_t len;
-
-		len = pagesize + (pagesize * i & 7);
-		p1 = mmap(NULL, len, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		p2 = mmap(NULL, len, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		p3 = mmap(NULL, len, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		p4 = mmap(NULL, len, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		p5 = mmap(NULL, len, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-
-		/* Make sure we get pages aligned with the pagesize. The
-		   target expects this.  */
-		fail_unless (p1 != MAP_FAILED);
-		fail_unless (p2 != MAP_FAILED);
-		fail_unless (p3 != MAP_FAILED);
-		fail_unless (p4 != MAP_FAILED);
-		fail_unless (p5 != MAP_FAILED);
-		p = (uintptr_t) p1;
-		D(printf ("p=%x\n", p));
-		fail_unless ((p & pagemask) == 0);
-		p = (uintptr_t) p2;
-		fail_unless ((p & pagemask) == 0);
-		p = (uintptr_t) p3;
-		fail_unless ((p & pagemask) == 0);
-		p = (uintptr_t) p4;
-		fail_unless ((p & pagemask) == 0);
-		p = (uintptr_t) p5;
-		fail_unless ((p & pagemask) == 0);
-
-		/* Make sure we can read from the entire area.  */
-		memcpy (dummybuf, p1, pagesize);
-		memcpy (dummybuf, p2, pagesize);
-		memcpy (dummybuf, p3, pagesize);
-		memcpy (dummybuf, p4, pagesize);
-		memcpy (dummybuf, p5, pagesize);
-
-		munmap (p1, len);
-		munmap (p2, len);
-		munmap (p3, len);
-		munmap (p4, len);
-		munmap (p5, len);
-	}
-	fprintf(stdout, " passed\n");
+    void *p1;
+    void *p2;
+    void *p3;
+    void *p4;
+    void *p5;
+    uintptr_t p;
+    int i;
+    fprintf(stdout, "%s", __func__);
+    for (i = 0; i < 8; i++) {
+        size_t len;
+        len = pagesize + (pagesize * i);
+        p1 = mmap(NULL, len, PROT_READ,
+                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        p2 = mmap(NULL, len, PROT_READ,
+                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        p3 = mmap(NULL, len, PROT_READ,
+                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        p4 = mmap(NULL, len, PROT_READ,
+                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        p5 = mmap(NULL, len, PROT_READ,
+                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+        /*
+         * Make sure we get pages aligned with the pagesize. The
+         * target expects this.
+         */
+        fail_unless (p1 != MAP_FAILED);
+        fail_unless (p2 != MAP_FAILED);
+        fail_unless (p3 != MAP_FAILED);
+        fail_unless (p4 != MAP_FAILED);
+        fail_unless (p5 != MAP_FAILED);
+        p = (uintptr_t) p1;
+        D(printf ("p=%x\n", p));
+        fail_unless ((p & pagemask) == 0);
+        p = (uintptr_t) p2;
+        fail_unless ((p & pagemask) == 0);
+        p = (uintptr_t) p3;
+        fail_unless ((p & pagemask) == 0);
+        p = (uintptr_t) p4;
+        fail_unless ((p & pagemask) == 0);
+        p = (uintptr_t) p5;
+        fail_unless ((p & pagemask) == 0);
+
+        /* Make sure we can read from the entire area.  */
+        memcpy (dummybuf, p1, pagesize);
+        memcpy (dummybuf, p2, pagesize);
+        memcpy (dummybuf, p3, pagesize);
+        memcpy (dummybuf, p4, pagesize);
+        memcpy (dummybuf, p5, pagesize);
+        munmap (p1, len);
+        munmap (p2, len);
+        munmap (p3, len);
+        munmap (p4, len);
+        munmap (p5, len);
+    }
+    fprintf(stdout, " passed\n");
 }
 
 void check_large_anonymous_unfixed_mmap(void)
@@ -135,52 +133,52 @@ void check_large_anonymous_unfixed_mmap(void)
 
 void check_aligned_anonymous_unfixed_colliding_mmaps(void)
 {
-	char *p1;
-	char *p2;
-	char *p3;
-	uintptr_t p;
-	int i;
-
-	fprintf(stdout, "%s", __func__);
-	for (i = 0; i < 0x2fff; i++)
-	{
-		int nlen;
-		p1 = mmap(NULL, pagesize, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		fail_unless (p1 != MAP_FAILED);
-		p = (uintptr_t) p1;
-		fail_unless ((p & pagemask) == 0);
-		memcpy (dummybuf, p1, pagesize);
-
-		p2 = mmap(NULL, pagesize, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		fail_unless (p2 != MAP_FAILED);
-		p = (uintptr_t) p2;
-		fail_unless ((p & pagemask) == 0);
-		memcpy (dummybuf, p2, pagesize);
-
-
-		munmap (p1, pagesize);
-		nlen = pagesize * 8;
-		p3 = mmap(NULL, nlen, PROT_READ, 
-			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-		fail_unless (p3 != MAP_FAILED);
-
-		/* Check if the mmaped areas collide.  */
-		if (p3 < p2 
-		    && (p3 + nlen) > p2)
-			fail_unless (0);
-
-		memcpy (dummybuf, p3, pagesize);
-
-		/* Make sure we get pages aligned with the pagesize. The
-		   target expects this.  */
-		p = (uintptr_t) p3;
-		fail_unless ((p & pagemask) == 0);
-		munmap (p2, pagesize);
-		munmap (p3, nlen);
-	}
-	fprintf(stdout, " passed\n");
+    char *p1;
+    char *p2;
+    char *p3;
+    uintptr_t p;
+    int i;
+
+    fprintf(stdout, "%s", __func__);
+    for (i = 0; i < 2; i++) {
+        int nlen;
+        p1 = mmap(NULL, pagesize, PROT_READ,
+              MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        fail_unless (p1 != MAP_FAILED);
+        p = (uintptr_t) p1;
+        fail_unless ((p & pagemask) == 0);
+        memcpy (dummybuf, p1, pagesize);
+
+        p2 = mmap(NULL, pagesize, PROT_READ,
+              MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        fail_unless (p2 != MAP_FAILED);
+        p = (uintptr_t) p2;
+        fail_unless ((p & pagemask) == 0);
+        memcpy (dummybuf, p2, pagesize);
+
+
+        munmap (p1, pagesize);
+        nlen = pagesize * 8;
+        p3 = mmap(NULL, nlen, PROT_READ,
+              MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        fail_unless (p3 != MAP_FAILED);
+
+        /* Check if the mmaped areas collide.  */
+        if (p3 < p2
+            && (p3 + nlen) > p2) {
+            fail_unless (0);
+        }
+
+        memcpy (dummybuf, p3, pagesize);
+
+        /* Make sure we get pages aligned with the pagesize. The
+           target expects this.  */
+        p = (uintptr_t) p3;
+        fail_unless ((p & pagemask) == 0);
+        munmap (p2, pagesize);
+        munmap (p3, nlen);
+    }
+    fprintf(stdout, " passed\n");
 }
 
 void check_aligned_anonymous_fixed_mmaps(void)
modified   tests/tcg/hexagon/Makefile.target
@@ -18,15 +18,6 @@
 # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
 EXTRA_RUNS =
 
-# Hexagon has 64K pages, so increase the timeout to keep
-# test-mmap from timing out
-ifeq ($(CONFIG_DEBUG_TCG),y)
-TIMEOUT=800
-else
-TIMEOUT=500
-endif
-
-
 CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
 CFLAGS += -fno-unroll-loops
--8<---------------cut here---------------end--------------->8---

-- 
Alex Bennée

Re: [PATCH v2 26/39] tests/tcg: make test-mmap a little less aggressive
Posted by Thomas Huth 4 years, 7 months ago
On 08/07/2021 21.09, Alex Bennée wrote:
> The check_aligned_anonymous_unfixed_mmaps and
> check_aligned_anonymous_unfixed_colliding_mmaps do a lot of mmap's and
> copying of data. This is especially unfriendly to targets like hexagon
> which have quite large pages and need to do sanity checks on each
> memory access.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/tcg/multiarch/test-mmap.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
> index 11d0e777b1..b77deee37e 100644
> --- a/tests/tcg/multiarch/test-mmap.c
> +++ b/tests/tcg/multiarch/test-mmap.c
> @@ -58,12 +58,12 @@ void check_aligned_anonymous_unfixed_mmaps(void)
>   	int i;
>   
>   	fprintf(stdout, "%s", __func__);
> -	for (i = 0; i < 0x1fff; i++)
> +	for (i = 0; i < 0x1ff; i++)
>   	{

While you're at it, you could also fix the coding style here and put the 
curly bracket on the right hand side of the for-statement.

>   		size_t len;
>   
>   		len = pagesize + (pagesize * i & 7);
> -		p1 = mmap(NULL, len, PROT_READ,
> +		p1 = mmap(NULL, len, PROT_READ,
>   			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>   		p2 = mmap(NULL, len, PROT_READ,
>   			  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> @@ -142,7 +142,7 @@ void check_aligned_anonymous_unfixed_colliding_mmaps(void)
>   	int i;
>   
>   	fprintf(stdout, "%s", __func__);
> -	for (i = 0; i < 0x2fff; i++)
> +	for (i = 0; i < 0x2ff; i++)

dito

>   	{
>   		int nlen;
>   		p1 = mmap(NULL, pagesize, PROT_READ,
> 

  Thomas