Received-SPF: pass (google.com: domain of linux-kernel+bounces-102778-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <12b0aa90-3b4f-4fb0-b4df-c677f496cb09@arm.com>
Date: Thu, 14 Mar 2024 10:30:44 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 12/12] selftests/mm: virtual_address_range: conform to
 TAP format output
Content-Language: en-US
To: Muhammad Usama Anjum <usama.anjum@collabora.com>,
 Andrew Morton <akpm@linux-foundation.org>, Shuah Khan <shuah@kernel.org>
Cc: kernel@collabora.com, linux-mm@kvack.org,
 linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
 Anshuman.Khandual@arm.com
References: <20240202113119.2047740-1-usama.anjum@collabora.com>
 <20240202113119.2047740-13-usama.anjum@collabora.com>
From: Dev Jain <dev.jain@arm.com>
In-Reply-To: <20240202113119.2047740-13-usama.anjum@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


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();
>   }