Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 11 May 2022 21:39:48 +0200
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Dave Airlie <airlied@gmail.com>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Daniel Vetter <daniel@ffwll.ch>,
        Jonathan Corbet <corbet@lwn.net>,
        Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        freedreno <freedreno@lists.freedesktop.org>
Subject: Re: Adding CI results to the kernel tree was Re: [RFC v2] drm/msm:
 Add initial ci/ subdirectory
Message-ID: <YnwRBFNyygB0Ub6S@phenom.ffwll.local>
Mail-Followup-To: Linus Torvalds <torvalds@linux-foundation.org>,
        Dave Airlie <airlied@gmail.com>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Jonathan Corbet <corbet@lwn.net>, Rob Clark <robdclark@gmail.com>,
        Sean Paul <sean@poorly.run>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        freedreno <freedreno@lists.freedesktop.org>
References: <20220510070140.45407-1-tomeu.vizoso@collabora.com>
 <20220510141329.54414-1-tomeu.vizoso@collabora.com>
 <CAPM=9tzLR-wsLhg2ikGjoK06s-ju5XWa1rtPPiUpN=pwD1vgtA@mail.gmail.com>
 <CAHk-=wg8YgH1h3wrm9CtXff7rSewa+NE0Z5upb1GOE8XiTL9HA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHk-=wg8YgH1h3wrm9CtXff7rSewa+NE0Z5upb1GOE8XiTL9HA@mail.gmail.com>
Precedence: bulk

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