linux-kernel - Re: [PATCH v5 9/9] drm: selftest: convert drm

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: <j4twjg4rd74qq6wjr7nrtrtkh6cdxehuw5lmeavu7z5q5lhtih@nofkcrdnwty7>
Date:   Tue, 25 Jul 2023 10:38:07 +0200
From:   Maxime Ripard <mripard@...nel.org>
To:     Maíra Canal <maira.canal@....br>,
        David Gow <davidgow@...gle.com>,
        Daniel Latypov <dlatypov@...gle.com>, brendanhiggins@...gle.com
Cc:     Isabella Basso <isabbasso@...eup.net>, magalilemes00@...il.com,
        tales.aparecida@...il.com, mwen@...lia.com, andrealmeid@...eup.net,
        siqueirajordao@...eup.net, Trevor Woerner <twoerner@...il.com>,
        leandro.ribeiro@...labora.com, n@...aprado.net,
        Daniel Vetter <daniel@...ll.ch>,
        Shuah Khan <skhan@...uxfoundation.org>,
        David Airlie <airlied@...ux.ie>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        michal.winiarski@...el.com,
        Javier Martinez Canillas <javierm@...hat.com>,
        José Expósito <jose.exposito89@...il.com>,
        kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Arthur Grillo <arthur.grillo@....br>
Subject: Re: [PATCH v5 9/9] drm: selftest: convert drm_mm selftest to KUnit

Hi,

On Thu, Apr 27, 2023 at 03:14:39PM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Jul 08, 2022 at 05:30:52PM -0300, Maíra Canal wrote:
> > From: Arthur Grillo <arthur.grillo@....br>
> > 
> > Considering the current adoption of the KUnit framework, convert the
> > DRM mm selftest to the KUnit API.
> > 
> > Signed-off-by: Arthur Grillo <arthur.grillo@....br>
> > Tested-by: David Gow <davidgow@...gle.com>
> > Acked-by: Daniel Latypov <dlatypov@...gle.com>
> > Reviewed-by: Javier Martinez Canillas <javierm@...hat.com>
> > Signed-off-by: Maíra Canal <maira.canal@....br>
> 
> I'm very late to the party, but I'd like to discuss that patch some more.
> 
> Two tests (drm_test_mm_reserve, drm_test_mm_insert) in it take a super
> long time to run (about 30s each on my machine).
> 
> If we run all the DRM tests and VC4 tests, each of those two are longer
> to run than all the ~300 tests combined. About 100 times longer.
> 
> I don't think that running for so long is reasonable, and for multiple
> reasons:
> 
>   - While I don't know drm_mm well, it doesn't look like any of those
>     tests do something that really should take this long. I'm especially
>     skeptical about the fact that we test each operation 8192 times by
>     default.
> 
>   - It makes using kunit more tedious than it should be. Like I said, on
>     a very capable machine, running the all the DRM and VC4 tests takes
>     about 50s with those two tests, ~0.4s without.
> 
>   - The corollary is that it will get in the way of people that really
>     want to use kunit will just remove those tests before doing so,
>     defeating the original intent.
> 
> 
> I understand that it came from selftests initially, but I think we
> should rewrite the tests entirely to have smaller, faster tests. It's
> not clear to me why those tests are as complicated as they are though.
> 
> Also, going forward we should probably put disencourage tests running
> that long. Could Kunit timeout/warn after a while if a test is taking
> more than X seconds to run?

I'd still like to address this. We spend ~90% of the DRM kunit tests
execution time executing those two tests, which doesn't seem like a
reasonable thing to do.

I'm fine with doing that work, but I'd still need to figure out what
those tests are doing exactly. Can someone help?

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)