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 other
> 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 proof
that all is working for a specific commit, but I can't see how this will
help the kernel out much.
What are you going to do with these results being checked in all the
time?
thanks,
greg k-h
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 other
>> 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 proof
> that all is working for a specific commit, but I can't see how this will
> 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 merging driver changes which make any of the tests deviate from the expected result.
Keeping them separate inevitably results in divergence between the driver code and the expected test results, which would result in spurious failures of such CI.
I expect the main complication for the kernel will be due to driver changes merged via different trees, e.g. for cross-subsystem reworks. Since those will not go through the same CI, they may accidentally introduce inconsistencies. The ideal solution for this IMO would be centralizing CI such that 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. :)
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
On Wed, May 11, 2022 at 12:26:05PM +0200, Michel D?nzer 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 other
> >> 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 proof
> > that all is working for a specific commit, but I can't see how this will
> > 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 merging driver changes which make any of the tests deviate from the expected result.
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.
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 driver code and the expected test results, which would result in spurious failures of such CI.
Again, "pass" should be the expected results :)
> I expect the main complication for the kernel will be due to driver changes merged via different trees, e.g. for cross-subsystem reworks. Since those will not go through the same CI, they may accidentally introduce inconsistencies. The ideal solution for this IMO would be centralizing CI such that 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?
thanks,
greg k-h
On Wed, May 11, 2022 at 4:50 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Wed, May 11, 2022 at 12:26:05PM +0200, Michel Dänzer 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 other
> > >> 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 proof
> > > that all is working for a specific commit, but I can't see how this will
> > > 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 merging driver changes which make any of the tests deviate from the expected result.
>
> 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 driver code and the expected test results, which would result in spurious failures of such CI.
>
> Again, "pass" should be the expected results :)
>
> > I expect the main complication for the kernel will be due to driver changes merged via different trees, e.g. for cross-subsystem reworks. Since those will not go through the same CI, they may accidentally introduce inconsistencies. The ideal solution for this IMO would be centralizing CI such that 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
On Wed, May 11, 2022 at 06:33:32AM -0700, Rob Clark wrote:
>
> 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.)
There are tradeoffs both ways, whether the patches are kept separate,
opr in the kernel tree.
In the file system world, when we discover a bug, very often a test
case is found to test the fix, and to protect us against regressions.
It has one other benefit; since the tests (xfstests) are kept separate
from the kernel, it's a useful way to identify when some patch didn't
get automatically backported to a LTS or distro kernel. (For example,
because the patch didn't cherry-pick cleanly and the manual backport
process fell through the cracks.)
It does make things annoying when we have bugs that can not be safely
backported (which results in tests that fail on the LTS kernel without
kernel-version exclude files), and/or when the expectations change
between versions. (Although to be honest, for us, the more common
annoyance is when some userspace package --- e.g., bash or coreutils
or util-linux --- changes their output, and we have to add filter
functions to accomodate expected output differences.)
- Ted