[PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output

Muhammad Usama Anjum posted 12 patches 1 year, 10 months ago
[PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output
Posted by Muhammad Usama Anjum 1 year, 10 months ago
Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 .../selftests/mm/virtual_address_range.c      | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
index bae0ceaf95b13..7bcf8d48256a6 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <sys/mman.h>
 #include <sys/time.h>
+#include "../kselftest.h"
 
 /*
  * Maximum address range mapped with a single mmap()
@@ -68,23 +69,15 @@ static char *hind_addr(void)
 	return (char *) (1UL << bits);
 }
 
-static int validate_addr(char *ptr, int high_addr)
+static void validate_addr(char *ptr, int high_addr)
 {
 	unsigned long addr = (unsigned long) ptr;
 
-	if (high_addr) {
-		if (addr < HIGH_ADDR_MARK) {
-			printf("Bad address %lx\n", addr);
-			return 1;
-		}
-		return 0;
-	}
+	if (high_addr && addr < HIGH_ADDR_MARK)
+		ksft_exit_fail_msg("Bad address %lx\n", addr);
 
-	if (addr > HIGH_ADDR_MARK) {
-		printf("Bad address %lx\n", addr);
-		return 1;
-	}
-	return 0;
+	if (addr > HIGH_ADDR_MARK)
+		ksft_exit_fail_msg("Bad address %lx\n", addr);
 }
 
 static int validate_lower_address_hint(void)
@@ -107,23 +100,29 @@ int main(int argc, char *argv[])
 	char *hint;
 	unsigned long i, lchunks, hchunks;
 
+	ksft_print_header();
+	ksft_set_plan(1);
+
 	for (i = 0; i < NR_CHUNKS_LOW; i++) {
 		ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
 					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 		if (ptr[i] == MAP_FAILED) {
-			if (validate_lower_address_hint())
-				return 1;
+			if (validate_lower_address_hint()) {
+				ksft_test_result_skip("Memory constraint not fulfilled\n");
+				ksft_finished();
+			}
 			break;
 		}
 
-		if (validate_addr(ptr[i], 0))
-			return 1;
+		validate_addr(ptr[i], 0);
 	}
 	lchunks = i;
 	hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *));
-	if (hptr == NULL)
-		return 1;
+	if (hptr == NULL) {
+		ksft_test_result_skip("Memory constraint not fulfilled\n");
+		ksft_finished();
+	}
 
 	for (i = 0; i < NR_CHUNKS_HIGH; i++) {
 		hint = hind_addr();
@@ -133,8 +132,7 @@ int main(int argc, char *argv[])
 		if (hptr[i] == MAP_FAILED)
 			break;
 
-		if (validate_addr(hptr[i], 1))
-			return 1;
+		validate_addr(hptr[i], 1);
 	}
 	hchunks = i;
 
@@ -145,5 +143,7 @@ int main(int argc, char *argv[])
 		munmap(hptr[i], MAP_CHUNK_SIZE);
 
 	free(hptr);
-	return 0;
+
+	ksft_test_result_pass("Test\n");
+	ksft_finished();
 }
-- 
2.42.0
Re: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output
Posted by Dev Jain 1 year, 9 months ago
On 2/2/24 17:01, Muhammad Usama Anjum wrote:
> Conform the layout, informational and status messages to TAP. No
> functional change is intended other than the layout of output messages.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   .../selftests/mm/virtual_address_range.c      | 44 +++++++++----------
>   1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
> index bae0ceaf95b13..7bcf8d48256a6 100644
> --- a/tools/testing/selftests/mm/virtual_address_range.c
> +++ b/tools/testing/selftests/mm/virtual_address_range.c
> @@ -12,6 +12,7 @@
>   #include <errno.h>
>   #include <sys/mman.h>
>   #include <sys/time.h>
> +#include "../kselftest.h"
>   
>   /*
>    * Maximum address range mapped with a single mmap()
> @@ -68,23 +69,15 @@ static char *hind_addr(void)
>   	return (char *) (1UL << bits);
>   }
>   
> -static int validate_addr(char *ptr, int high_addr)
> +static void validate_addr(char *ptr, int high_addr)
>   {
>   	unsigned long addr = (unsigned long) ptr;
>   
> -	if (high_addr) {
> -		if (addr < HIGH_ADDR_MARK) {
> -			printf("Bad address %lx\n", addr);
> -			return 1;
> -		}
> -		return 0;
> -	}
> +	if (high_addr && addr < HIGH_ADDR_MARK)
> +		ksft_exit_fail_msg("Bad address %lx\n", addr);
>   
> -	if (addr > HIGH_ADDR_MARK) {
> -		printf("Bad address %lx\n", addr);
> -		return 1;
> -	}
> -	return 0;
> +	if (addr > HIGH_ADDR_MARK)
> +		ksft_exit_fail_msg("Bad address %lx\n", addr);
>   }
>   
>   static int validate_lower_address_hint(void)
> @@ -107,23 +100,29 @@ int main(int argc, char *argv[])
>   	char *hint;
>   	unsigned long i, lchunks, hchunks;
>   
> +	ksft_print_header();
> +	ksft_set_plan(1);
> +
>   	for (i = 0; i < NR_CHUNKS_LOW; i++) {
>   		ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
>   					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>   
>   		if (ptr[i] == MAP_FAILED) {
> -			if (validate_lower_address_hint())
> -				return 1;
> +			if (validate_lower_address_hint()) {
> +				ksft_test_result_skip("Memory constraint not fulfilled\n");
> +				ksft_finished();
> +			}

Hi,

When validate_lower_address_hint() returns 1, it implies that despite 
filling the lower

range, mmap succeeded. IMHO, ksft_exit_fail_msg() should be used 
instead, with a

more descriptive message indicating that the memory was unexpectedly 
allocated.


Regards

Dev

>   			break;
>   		}
>   
> -		if (validate_addr(ptr[i], 0))
> -			return 1;
> +		validate_addr(ptr[i], 0);
>   	}
>   	lchunks = i;
>   	hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *));
> -	if (hptr == NULL)
> -		return 1;
> +	if (hptr == NULL) {
> +		ksft_test_result_skip("Memory constraint not fulfilled\n");
> +		ksft_finished();
> +	}
>   
>   	for (i = 0; i < NR_CHUNKS_HIGH; i++) {
>   		hint = hind_addr();
> @@ -133,8 +132,7 @@ int main(int argc, char *argv[])
>   		if (hptr[i] == MAP_FAILED)
>   			break;
>   
> -		if (validate_addr(hptr[i], 1))
> -			return 1;
> +		validate_addr(hptr[i], 1);
>   	}
>   	hchunks = i;
>   
> @@ -145,5 +143,7 @@ int main(int argc, char *argv[])
>   		munmap(hptr[i], MAP_CHUNK_SIZE);
>   
>   	free(hptr);
> -	return 0;
> +
> +	ksft_test_result_pass("Test\n");
> +	ksft_finished();
>   }
Re: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to TAP format output
Posted by Muhammad Usama Anjum 1 year, 9 months ago
On 3/14/24 10:00 AM, Dev Jain wrote:
> 
> On 2/2/24 17:01, Muhammad Usama Anjum wrote:
>> Conform the layout, informational and status messages to TAP. No
>> functional change is intended other than the layout of output messages.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   .../selftests/mm/virtual_address_range.c      | 44 +++++++++----------
>>   1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/virtual_address_range.c
>> b/tools/testing/selftests/mm/virtual_address_range.c
>> index bae0ceaf95b13..7bcf8d48256a6 100644
>> --- a/tools/testing/selftests/mm/virtual_address_range.c
>> +++ b/tools/testing/selftests/mm/virtual_address_range.c
>> @@ -12,6 +12,7 @@
>>   #include <errno.h>
>>   #include <sys/mman.h>
>>   #include <sys/time.h>
>> +#include "../kselftest.h"
>>     /*
>>    * Maximum address range mapped with a single mmap()
>> @@ -68,23 +69,15 @@ static char *hind_addr(void)
>>       return (char *) (1UL << bits);
>>   }
>>   -static int validate_addr(char *ptr, int high_addr)
>> +static void validate_addr(char *ptr, int high_addr)
>>   {
>>       unsigned long addr = (unsigned long) ptr;
>>   -    if (high_addr) {
>> -        if (addr < HIGH_ADDR_MARK) {
>> -            printf("Bad address %lx\n", addr);
>> -            return 1;
>> -        }
>> -        return 0;
>> -    }
>> +    if (high_addr && addr < HIGH_ADDR_MARK)
>> +        ksft_exit_fail_msg("Bad address %lx\n", addr);
>>   -    if (addr > HIGH_ADDR_MARK) {
>> -        printf("Bad address %lx\n", addr);
>> -        return 1;
>> -    }
>> -    return 0;
>> +    if (addr > HIGH_ADDR_MARK)
>> +        ksft_exit_fail_msg("Bad address %lx\n", addr);
>>   }
>>     static int validate_lower_address_hint(void)
>> @@ -107,23 +100,29 @@ int main(int argc, char *argv[])
>>       char *hint;
>>       unsigned long i, lchunks, hchunks;
>>   +    ksft_print_header();
>> +    ksft_set_plan(1);
>> +
>>       for (i = 0; i < NR_CHUNKS_LOW; i++) {
>>           ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
>>                       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>             if (ptr[i] == MAP_FAILED) {
>> -            if (validate_lower_address_hint())
>> -                return 1;
>> +            if (validate_lower_address_hint()) {
>> +                ksft_test_result_skip("Memory constraint not fulfilled\n");
>> +                ksft_finished();
>> +            }
> 
> Hi,
> 
> When validate_lower_address_hint() returns 1, it implies that despite
> filling the lower
> 
> range, mmap succeeded. IMHO, ksft_exit_fail_msg() should be used instead,
> with a
> 
> more descriptive message indicating that the memory was unexpectedly
> allocated.
Can you fire up a patch for this? Otherwise I'll get back to it next week.

> 
> 
> Regards
> 
> Dev
> 
>>               break;
>>           }
>>   -        if (validate_addr(ptr[i], 0))
>> -            return 1;
>> +        validate_addr(ptr[i], 0);
>>       }
>>       lchunks = i;
>>       hptr = (char **) calloc(NR_CHUNKS_HIGH, sizeof(char *));
>> -    if (hptr == NULL)
>> -        return 1;
>> +    if (hptr == NULL) {
>> +        ksft_test_result_skip("Memory constraint not fulfilled\n");
>> +        ksft_finished();
>> +    }
>>         for (i = 0; i < NR_CHUNKS_HIGH; i++) {
>>           hint = hind_addr();
>> @@ -133,8 +132,7 @@ int main(int argc, char *argv[])
>>           if (hptr[i] == MAP_FAILED)
>>               break;
>>   -        if (validate_addr(hptr[i], 1))
>> -            return 1;
>> +        validate_addr(hptr[i], 1);
>>       }
>>       hchunks = i;
>>   @@ -145,5 +143,7 @@ int main(int argc, char *argv[])
>>           munmap(hptr[i], MAP_CHUNK_SIZE);
>>         free(hptr);
>> -    return 0;
>> +
>> +    ksft_test_result_pass("Test\n");
>> +    ksft_finished();
>>   }

-- 
BR,
Muhammad Usama Anjum
[PATCH] selftests/mm: virtual_address_range: Switch to ksft_exit_fail_msg
Posted by Dev Jain 1 year, 9 months ago
mmap() must not succeed in validate_lower_address_hint(), for if it does, it
is a bug in mmap() itself. Reflect this behaviour with ksft_exit_fail_msg().
While at it, do some formatting changes.

Signed-off-by: Dev Jain <dev.jain@arm.com>
---
 tools/testing/selftests/mm/virtual_address_range.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
index 7bcf8d48256a..426ddfc345fb 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -85,7 +85,7 @@ static int validate_lower_address_hint(void)
 	char *ptr;
 
 	ptr = mmap((void *) (1UL << 45), MAP_CHUNK_SIZE, PROT_READ |
-			PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+		   PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 	if (ptr == MAP_FAILED)
 		return 0;
@@ -105,13 +105,11 @@ int main(int argc, char *argv[])
 
 	for (i = 0; i < NR_CHUNKS_LOW; i++) {
 		ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
-					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 		if (ptr[i] == MAP_FAILED) {
-			if (validate_lower_address_hint()) {
-				ksft_test_result_skip("Memory constraint not fulfilled\n");
-				ksft_finished();
-			}
+			if (validate_lower_address_hint())
+				ksft_exit_fail_msg("mmap unexpectedly succeeded with hint\n");
 			break;
 		}
 
@@ -127,7 +125,7 @@ int main(int argc, char *argv[])
 	for (i = 0; i < NR_CHUNKS_HIGH; i++) {
 		hint = hind_addr();
 		hptr[i] = mmap(hint, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
-					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+			       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
 		if (hptr[i] == MAP_FAILED)
 			break;
-- 
2.34.1
Re: [PATCH] selftests/mm: virtual_address_range: Switch to ksft_exit_fail_msg
Posted by Muhammad Usama Anjum 1 year, 9 months ago
Thanks for the patch. The patch should have been sent to a separate new thread.

On 3/14/24 5:22 PM, Dev Jain wrote:
> mmap() must not succeed in validate_lower_address_hint(), for if it does, it
> is a bug in mmap() itself. Reflect this behaviour with ksft_exit_fail_msg().
> While at it, do some formatting changes.
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  tools/testing/selftests/mm/virtual_address_range.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
> index 7bcf8d48256a..426ddfc345fb 100644
> --- a/tools/testing/selftests/mm/virtual_address_range.c
> +++ b/tools/testing/selftests/mm/virtual_address_range.c
> @@ -85,7 +85,7 @@ static int validate_lower_address_hint(void)
>  	char *ptr;
>  
>  	ptr = mmap((void *) (1UL << 45), MAP_CHUNK_SIZE, PROT_READ |
> -			PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +		   PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  
>  	if (ptr == MAP_FAILED)
>  		return 0;
> @@ -105,13 +105,11 @@ int main(int argc, char *argv[])
>  
>  	for (i = 0; i < NR_CHUNKS_LOW; i++) {
>  		ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> -					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  
>  		if (ptr[i] == MAP_FAILED) {
> -			if (validate_lower_address_hint()) {
> -				ksft_test_result_skip("Memory constraint not fulfilled\n");
> -				ksft_finished();
> -			}
> +			if (validate_lower_address_hint())
> +				ksft_exit_fail_msg("mmap unexpectedly succeeded with hint\n");
>  			break;
>  		}
>  
> @@ -127,7 +125,7 @@ int main(int argc, char *argv[])
>  	for (i = 0; i < NR_CHUNKS_HIGH; i++) {
>  		hint = hind_addr();
>  		hptr[i] = mmap(hint, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> -					MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +			       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  
>  		if (hptr[i] == MAP_FAILED)
>  			break;

-- 
BR,
Muhammad Usama Anjum