[PATCH v2 1/3] selftests/mm: compaction_test: Fix bogus test success on Aarch64

[PATCH v2 1/3] selftests/mm: compaction_test: Fix bogus test success on Aarch64

[Dev Jain]

Currently, if at runtime we are not able to allocate a huge page, the
test will trivially pass on Aarch64 due to no exception being raised on
division by zero while computing compaction_index. Fix that by checking
for nr_hugepages == 0. Anyways, in general, avoid a division by zero by
exiting the program beforehand. While at it, fix a typo.

Changes in v2:
- Combine with this series, handle unsigned long number of hugepages

Fixes: bd67d5c15cc1 ("Test compaction of mlocked memory")
Cc: stable@vger.kernel.org
Signed-off-by: Dev Jain <dev.jain@arm.com>
---
 tools/testing/selftests/mm/compaction_test.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 4f42eb7d7636..0b249a06a60b 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -82,12 +82,13 @@ int prereq(void)
 	return -1;
 }
 
-int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
+int check_compaction(unsigned long mem_free, unsigned long hugepage_size)
 {
+	unsigned long nr_hugepages_ul;
 	int fd, ret = -1;
 	int compaction_index = 0;
-	char initial_nr_hugepages[10] = {0};
-	char nr_hugepages[10] = {0};
+	char initial_nr_hugepages[20] = {0};
+	char nr_hugepages[20] = {0};
 
 	/* We want to test with 80% of available memory. Else, OOM killer comes
 	   in to play */
@@ -134,7 +135,12 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
 
 	/* We should have been able to request at least 1/3 rd of the memory in
 	   huge pages */
-	compaction_index = mem_free/(atoi(nr_hugepages) * hugepage_size);
+	nr_hugepages_ul = strtoul(nr_hugepages, NULL, 10);
+	if (!nr_hugepages_ul) {
+		ksft_print_msg("ERROR: No memory is available as huge pages\n");
+		goto close_fd;
+	}
+	compaction_index = mem_free/(nr_hugepages_ul * hugepage_size);
 
 	lseek(fd, 0, SEEK_SET);
 
@@ -145,11 +151,11 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
 		goto close_fd;
 	}
 
-	ksft_print_msg("Number of huge pages allocated = %d\n",
-		       atoi(nr_hugepages));
+	ksft_print_msg("Number of huge pages allocated = %lu\n",
+		       nr_hugepages_ul);
 
 	if (compaction_index > 3) {
-		ksft_print_msg("ERROR: Less that 1/%d of memory is available\n"
+		ksft_print_msg("ERROR: Less than 1/%d of memory is available\n"
 			       "as huge pages\n", compaction_index);
 		goto close_fd;
 	}
-- 
2.34.1

Re: [PATCH v2 1/3] selftests/mm: compaction_test: Fix bogus test success on Aarch64

[Dev Jain]

Sorry, I was just sending to myself for testing, git send-email picked

up stable@vger.kernel.org too. Please ignore this.

On 5/21/24 12:07, Dev Jain wrote:
> Currently, if at runtime we are not able to allocate a huge page, the
> test will trivially pass on Aarch64 due to no exception being raised on
> division by zero while computing compaction_index. Fix that by checking
> for nr_hugepages == 0. Anyways, in general, avoid a division by zero by
> exiting the program beforehand. While at it, fix a typo.
>
> Changes in v2:
> - Combine with this series, handle unsigned long number of hugepages
>
> Fixes: bd67d5c15cc1 ("Test compaction of mlocked memory")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>   tools/testing/selftests/mm/compaction_test.c | 20 +++++++++++++-------
>   1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
> index 4f42eb7d7636..0b249a06a60b 100644
> --- a/tools/testing/selftests/mm/compaction_test.c
> +++ b/tools/testing/selftests/mm/compaction_test.c
> @@ -82,12 +82,13 @@ int prereq(void)
>   	return -1;
>   }
>   
> -int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
> +int check_compaction(unsigned long mem_free, unsigned long hugepage_size)
>   {
> +	unsigned long nr_hugepages_ul;
>   	int fd, ret = -1;
>   	int compaction_index = 0;
> -	char initial_nr_hugepages[10] = {0};
> -	char nr_hugepages[10] = {0};
> +	char initial_nr_hugepages[20] = {0};
> +	char nr_hugepages[20] = {0};
>   
>   	/* We want to test with 80% of available memory. Else, OOM killer comes
>   	   in to play */
> @@ -134,7 +135,12 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
>   
>   	/* We should have been able to request at least 1/3 rd of the memory in
>   	   huge pages */
> -	compaction_index = mem_free/(atoi(nr_hugepages) * hugepage_size);
> +	nr_hugepages_ul = strtoul(nr_hugepages, NULL, 10);
> +	if (!nr_hugepages_ul) {
> +		ksft_print_msg("ERROR: No memory is available as huge pages\n");
> +		goto close_fd;
> +	}
> +	compaction_index = mem_free/(nr_hugepages_ul * hugepage_size);
>   
>   	lseek(fd, 0, SEEK_SET);
>   
> @@ -145,11 +151,11 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
>   		goto close_fd;
>   	}
>   
> -	ksft_print_msg("Number of huge pages allocated = %d\n",
> -		       atoi(nr_hugepages));
> +	ksft_print_msg("Number of huge pages allocated = %lu\n",
> +		       nr_hugepages_ul);
>   
>   	if (compaction_index > 3) {
> -		ksft_print_msg("ERROR: Less that 1/%d of memory is available\n"
> +		ksft_print_msg("ERROR: Less than 1/%d of memory is available\n"
>   			       "as huge pages\n", compaction_index);
>   		goto close_fd;
>   	}

[PATCH v2 0/3] Fixes for compaction_test

[Dev Jain]

The compaction_test memory selftest introduces fragmentation in memory
and then tries to allocate as many hugepages as possible. This series
addresses some problems.

On Aarch64, if nr_hugepages == 0, then the test trivially succeeds since
compaction_index becomes 0, which is less than 3, due to no division by
zero exception being raised. We fix that by checking for division by
zero.

Secondly, correctly set the number of hugepages to zero before trying
to set a large number of them.

Now, consider a situation in which, at the start of the test, a non-zero
number of hugepages have been already set (while running the entire
selftests/mm suite, or manually by the admin). The test operates on 80%
of memory to avoid OOM-killer invocation, and because some memory is
already blocked by hugepages, it would increase the chance of OOM-killing.
Also, since mem_free used in check_compaction() is the value before we
set nr_hugepages to zero, the chance that the compaction_index will
be small is very high if the preset nr_hugepages was high, leading to a
bogus test success.

This series applies on top of the stable 6.9 kernel.

Changes in v2:
- Handle an unsigned long number of hugepages
- Combine the first patch (previously standalone) with this series

Link to v1:
https://lore.kernel.org/all/20240513082842.4117782-1-dev.jain@arm.com/
https://lore.kernel.org/all/20240515093633.54814-1-dev.jain@arm.com/

Dev Jain (3):
  selftests/mm: compaction_test: Fix bogus test success on Aarch64
  selftests/mm: compaction_test: Fix incorrect write of zero to
    nr_hugepages
  selftests/mm: compaction_test: Fix bogus test success and reduce
    probability of OOM-killer invocation

 tools/testing/selftests/mm/compaction_test.c | 85 ++++++++++++++------
 1 file changed, 60 insertions(+), 25 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] selftests/mm: compaction_test: Fix bogus test success on Aarch64

