Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To:     akpm@linux-foundation.org,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] selftests/vm: Update max va test to check for high address return.
In-Reply-To: <20180228035830.10089-1-aneesh.kumar@linux.vnet.ibm.com>
References: <20180228035830.10089-1-aneesh.kumar@linux.vnet.ibm.com>
Date:   Wed, 28 Feb 2018 13:38:05 +0530
MIME-Version: 1.0
Content-Type: text/plain
Message-Id: <87vaeha656.fsf@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> mmap(-1,..) is expected to search from max supported VA top down. It should find
> an address above ADDR_SWITCH_HINT. Explicitly check for this.
>
> Also derefer the address even if we failed the addr check.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

One issue I noticed is how to make this conditional so that we can still
run the test on x86 with 4 level page table?

> ---
>  tools/testing/selftests/vm/va_128TBswitch.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/va_128TBswitch.c b/tools/testing/selftests/vm/va_128TBswitch.c
> index e7fe734c374f..f68fa4bd8179 100644
> --- a/tools/testing/selftests/vm/va_128TBswitch.c
> +++ b/tools/testing/selftests/vm/va_128TBswitch.c
> @@ -44,6 +44,7 @@ struct testcase {
>  	unsigned long flags;
>  	const char *msg;
>  	unsigned int low_addr_required:1;
> +	unsigned int high_addr_required:1;
>  	unsigned int keep_mapped:1;
>  };
>  
> @@ -108,6 +109,7 @@ static struct testcase testcases[] = {
>  		.flags = MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(HIGH_ADDR)",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = HIGH_ADDR,
> @@ -115,12 +117,14 @@ static struct testcase testcases[] = {
>  		.flags = MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(HIGH_ADDR) again",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = HIGH_ADDR,
>  		.size = 2 * PAGE_SIZE,
>  		.flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
>  		.msg = "mmap(HIGH_ADDR, MAP_FIXED)",
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = (void *) -1,
> @@ -128,12 +132,14 @@ static struct testcase testcases[] = {
>  		.flags = MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(-1)",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = (void *) -1,
>  		.size = 2 * PAGE_SIZE,
>  		.flags = MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(-1) again",
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
> @@ -193,6 +199,7 @@ static struct testcase hugetlb_testcases[] = {
>  		.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(HIGH_ADDR, MAP_HUGETLB)",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = HIGH_ADDR,
> @@ -200,12 +207,14 @@ static struct testcase hugetlb_testcases[] = {
>  		.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(HIGH_ADDR, MAP_HUGETLB) again",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = HIGH_ADDR,
>  		.size = HUGETLB_SIZE,
>  		.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
>  		.msg = "mmap(HIGH_ADDR, MAP_FIXED | MAP_HUGETLB)",
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = (void *) -1,
> @@ -213,12 +222,14 @@ static struct testcase hugetlb_testcases[] = {
>  		.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(-1, MAP_HUGETLB)",
>  		.keep_mapped = 1,
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = (void *) -1,
>  		.size = HUGETLB_SIZE,
>  		.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
>  		.msg = "mmap(-1, MAP_HUGETLB) again",
> +		.high_addr_required = 1,
>  	},
>  	{
>  		.addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE),
> @@ -257,14 +268,16 @@ static int run_test(struct testcase *test, int count)
>  		if (t->low_addr_required && p >= (void *)(ADDR_SWITCH_HINT)) {
>  			printf("FAILED\n");
>  			ret = 1;
> -		} else {
> -			/*
> -			 * Do a dereference of the address returned so that we catch
> -			 * bugs in page fault handling
> -			 */
> -			memset(p, 0, t->size);
> +		} else if (t->high_addr_required && p < (void *)(ADDR_SWITCH_HINT)) {
> +			printf("FAILED\n");
> +			ret = 1;
> +		} else
>  			printf("OK\n");
> -		}
> +		/*
> +		 * Do a dereference of the address returned so that we catch
> +		 * bugs in page fault handling
> +		 */
> +		memset(p, 0, t->size);
>  		if (!t->keep_mapped)
>  			munmap(p, t->size);
>  	}
> -- 
> 2.14.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>