linux-kernel - Re: [PATCH 1/2] kunit: Warn if tests are slow

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: <87leddf2fs.fsf@intel.com>
Date:   Mon, 11 Sep 2023 13:07:35 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Maxime Ripard <mripard@...nel.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        David Gow <davidgow@...gle.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>
Cc:     linux-kernel@...r.kernel.org, Maxime Ripard <mripard@...nel.org>,
        Maíra Canal <mairacanal@...eup.net>,
        dri-devel@...ts.freedesktop.org, linux-kselftest@...r.kernel.org,
        kunit-dev@...glegroups.com
Subject: Re: [PATCH 1/2] kunit: Warn if tests are slow

On Mon, 11 Sep 2023, Maxime Ripard <mripard@...nel.org> wrote:
> Kunit recently gained support to setup attributes, the first one being
> the speed of a given test, then allowing to filter out slow tests.
>
> A slow test is defined in the documentation as taking more than one
> second. There's an another speed attribute called "super slow" but whose
> definition is less clear.
>
> Add support to the test runner to check the test execution time, and
> report tests that should be marked as slow but aren't.
>
> Signed-off-by: Maxime Ripard <mripard@...nel.org>
> ---
>  lib/kunit/test.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 49698a168437..a3b924501f3d 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -379,6 +379,9 @@ static void kunit_run_case_internal(struct kunit *test,
>  				    struct kunit_suite *suite,
>  				    struct kunit_case *test_case)
>  {
> +	struct timespec64 start, end;
> +	struct timespec64 duration;
> +
>  	if (suite->init) {
>  		int ret;
>  
> @@ -390,7 +393,20 @@ static void kunit_run_case_internal(struct kunit *test,
>  		}
>  	}
>  
> +	ktime_get_ts64(&start);
> +
>  	test_case->run_case(test);
> +
> +	ktime_get_ts64(&end);
> +
> +	duration = timespec64_sub(end, start);
> +
> +	if (duration.tv_sec >= 1 &&
> +	    (test_case->attr.speed == KUNIT_SPEED_UNSET ||
> +	     test_case->attr.speed >= KUNIT_SPEED_NORMAL))
> +		kunit_warn(test,
> +			   "Test should be marked slow (runtime: %lld.%09lds)",
> +			   duration.tv_sec, duration.tv_nsec);

Two thoughts:

Should there be some tolerance here? Otherwise we're flagging this on
the slowest machines, and we'll be defining tests slow based on
that. Like, warn if it takes more than 2 seconds.

What if someone makes a test faster, but forgets to update the
attribute? Should we also flag slow tests that are in fact fast?


BR,
Jani.


>  }
>  
>  static void kunit_case_internal_cleanup(struct kunit *test)

-- 
Jani Nikula, Intel Open Source Graphics Center