[Dev Jain]

Currently, if at runtime we are not able to allocate a huge page, the
test will trivially pass on Aarch64 due to no exception being raised on
division by zero while computing compaction_index. Fix that by checking
for nr_hugepages == 0. Anyways, in general, avoid a division by zero by
exiting the program beforehand. While at it, fix a typo, and handle the
case where the number of hugepages may overflow an integer.

Changes in v2:
- Combine with this series, handle unsigned long number of hugepages

v1:
- https://lore.kernel.org/all/20240513082842.4117782-1-dev.jain@arm.com/

Fixes: bd67d5c15cc1 ("Test compaction of mlocked memory")
Cc: stable@vger.kernel.org
Signed-off-by: Dev Jain <dev.jain@arm.com>
---
 tools/testing/selftests/mm/compaction_test.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 4f42eb7d7636..0b249a06a60b 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -82,12 +82,13 @@ int prereq(void)
 	return -1;
 }
 
-int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
+int check_compaction(unsigned long mem_free, unsigned long hugepage_size)
 {
+	unsigned long nr_hugepages_ul;
 	int fd, ret = -1;
 	int compaction_index = 0;
-	char initial_nr_hugepages[10] = {0};
-	char nr_hugepages[10] = {0};
+	char initial_nr_hugepages[20] = {0};
+	char nr_hugepages[20] = {0};
 
 	/* We want to test with 80% of available memory. Else, OOM killer comes
 	   in to play */
@@ -134,7 +135,12 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
 
 	/* We should have been able to request at least 1/3 rd of the memory in
 	   huge pages */
-	compaction_index = mem_free/(atoi(nr_hugepages) * hugepage_size);
+	nr_hugepages_ul = strtoul(nr_hugepages, NULL, 10);
+	if (!nr_hugepages_ul) {
+		ksft_print_msg("ERROR: No memory is available as huge pages\n");
+		goto close_fd;
+	}
+	compaction_index = mem_free/(nr_hugepages_ul * hugepage_size);
 
 	lseek(fd, 0, SEEK_SET);
 
@@ -145,11 +151,11 @@ int check_compaction(unsigned long mem_free, unsigned int hugepage_size)
 		goto close_fd;
 	}
 
-	ksft_print_msg("Number of huge pages allocated = %d\n",
-		       atoi(nr_hugepages));
+	ksft_print_msg("Number of huge pages allocated = %lu\n",
+		       nr_hugepages_ul);
 
 	if (compaction_index > 3) {
-		ksft_print_msg("ERROR: Less that 1/%d of memory is available\n"
+		ksft_print_msg("ERROR: Less than 1/%d of memory is available\n"
 			       "as huge pages\n", compaction_index);
 		goto close_fd;
 	}
-- 
2.34.1