lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <13a96176-1bfa-4567-8ce5-a2b75b110afc@linux.ibm.com>
Date: Fri, 8 Nov 2024 16:19:31 +0530
From: Donet Tom <donettom@...ux.ibm.com>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Shuah Khan <shuah@...nel.org>
Cc: kernel@...labora.com, linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selftests: hugetlb_dio: Check for initial conditions to
 skip in the start


On 11/8/24 16:05, Donet Tom wrote:
>
> On 11/1/24 19:45, Muhammad Usama Anjum wrote:
>> The test should be skipped if initial conditions aren't fulfilled in
>> the start instead of failing and outputting non-compliant TAP logs. This
>> kind of failure pollutes the results. The initial conditions are:
>> - The test should only execute if /tmp file can be allocated.
>> - The test should only execute if huge pages are free.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
>> ---
>> Before:
>> TAP version 13
>> 1..4
>> Bail out! Error opening file
>> : Read-only file system (30)
>>   # Planned tests != run tests (4 != 0)
>>   # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
>>
>> After:
>> TAP version 13
>> 1..0 # SKIP Unable to allocate file: Read-only file system
>> ---
>>   tools/testing/selftests/mm/hugetlb_dio.c | 19 ++++++++++++-------
>>   1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/hugetlb_dio.c 
>> b/tools/testing/selftests/mm/hugetlb_dio.c
>> index f9ac20c657ec6..60001c142ce99 100644
>> --- a/tools/testing/selftests/mm/hugetlb_dio.c
>> +++ b/tools/testing/selftests/mm/hugetlb_dio.c
>> @@ -44,13 +44,6 @@ void run_dio_using_hugetlb(unsigned int start_off, 
>> unsigned int end_off)
>>       if (fd < 0)
>>           ksft_exit_fail_perror("Error opening file\n");
>>   -    /* Get the free huge pages before allocation */
>> -    free_hpage_b = get_free_hugepages();
>
> Hi Muhammed Usman Anjum
>
> Reading the free pages is required before starting the test. This 
> value will be compared to the free pages after the test. If they are 
> not the same, the test will be considered a failure.
>
> Since reading the free pages before the test was 
> removed,|free_hpage_b|  is always 0, causing the test to fail.
>
> ./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No. 
> Free pages before allocation : 0 # No. Free pages after munmap : 100 
> not ok 1 : Huge pages not freed! # No. Free pages before allocation : 
> 0 # No. Free pages after munmap : 100 not ok 2 : Huge pages not freed! 
> # No. Free pages before allocation : 0 # No. Free pages after munmap : 
> 100 not ok 3 : Huge pages not freed! # No. Free pages before 
> allocation : 0 # No. Free pages after munmap : 100 not ok 4 : Huge 
> pages not freed! # Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 
> error:0 I think below changes are required. --- 
> a/tools/testing/selftests/mm/hugetlb_dio.c +++ 
> b/tools/testing/selftests/mm/hugetlb_dio.c @@ -44,6 +44,9 @@ void 
> run_dio_using_hugetlb(unsigned int start_off, unsigned int end_off) if 
> (fd < 0) ksft_exit_fail_perror("Error opening file\n"); + /* Get the 
> free huge pages before allocation */ + free_hpage_b = 
> get_free_hugepages(); + /* Allocate a hugetlb page */ orig_buffer = 
> mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0); if (orig_buffer 
> == MAP_FAILED) { With this change the tests are passing. 
> ./tools/testing/selftests/mm/hugetlb_dio TAP version 13 1..4 # No. 
> Free pages before allocation : 100 # No. Free pages after munmap : 100 
> ok 1 : Huge pages freed successfully ! # No. Free pages before 
> allocation : 100 # No. Free pages after munmap : 100 ok 2 : Huge pages 
> freed successfully ! # No. Free pages before allocation : 100 # No. 
> Free pages after munmap : 100 ok 3 : Huge pages freed successfully ! # 
> No. Free pages before allocation : 100 # No. Free pages after munmap : 
> 100 ok 4 : Huge pages freed successfully ! # Totals: pass:4 fail:0 
> xfail:0 xpass:0 skip:0 error:0 Thank
>
> Donet
>
>
Sorry. Please ignore above mail.

Reading the free pages is required before starting the test. This
value will be compared to the free pages after the test. If they are not the same, the test will be considered a failure.

Since reading the free pages before the test was removed,free_hpage_b is always 0, causing the test to fail.

./tools/testing/selftests/mm/hugetlb_dio
TAP version 13
1..4
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 1 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 2 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 3 : Huge pages not freed!
# No. Free pages before allocation : 0
# No. Free pages after munmap : 100
not ok 4 : Huge pages not freed!
# Totals: pass:0 fail:4 xfail:0 xpass:0 skip:0 error:0

I think below changes are required.

iff --git a/tools/testing/selftests/mm/hugetlb_dio.c b/tools/testing/selftests/mm/hugetlb_dio.c
index 60001c142ce9..4b52106b8124 100644
--- a/tools/testing/selftests/mm/hugetlb_dio.c
+++ b/tools/testing/selftests/mm/hugetlb_dio.c
@@ -44,6 +44,9 @@ void run_dio_using_hugetlb(unsigned int start_off, unsigned int end_off)
         if (fd < 0)
                 ksft_exit_fail_perror("Error opening file\n");
  
+       /* Get the free huge pages before allocation */
+       free_hpage_b = get_free_hugepages();
+
         /* Allocate a hugetlb page */

         orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 0);

         if (orig_buffer == MAP_FAILED) {

  With this change the tests are passing.

./tools/testing/selftests/mm/hugetlb_dio

TAP version 131..4
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 1 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 2 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 3 : Huge pages freed successfully !
# No. Free pages before allocation : 100
# No. Free pages after munmap : 100
ok 4 : Huge pages freed successfully !
# Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0

Thanks
Donet


>> -    if (free_hpage_b == 0) {
>> -        close(fd);
>> -        ksft_exit_skip("No free hugepage, exiting!\n");
>> -    }
>> -
>>       /* Allocate a hugetlb page */
>>       orig_buffer = mmap(NULL, h_pagesize, mmap_prot, mmap_flags, -1, 
>> 0);
>>       if (orig_buffer == MAP_FAILED) {
>> @@ -94,8 +87,20 @@ void run_dio_using_hugetlb(unsigned int start_off, 
>> unsigned int end_off)
>>   int main(void)
>>   {
>>       size_t pagesize = 0;
>> +    int fd;
>>         ksft_print_header();
>> +
>> +    /* Open the file to DIO */
>> +    fd = open("/tmp", O_TMPFILE | O_RDWR | O_DIRECT, 0664);
>> +    if (fd < 0)
>> +        ksft_exit_skip("Unable to allocate file: %s\n", 
>> strerror(errno));
>> +    close(fd);
>> +
>> +    /* Check if huge pages are free */
>> +    if (!get_free_hugepages())
>> +        ksft_exit_skip("No free hugepage, exiting\n");
>> +
>>       ksft_set_plan(4);
>>         /* Get base page size */

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