From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8A684C001B0 for ; Thu, 13 Jul 2023 14:16:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 20897900025; Thu, 13 Jul 2023 10:16:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1B7E190001C; Thu, 13 Jul 2023 10:16:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 080FB900025; Thu, 13 Jul 2023 10:16:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id ED34490001C for ; Thu, 13 Jul 2023 10:16:16 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 5D960C02C5 for ; Thu, 13 Jul 2023 14:16:16 +0000 (UTC) X-FDA: 81006788352.07.6049657 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf01.hostedemail.com (Postfix) with ESMTP id B22F640039 for ; Thu, 13 Jul 2023 14:16:12 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689257773; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mQuYUmL8kloxiKjcr21MTE7zCQNGhGTrUyiqLqubxcM=; b=Zg7K+JxQGlIaKvuAhNUl2oS2CDT7GFGrd8ftCr1VeGr95J7fntsXQ+sZ2fUT3fwzjbE8g2 2I3pqdR96bqkaVymJjU1vcDCr2f2ITEawak10w/7UVU0TqL9CUhKodyCCKOp2EEUVpj5AV 9dAxUId050qYoXGmxkDVSE61YqPx8Dc= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf01.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689257773; a=rsa-sha256; cv=none; b=TI1KX9qTr6I1eMmphPcfdtHlTUDWEhHOZRJbaGS7vT97b+Fq6VBNTmTBszJM/dok7MgT0f 4WNEnOIJFCGmq8p9YAa/Mj5bQfiu2LcN6xnDwhbceMM8u0+OqQox4IjM0Zs/WYmQYEsHpC eNgObXkPP5Uyrz3/I4nL2rOi9J65dn8= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C83601570; Thu, 13 Jul 2023 07:16:54 -0700 (PDT) Received: from [10.1.30.48] (C02Z41KALVDN.cambridge.arm.com [10.1.30.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E9EAF3F73F; Thu, 13 Jul 2023 07:16:10 -0700 (PDT) Message-ID: <062255de-90c9-72a5-dc04-714e96837fd2@arm.com> Date: Thu, 13 Jul 2023 15:16:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v1 3/9] selftests/mm: Skip soft-dirty tests on arm64 To: David Hildenbrand , Andrew Morton , Shuah Khan , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Mark Brown , John Hubbard , Florent Revest , "Liam R. Howlett" Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org References: <20230713135440.3651409-1-ryan.roberts@arm.com> <20230713135440.3651409-4-ryan.roberts@arm.com> <773cc0a8-24b8-7fcb-2980-7676fc772014@arm.com> <3c566e28-c7ad-7ba8-4583-619266282387@redhat.com> <373e7e67-6ccc-5508-6937-6ea5a3eed5ea@redhat.com> From: Ryan Roberts In-Reply-To: <373e7e67-6ccc-5508-6937-6ea5a3eed5ea@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B22F640039 X-Stat-Signature: g8jt8nxejoimhcnfzjzx5ceu8gk9xb8c X-HE-Tag: 1689257772-519368 X-HE-Meta: U2FsdGVkX1+PoM7Q5ekTKjREPGUF/Lj3SlMZmXjTW+tNORb0ITegkNKdkpfZaAosIMKzo35hyMNy86W87iPathqqbTdxoegPA1rByIPcX9+CDHzhbs7c7PLoQ2wm51Qrt2aFMrKtVTagFeIXJy4E2C2ar/QEbjf90ECoOOQuXSlVidD43cwUXaT288JxobEVdQ5pEEgT+ZS0X5hq9NVlObcKhLzNFLrTRNKs/vGkiMAhZfO/HPK0RIxNR0D7k+RfmDNN7OGOnS1ftG0Vjdrd8cJLsulqI0CD0NLnwTqvK6xjxZhBvGco+GSG7IInS+s2d6mCAwsj+XSP8V7hqmdQsOXxVm6vax3mT2Z+C+Ss+hq5INrTPmukx1MecmfpxYQrbCM8609ucK6qfMkB83WNdEsWyWA7eveV6/3httf8psP5/98KtwBlHuu4KNFf9+IuYC0tW+e2Z1l1+GIPE1Leq2Y0S2U19ADGN2NJNmcZZZnDSsrlHEL6bnL/FpdpTm/N78uabH1em7HrrMndpVaH/E9DFoNhPXlv2H/t6pEQ6MacwrersQ/lO5tIPUE41dUTJMupTlPgmiA/2sLamb6Gl2IiDAT2STyeiEoCX9/y4kc7p0v6jn8RrIwxxpw3c72LKvYDAzNweGu8+l39NnmSqoFIJl81JtrcPq1j5oAPxNq0Loi7fQrxZ0Kw92wNCFeQr5ZsuWa2jpEzb9WXP3yWjsDF3e/3XcJjjOxJAUJ1wYUvnXnDn3PT3KSjZV2FQrxzLYNfxXUZd97VtR6q63D7VsStxmi2at+Ga2wO0tH9XkaEhlKOzfFW3nhCziKhUWh5MkMKeD3aWrjGMnwkZgBmrBApIQi1F2vFwzhjbJyHDN/0cv9Ob7o4/jbE/QfI5WApRFttwdyjyImUoArBVrn/MiNsFZuMfquJ6IJqJ3qdMpA8cj+Myw2c82FDV3wqqb8ldVaVMV9VAPF8qzbG1uN j3EbcavE 5tZ72cq5B/ZrVWixUMlrUxxTk/ZbGSyEJLTgfc/WQBiOuitkCiSmK52wDtasxshvdZWpC5548h1/texDPzDv/5PtsSmaeARV110jWulO4Ha54+8o5xQhGqWbLt8tctzfYh8cQ6eTPoGHHVGcpO7yZaUBNLv20NTTd9rSjINI+yhp+7X4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 13/07/2023 15:12, David Hildenbrand wrote: > On 13.07.23 16:09, David Hildenbrand wrote: >> On 13.07.23 16:03, Ryan Roberts wrote: >>> On 13/07/2023 14:56, David Hildenbrand wrote: >>>> On 13.07.23 15:54, Ryan Roberts wrote: >>>>> arm64 does not support the soft-dirty PTE bit. However there are tests >>>>> in `madv_populate` and `soft-dirty` which assume it is supported and >>>>> cause spurious failures to be reported when preferred behaviour would be >>>>> to mark the tests as skipped. >>>>> >>>>> 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. This is wrapped up into a utility >>>>> function `system_has_softdirty()`, which is used to skip the whole >>>>> `soft-dirty` suite, and mark the soft-dirty tests in the `madv_populate` >>>>> suite as skipped. >>>>> >>>>> Signed-off-by: Ryan Roberts >>>>> --- >>>>>     tools/testing/selftests/mm/madv_populate.c | 18 +++++++++++++----- >>>>>     tools/testing/selftests/mm/soft-dirty.c    |  3 +++ >>>>>     tools/testing/selftests/mm/vm_util.c       | 17 +++++++++++++++++ >>>>>     tools/testing/selftests/mm/vm_util.h       |  1 + >>>>>     4 files changed, 34 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/tools/testing/selftests/mm/madv_populate.c >>>>> b/tools/testing/selftests/mm/madv_populate.c >>>>> index 60547245e479..5a8c176d7fec 100644 >>>>> --- a/tools/testing/selftests/mm/madv_populate.c >>>>> +++ b/tools/testing/selftests/mm/madv_populate.c >>>>> @@ -232,6 +232,14 @@ static bool range_is_not_softdirty(char *start, ssize_t >>>>> size) >>>>>         return ret; >>>>>     } >>>>> >>>>> +#define ksft_test_result_if_softdirty(cond, ...)    \ >>>>> +do {                            \ >>>>> +    if (system_has_softdirty())            \ >>>>> +        ksft_test_result(cond, __VA_ARGS__);    \ >>>>> +    else                        \ >>>>> +        ksft_test_result_skip(__VA_ARGS__);    \ >>>>> +} while (0) >>>>> + >>>>>     static void test_softdirty(void) >>>>>     { >>>>>         char *addr; >>>>> @@ -246,19 +254,19 @@ static void test_softdirty(void) >>>>> >>>>>         /* Clear any softdirty bits. */ >>>>>         clear_softdirty(); >>>>> -    ksft_test_result(range_is_not_softdirty(addr, SIZE), >>>>> +    ksft_test_result_if_softdirty(range_is_not_softdirty(addr, SIZE), >>>>>                  "range is not softdirty\n"); >>>>> >>>>>         /* Populating READ should set softdirty. */ >>>>>         ret = madvise(addr, SIZE, MADV_POPULATE_READ); >>>>> -    ksft_test_result(!ret, "MADV_POPULATE_READ\n"); >>>>> -    ksft_test_result(range_is_not_softdirty(addr, SIZE), >>>>> +    ksft_test_result_if_softdirty(!ret, "MADV_POPULATE_READ\n"); >>>>> +    ksft_test_result_if_softdirty(range_is_not_softdirty(addr, SIZE), >>>>>                  "range is not softdirty\n"); >>>>> >>>>>         /* Populating WRITE should set softdirty. */ >>>>>         ret = madvise(addr, SIZE, MADV_POPULATE_WRITE); >>>>> -    ksft_test_result(!ret, "MADV_POPULATE_WRITE\n"); >>>>> -    ksft_test_result(range_is_softdirty(addr, SIZE), >>>>> +    ksft_test_result_if_softdirty(!ret, "MADV_POPULATE_WRITE\n"); >>>>> +    ksft_test_result_if_softdirty(range_is_softdirty(addr, SIZE), >>>>>                  "range is softdirty\n"); >>>> >>>> We probably want to skip the whole test_*softdirty* test instead of adding this >>>> (IMHO suboptimal) ksft_test_result_if_softdirty. >>> >>> Yeah I thought about doing it that way, but then the output just looks like >>> there were fewer tests and they all passed. But thinking about it now, I guess >>> the TAP header outputs the number of planned tests and the number of tests >>> executed are fewer, so a machine parser would still notice. I just don't like >>> that it outputs skipped:0. >>> >>> But it a lightly held view. Happy to just do: >>> >>>     if (system_has_softdirty()) >>>         test_softdirty() >>> >>> If you insist. ;-) >> >> diff --git a/tools/testing/selftests/mm/madv_populate.c >> b/tools/testing/selftests/mm/madv_populate.c >> index 60547245e479..33fda0337b32 100644 >> --- a/tools/testing/selftests/mm/madv_populate.c >> +++ b/tools/testing/selftests/mm/madv_populate.c >> @@ -266,12 +266,16 @@ static void test_softdirty(void) >>       int main(int argc, char **argv) >>    { >> +       int nr_tests = 16; >>           int err; >>              pagesize = getpagesize(); >>    +       if (system_has_softdirty()) >> +               nr_tests += 5; >> + >>           ksft_print_header(); >> -       ksft_set_plan(21); >> +       ksft_set_plan(nr_tests); >>              sense_support(); >>           test_prot_read(); >> @@ -279,7 +283,8 @@ int main(int argc, char **argv) >>           test_holes(); >>           test_populate_read(); >>           test_populate_write(); >> -       test_softdirty(); >> +       if (system_has_softdirty()) >> +               test_softdirty(); >>              err = ksft_get_fail_cnt(); >>           if (err) >> >> > > Oh, and if you want to have the skip, then you can think about converting > test_softdirty() to only perform a single ksft_test_result(), and have a single > skip on top. > > All cleaner IMHO than ksft_test_result_if_softdirty ;) I'll do it the way you recommend above. Like I said, its a lightly held opinion, and your way looks like less effort. ;-) >