Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <99dead0d-08e2-ab1c-46dc-d3f4f97a71e8@redhat.com>
Date:   Mon, 17 Jul 2023 19:20:30 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH v2 2/8] selftests/mm: Skip soft-dirty tests on arm64
Content-Language: en-US
To:     Ryan Roberts <ryan.roberts@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Shuah Khan <shuah@kernel.org>,
        =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= <jglisse@redhat.com>,
        Mark Brown <broonie@kernel.org>,
        John Hubbard <jhubbard@nvidia.com>,
        Florent Revest <revest@chromium.org>,
        Peter Xu <peterx@redhat.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-kselftest@vger.kernel.org
References: <20230717103152.202078-1-ryan.roberts@arm.com>
 <20230717103152.202078-3-ryan.roberts@arm.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
In-Reply-To: <20230717103152.202078-3-ryan.roberts@arm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 17.07.23 12:31, Ryan Roberts wrote:
> arm64 does not support the soft-dirty PTE bit. However, the `soft-dirty`
> test suite is currently run unconditionally and therefore generates
> spurious test failures on arm64. There are also some tests in
> `madv_populate` which assume it is supported.
> 
> For `soft-dirty` lets disable the whole suite for arm64; it is no longer
> built and run_vmtests.sh will skip it if its not present.
> 
> For `madv_populate`, we need a runtime mechanism so that the remaining
> tests continue to be run. Unfortunately, the only way to determine if
> the soft-dirty dirty bit is supported is to write to a page, then see if
> the bit is set in /proc/self/pagemap. But the tests that we want to
> conditionally execute are testing precicesly this. So if we introduced
> this feature check, we could accedentally turn a real failure (on a
> system that claims to support soft-dirty) into a skip. So instead, do
> the check based on architecture; for arm64, we report that soft-dirty is
> not supported.
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>   tools/testing/selftests/mm/Makefile        |  5 ++++-
>   tools/testing/selftests/mm/madv_populate.c | 26 ++++++++++++++++++++--
>   tools/testing/selftests/mm/run_vmtests.sh  |  5 ++++-
>   3 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 66d7c07dc177..3514697fc2db 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -63,12 +63,15 @@ TEST_GEN_PROGS += thuge-gen
>   TEST_GEN_PROGS += transhuge-stress
>   TEST_GEN_PROGS += uffd-stress
>   TEST_GEN_PROGS += uffd-unit-tests
> -TEST_GEN_PROGS += soft-dirty
>   TEST_GEN_PROGS += split_huge_page_test
>   TEST_GEN_PROGS += ksm_tests
>   TEST_GEN_PROGS += ksm_functional_tests
>   TEST_GEN_PROGS += mdwe_test
>   
> +ifneq ($(ARCH),arm64)
> +TEST_GEN_PROGS += soft-dirty
> +endif
> +
>   ifeq ($(ARCH),x86_64)
>   CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32)
>   CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c)
> diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/testing/selftests/mm/madv_populate.c
> index 60547245e479..17bcb07f19f3 100644
> --- a/tools/testing/selftests/mm/madv_populate.c
> +++ b/tools/testing/selftests/mm/madv_populate.c
> @@ -264,14 +264,35 @@ static void test_softdirty(void)
>   	munmap(addr, SIZE);
>   }
>   
> +static int system_has_softdirty(void)
> +{
> +	/*
> +	 * There is no way to check if the kernel supports soft-dirty, other
> +	 * than by writing to a page and seeing if the bit was set. But the
> +	 * tests are intended to check that the bit gets set when it should, so
> +	 * doing that check would turn a potentially legitimate fail into a
> +	 * skip. Fortunately, we know for sure that arm64 does not support
> +	 * soft-dirty. So for now, let's just use the arch as a corse guide.
> +	 */
> +#if defined(__aarch64__)
> +	return 0;
> +#else
> +	return 1;
> +#endif
> +}

I guess that will also make the compiler remove any traces of 
test_softdirty()( from the binary.

Acked-by: David Hildenbrand <david@redhat.com>


-- 
Cheers,

David / dhildenb