dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Sean Paul <sean@poorly.run>,
	freedreno <freedreno@lists.freedesktop.org>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: Adding CI results to the kernel tree was Re: [RFC v2] drm/msm: Add initial ci/ subdirectory
Date: Wed, 11 May 2022 21:39:48 +0200	[thread overview]
Message-ID: <YnwRBFNyygB0Ub6S@phenom.ffwll.local> (raw)
In-Reply-To: <CAHk-=wg8YgH1h3wrm9CtXff7rSewa+NE0Z5upb1GOE8XiTL9HA@mail.gmail.com>

On Wed, May 11, 2022 at 10:33:06AM -0700, Linus Torvalds wrote:
> On Tue, May 10, 2022 at 10:07 PM Dave Airlie <airlied@gmail.com> wrote:
> >
> > > And use it to store expectations about what the drm/msm driver is
> > > supposed to pass in the IGT test suite.
> >
> > I wanted to loop in Linus/Greg to see if there are any issues raised
> > by adding CI results file to the tree in their minds, or if any other
> > subsystem has done this already, and it's all fine.
> >
> > I think this is a good thing after our Mesa experience, but Mesa has a
> > lot tighter integration here, so I want to get some more opinions
> > outside the group.
> 
> Honestly, my immediate reaction is that I think it might be ok, but
> 
>  (a) are these things going to absolutely balloon over time?
> 
>  (b) should these not be separated out?
> 
> Those two issues kind of interact.
> 
> If it's a small and targeted test-suite, by all means keep it in the
> kernel, but why not make it part of "tools/testing/selftests"
> 
> But if people expect this to balloon and we end up having megabytes of
> test output, then I really think it should be a separate git tree.
> 
> A diffstat like this:
> 
> >  7 files changed, 791 insertions(+)

Yeah I guess it's good to have some numbers for where this might go. Good
comparison is probably mesa3d, since it's the same-ish people doing the
same-ish ci on the same-ish infrastructure, just the userspace part of it.

mesa$ git ls-files | grep ci | xargs cat | wc -l
123077

mesa$ git ls-files | grep ci | wc -l
421

Compared to drivers/gpu it's really not much, and mesa is about the size
of drivers/gpu if you exclude the massive amount of register headers from
amd.

And I guess if we do stuff like result file compression like you mentioned
it should be quite a bit less even.

So yeah if this does take off it wil be substantially more, but I don't
think it'll ever get to a point where it'll swamp code changes. And if it
does that's kinda a solid indicator that something really wrong is going
on.

> is not a problem at all. But I get the feeling that this is just the
> tip of the iceberg, and people will want to not just have the result
> files, but start adding actual *input* files that may be largely
> automated stuff and may be tens of megabytes in size.
> 
> Because the result files on their own aren't really self-contained,
> and then people will want to keep them in sync with the test-files
> themselves, and start adding those, and now it *really* is likely very
> unwieldy.
> 
> Or if that doesn't happen, and the actual input test files stay in a
> separate CI repo, and then you end up having random coherency issues
> with that CI repo, and it all gets to be either horribly messy, or the
> result files in the kernel end up really stale.
> 
> So honestly, I personally don't see a good end result here.  This
> particular small patch? *This* one looks fine to me, except I really
> think tools/testing/selftests/gpu would be a much more logical place
> for it.
> 
> But I don't see a way forward that is sane.
> 
> Can somebody argue otherwise?

I do personally think we should add a bunch more things here, radically
putting everything into the drm-ci repo feels a bit much like appeasement
to get the foot in the door. Like some of the scripts are definitely
specific to the ci infra on freedesktop.org (or specific hw runners for
the drivers), and that makes sense to keep in that drm/fd ci repo. But
other scripts should probably migrate to scripts/ and at least start out
in a ci/ folder in the kernel.

igt itself might eventually move to tools/testing/selftests/gpu or
whatever, but that's kinda a huge discussion onto itself. And I haven't
seen a clear consensus yet among subsystem that these kind of tests (like
xfs-tests, and I think pretty much ever bigger subsystem that is old
enough to predate selftests has them somewhere) should all move into
tools/testing/selftest. Maybe they should, but feels like this is
orthogonal to ci integration.

