Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220510070140.45407-1-tomeu.vizoso@collabora.com>
 <20220510141329.54414-1-tomeu.vizoso@collabora.com> <CAPM=9tzLR-wsLhg2ikGjoK06s-ju5XWa1rtPPiUpN=pwD1vgtA@mail.gmail.com>
 <YntWQIXSqMCd6TYV@kroah.com> <1255a66a-121d-988a-19a7-316f703cb37d@mailbox.org>
 <YnujG0nkF0U6d5kd@kroah.com>
In-Reply-To: <YnujG0nkF0U6d5kd@kroah.com>
From:   Rob Clark <robdclark@gmail.com>
Date:   Wed, 11 May 2022 06:33:32 -0700
Message-ID: <CAF6AEGsmD-CNGj4bAE952JQpquaWA+Nxo5TGpFiHqaPK9doP-g@mail.gmail.com>
Subject: Re: [Freedreno] Adding CI results to the kernel tree was Re: [RFC v2]
 drm/msm: Add initial ci/ subdirectory
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     =?UTF-8?Q?Michel_D=C3=A4nzer?= <michel.daenzer@mailbox.org>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Jonathan Corbet <corbet@lwn.net>,
        freedreno <freedreno@lists.freedesktop.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Dave Airlie <airlied@gmail.com>, Sean Paul <sean@poorly.run>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 11, 2022 at 4:50 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, May 11, 2022 at 12:26:05PM +0200, Michel D=C3=A4nzer wrote:
> > On 2022-05-11 08:22, Greg Kroah-Hartman wrote:
> > > On Wed, May 11, 2022 at 03:06:47PM +1000, Dave Airlie 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 othe=
r
> > >> subsystem has done this already, and it's all fine.
> > >
> > > Why does the results need to be added to the tree?  Shouldn't they be
> > > either "all is good" or "constantly changing and a constant churn"?
> > >
> > >> 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.
> > >
> > > For systems that have "tight integration" this might make sense as pr=
oof
> > > that all is working for a specific commit, but I can't see how this w=
ill
> > > help the kernel out much.
> > >
> > > What are you going to do with these results being checked in all the
> > > time?
> >
> > Having the expected results in the tree keeps them consistent with the =
driver code itself, and allows putting in place gating CI to prevent mergin=
g driver changes which make any of the tests deviate from the expected resu=
lt.
>
> Shouldn't "expected result" always be "pass"?
>
> If not, then the test should be changed to be "skipped" like we have
> today in the kselftest tests.

No, we want to run tests even if they are expected to fail.  This
prevents the scenario of a test getting fixed without being noticed
(for ex, developer was working on fixing test A and didn't notice that
the fix also fixed test B).  If a fix goes unnoticed, a later
regression would also go unnoticed ;-)

I was skeptical about this approach at first with mesa CI, but having
used mesa CI for a while, I am now a firm believer in the approach.

And ofc we want the expectations to be in the kernel tree because
there could be, for example, differences between -fixes and -next
branches.  (Or even stable kernel branches if/when we get to the point
of running CI on those.)

> And how about tieing this into the kselftest process as well, why would
> this be somehow separate from the rest of the kernel tests?
>
> > Keeping them separate inevitably results in divergence between the driv=
er code and the expected test results, which would result in spurious failu=
res of such CI.
>
> Again, "pass" should be the expected results :)
>
> > I expect the main complication for the kernel will be due to driver cha=
nges merged via different trees, e.g. for cross-subsystem reworks. Since th=
ose will not go through the same CI, they may accidentally introduce incons=
istencies. The ideal solution for this IMO would be centralizing CI such th=
at the same gating tests have to pass regardless of how the code is merged.=
 But there's likely quite a long way to go until we get there. :)
>
> We have in-kernel tests for the rest of the kernel, why can't you put
> your testing stuff into there as well?

We could ofc put a lot more of the gitlab yml and scripts into the
kernel tree.  Probably all of i-g-t is a bit much to put in the kernel
tree.  Not to mention I'd like to see this expand to also run some
deqp and/or piglit tests, which is definitely too much to vendor into
the kernel tree.

The approach of this RFC was to put only what was absolutely required
in the kernel tree (such as expectations), and then link out to an
external drm-ci tree[1] which has all the necessary scripts and yml
for building and running tests, to avoid having to put a whole lot
more in the kernel tree. (We should be specifying exact commit-sha for
that tree, IMO, as it controls the version of i-g-t which gets used,
and we need to be able to update expectations in sync with an i-g-t
uprev, for example when new tests are added or if a test fix caused a
fail->pass transition.)

BR,
-R

[1] https://gitlab.freedesktop.org/gfx-ci/drm-ci

> thanks,
>
> greg k-h