Note that mesa3d has the exact same issue going that you're raising, and
some of those are unfixable because the opengl/vulkan conformance test
suites are maintained entirely externally by Khronos (and you have to use
those or you're not conformant to the spec, which renders the point of
having a shared spec a bit moot). It's messy but workable, and the CI you
get seems very much to be worth the price.

One idea I tossed out on irc is to move this all under drivers/gpu/ci.
There's driver specific stuff like the test result/fail lists, and maybe
those could eventually move out to drivers. But for starting out it might
be better to keep it all in one place so it's a bit better under control
and doesn't accidentally become a kranken of some kind. And then make sure
pieces move to scripts/ or tools/testing/ appropriately.

In general I think any mess this causes is a pretty good indicator that
something is amiss, like if this causes messy history due to tests
flipping too much and causing issues then that also indicates an issue
with the kernel or testcase quality itself. And it might be good to shine
more light on that stuff.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  parent reply	other threads:[~2022-05-11 19:39 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10  7:01 [RFC] drm/msm: Add initial ci/ subdirectory Tomeu Vizoso
2022-05-10 14:13 ` [RFC v2] " Tomeu Vizoso
2022-05-10 19:39   ` [Freedreno] " Jessica Zhang
2022-05-10 20:25     ` Rob Clark
2022-05-11 17:12       ` Daniel Vetter
2022-05-11 17:46         ` Rob Clark
2022-05-11 19:14           ` Daniel Vetter
2022-05-11 20:32             ` Rob Clark
2022-05-11 21:09               ` Daniel Vetter
2022-05-12 13:28           ` Tomeu Vizoso
2022-05-12 14:02             ` Daniel Vetter
2022-05-11  4:25     ` Tomeu Vizoso
2022-05-11  5:06   ` Adding CI results to the kernel tree was " Dave Airlie
2022-05-11  6:22     ` Greg Kroah-Hartman
2022-05-11 10:26       ` Michel Dänzer
2022-05-11 11:50         ` Greg Kroah-Hartman
2022-05-11 13:33           ` [Freedreno] " Rob Clark
2022-05-11 16:43             ` Daniel Vetter
2022-05-11 17:23               ` Rob Clark
2022-05-12  2:24             ` Theodore Ts'o
2022-05-11 17:33     ` Linus Torvalds
2022-05-11 18:39       ` Rob Clark
2022-05-11 19:08         ` Linus Torvalds
2022-05-11 19:12           ` Linus Torvalds
2022-05-11 20:14             ` [Freedreno] " Rob Clark
2022-05-11 20:06           ` Rob Clark
2022-05-11 19:39       ` Daniel Vetter [this message]
2022-05-11  6:15   ` [RFC v3] " Tomeu Vizoso
2022-05-11 13:20     ` Rob Clark
2022-05-11 14:03       ` Tomeu Vizoso
2022-05-17  8:16     ` [RFC v4] drm: " Tomeu Vizoso
2022-05-17  9:18       ` Neil Armstrong
2022-05-17  9:24         ` Tomeu Vizoso
2022-07-26 18:16       ` [PATCH v5] " Tomeu Vizoso
2022-07-29 15:43         ` Rob Clark
2022-08-10 18:24         ` Rodrigo Siqueira Jordao
2022-08-10 22:08           ` Rob Clark
2022-08-11 10:10           ` Tomeu Vizoso
2022-08-16 15:37         ` [PATCH v6] " Tomeu Vizoso
2022-08-29  9:31           ` [PATCH v7] " Tomeu Vizoso
2022-09-09 14:15             ` [PATCH v8] " Tomeu Vizoso
2022-09-09 17:16               ` Daniel Stone
2022-10-25  7:32                 ` Daniel Vetter
2022-10-25 15:06                   ` Daniel Stone
2022-09-12  7:29               ` [PATCH v9] " Tomeu Vizoso
2022-10-14 17:10                 ` Rob Clark
2022-10-24 22:06                 ` Jessica Zhang
2022-05-11 14:26 ` [RFC] drm/msm: " Jani Nikula
2022-05-11 14:41   ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YnwRBFNyygB0Ub6S@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=corbet@lwn.net \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_abhinavk@quicinc.com \
    --cc=sean@poorly.run \
    --cc=tomeu.vizoso@collabora.com \
    --cc=torvalds@linux-foundation.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).