by Maxime Ripard

[permalink] [raw]

Subject: Re: [PATCH 0/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing

On Thu, Feb 29, 2024 at 01:07:25AM +0200, Laurent Pinchart wrote:
> > Chat Discussions
> > ================
> >
> > For those interested in further discussions:
> >
> > **Join Our Slack Channel:**
> > We have a Slack channel, #gitlab-ci, on the KernelCI Slack instance https://kernelci.slack.com/ .
> > Feel free to join and contribute to the conversation. The KernelCI team has
> > weekly calls where we also discuss the GitLab-CI pipeline.
>
> Could we communicate using free software please ? Furthermore, it's not
> possible to create an account on that slack instance unless you have an
> e-mail address affiliated with a small number of companies
> (https://kernelci.slack.com/signup#/domain-signup). That's a big no-go
> for me.

Yeah, and that list looks super restrictive and arbitrary. Like,
microsoft is there but kernel.org isn't?

I'm sure there's a reason, but we should at the very least open it to
everyone.

Maxime

Attachments:

(No filename) (959.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-29 09:02:47

Hi!

On Thu, Feb 29, 2024 at 11:23:22AM +0200, Nikolai Kondrashov wrote:
> Hi everyone,
>
> On 2/29/24 11:02, Maxime Ripard wrote:
> > On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote:
> > > Which rating would you select?
> >
> > 4.5 :)
> >
> > One thing I'm wondering here is how we're going to cope with the
> > different requirements each user / framework has.
> >
> > Like, Linus probably want to have a different set of CI before merging a
> > PR than (say) linux-next does, or stable, or before doing an actual
> > release.
> >
> > Similarly, DRM probably has a different set of requirements than
> > drm-misc, drm-amd or nouveau.
> >
> > I don't see how the current architecture could accomodate for that. I
> > know that Gitlab allows to store issues template in a separate repo,
> > maybe we could ask them to provide a feature where the actions would be
> > separate from the main repo? That way, any gitlab project could provide
> > its own set of tests, without conflicting with each others (and we could
> > still share them if we wanted to)
> >
> > I know some of use had good relationship with Gitlab, so maybe it would
> > be worth asking?
>
> GitLab already supports getting the CI YAML from other repos. You can change
> that in the repo settings.

I'm interested but couldn't find it in the doc, do you have a link to
the right section?

> However, I think a better approach would be *not* to add the .gitlab-ci.yaml
> file in the root of the source tree, but instead change the very same repo
> setting to point to a particular entry YAML, *inside* the repo (somewhere
> under "ci" directory) instead.
>
> This way all the different subtrees can have completely different setup, but
> some could still use Helen's work and employ the "scenarios" she
> implemented.

I'm worried that this kind of setup will just create duplicated YAML
that will be developped in complete silos and will become difficult to
maintain. But that's definitely an opinion :)

Maxime

Attachments:

(No filename) (2.00 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-29 10:07:30

by Laurent Pinchart

[permalink] [raw]

Subject: Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing

On Thu, Feb 29, 2024 at 10:56:58AM +0100, Maxime Ripard wrote:
> Hi!
>
> On Thu, Feb 29, 2024 at 11:23:22AM +0200, Nikolai Kondrashov wrote:
> > Hi everyone,
> >
> > On 2/29/24 11:02, Maxime Ripard wrote:
> > > On Wed, Feb 28, 2024 at 07:55:25PM -0300, Helen Koike wrote:
> > > > Which rating would you select?
> > >
> > > 4.5 :)
> > >
> > > One thing I'm wondering here is how we're going to cope with the
> > > different requirements each user / framework has.
> > >
> > > Like, Linus probably want to have a different set of CI before merging a
> > > PR than (say) linux-next does, or stable, or before doing an actual
> > > release.
> > >
> > > Similarly, DRM probably has a different set of requirements than
> > > drm-misc, drm-amd or nouveau.
> > >
> > > I don't see how the current architecture could accomodate for that. I
> > > know that Gitlab allows to store issues template in a separate repo,
> > > maybe we could ask them to provide a feature where the actions would be
> > > separate from the main repo? That way, any gitlab project could provide
> > > its own set of tests, without conflicting with each others (and we could
> > > still share them if we wanted to)
> > >
> > > I know some of use had good relationship with Gitlab, so maybe it would
> > > be worth asking?
> >
> > GitLab already supports getting the CI YAML from other repos. You can change
> > that in the repo settings.
>
> I'm interested but couldn't find it in the doc, do you have a link to
> the right section?

e.g. https://gitlab.freedesktop.org/camera/libcamera/-/settings/ci_cd

Expand "General pipelines", the setting is "CI/CD configuration file".
You can specify the path to a file in the local repository, or in a
separate repository. See
https://gitlab.freedesktop.org/help/ci/pipelines/settings#specify-a-custom-cicd-configuration-file
for the syntax.

> > However, I think a better approach would be *not* to add the .gitlab-ci.yaml
> > file in the root of the source tree, but instead change the very same repo
> > setting to point to a particular entry YAML, *inside* the repo (somewhere
> > under "ci" directory) instead.
> >
> > This way all the different subtrees can have completely different setup, but
> > some could still use Helen's work and employ the "scenarios" she
> > implemented.
>
> I'm worried that this kind of setup will just create duplicated YAML
> that will be developped in complete silos and will become difficult to
> maintain. But that's definitely an opinion :)

--
Regards,

Laurent Pinchart

2024-02-29 11:09:58

On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven <[email protected]> wrote:
> > On Sun, Mar 3, 2024 at 3:30 AM Randy Dunlap <[email protected]> wrote:
> > > On 3/2/24 14:10, Guenter Roeck wrote:
> > > > While checkpatch is indeed of arguable value, I think it would help a
> > > > lot not having to bother about the persistent _build_ failures on
> > > > 32-bit systems. You mentioned the fancy drm CI system above, but they
> > > > don't run tests and not even test builds on 32-bit targets, which has
> > > > repeatedly caused (and currently does cause) build failures in drm
> > > > code when trying to build, say, arm:allmodconfig in linux-next. Most
> > > > trivial build failures in linux-next (and, yes, sometimes mainline)
> > > > could be prevented with a simple generic CI.
> > >
> > > Yes, definitely. Thanks for bringing that up.
> >
> > +1
>
> > Kisskb can send out email when builds get broken, and when they get
> > fixed again. I receive such emails for the m68k builds.
>
> Like this (yes, one more in DRM; sometimes I wonder if DRM is meant only
> for 64-bit little-endian platforms with +200 GiB/s memory bandwidth):
>
> ---8<-------------------------------------------------------------------
> Subject: kisskb: FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
> To: [email protected]
> Date: Mon, 04 Mar 2024 08:05:14 -0000
>
> FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/15135537/
>
> Commit: Add linux-next specific files for 20240304
> 67908bf6954b7635d33760ff6dfc189fc26ccc89
> Compiler: m68k-linux-gcc (GCC) 8.5.0 / GNU ld (GNU Binutils) 2.36.1
>
> Possible errors
> ---------------
>
> ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> make[2]: *** [Makefile:1871: modpost] Error 2
> make[1]: *** [Makefile:240: __sub-make] Error 2
> make: *** [Makefile:240: __sub-make] Error 2
>
> No warnings found in log.
> ------------------------------------------------------------------->8---

The driver is meant for a controller featured in an SoC with a Cortex-A8
ARM CPU and less than a GiB/s memory bandwidth.

And I just sent a fix for that one, thanks for the report.

Maxime

Attachments:

(No filename) (2.39 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-04 09:29:18

On Mon, Mar 04, 2024 at 11:07:22AM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> On Mon, Mar 4, 2024 at 10:15 AM Maxime Ripard <[email protected]> wrote:
> > On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> > > On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > On Sun, Mar 3, 2024 at 3:30 AM Randy Dunlap <[email protected]> wrote:
> > > > > On 3/2/24 14:10, Guenter Roeck wrote:
> > > > > > While checkpatch is indeed of arguable value, I think it would help a
> > > > > > lot not having to bother about the persistent _build_ failures on
> > > > > > 32-bit systems. You mentioned the fancy drm CI system above, but they
> > > > > > don't run tests and not even test builds on 32-bit targets, which has
> > > > > > repeatedly caused (and currently does cause) build failures in drm
> > > > > > code when trying to build, say, arm:allmodconfig in linux-next. Most
> > > > > > trivial build failures in linux-next (and, yes, sometimes mainline)
> > > > > > could be prevented with a simple generic CI.
> > > > >
> > > > > Yes, definitely. Thanks for bringing that up.
> > > >
> > > > +1
> > >
> > > > Kisskb can send out email when builds get broken, and when they get
> > > > fixed again. I receive such emails for the m68k builds.
> > >
> > > Like this (yes, one more in DRM; sometimes I wonder if DRM is meant only
> > > for 64-bit little-endian platforms with +200 GiB/s memory bandwidth):
> > >
> > > ---8<-------------------------------------------------------------------
> > > Subject: kisskb: FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
> > > To: [email protected]
> > > Date: Mon, 04 Mar 2024 08:05:14 -0000
> > >
> > > FAILED linux-next/m68k-allmodconfig/m68k-gcc8 Mon Mar 04, 06:35
> > >
> > > http://kisskb.ellerman.id.au/kisskb/buildresult/15135537/
> > >
> > > Commit: Add linux-next specific files for 20240304
> > > 67908bf6954b7635d33760ff6dfc189fc26ccc89
> > > Compiler: m68k-linux-gcc (GCC) 8.5.0 / GNU ld (GNU Binutils) 2.36.1
> > >
> > > Possible errors
> > > ---------------
> > >
> > > ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> > > make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> > > make[2]: *** [Makefile:1871: modpost] Error 2
> > > make[1]: *** [Makefile:240: __sub-make] Error 2
> > > make: *** [Makefile:240: __sub-make] Error 2
> > >
> > > No warnings found in log.
> > > ------------------------------------------------------------------->8---
> >
> > The driver is meant for a controller featured in an SoC with a Cortex-A8
> > ARM CPU and less than a GiB/s memory bandwidth.
>
> Good, so the hardware cannot possibly need 64-bit pixel clock values ;-)

This is an early patch to convert that function into a framework hook
implementation. HDMI 2.1 has a max TMDS character rate of slightly less
than 6GHz, so larger than 2^32 - 1.

So yes, this driver doesn't need to. The framework does however.

> BTW, doesn't the build fail on arm32, too?

It seems like gcc vs clang plays a role too. I had the same defconfig
building for arm with gcc and reporting the error above with clang. I
didn't look further because there was something to fix indeed.

Maxime

Attachments:

(No filename) (3.26 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-04 11:13:13

by Geert Uytterhoeven

[permalink] [raw]

Subject: Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing

Hi Maxime,

On Mon, Mar 4, 2024 at 11:20 AM Maxime Ripard <[email protected]> wrote:
> On Mon, Mar 04, 2024 at 11:07:22AM +0100, Geert Uytterhoeven wrote:
> > On Mon, Mar 4, 2024 at 10:15 AM Maxime Ripard <[email protected]> wrote:
> > > On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> > > > On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> > > > make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> > > > make[2]: *** [Makefile:1871: modpost] Error 2
> > > > make[1]: *** [Makefile:240: __sub-make] Error 2
> > > > make: *** [Makefile:240: __sub-make] Error 2
> > > >
> > > > No warnings found in log.
> > > > ------------------------------------------------------------------->8---
> > >
> > > The driver is meant for a controller featured in an SoC with a Cortex-A8
> > > ARM CPU and less than a GiB/s memory bandwidth.
> >
> > Good, so the hardware cannot possibly need 64-bit pixel clock values ;-)
>
> This is an early patch to convert that function into a framework hook
> implementation. HDMI 2.1 has a max TMDS character rate of slightly less
> than 6GHz, so larger than 2^32 - 1.
>
> So yes, this driver doesn't need to. The framework does however.

That's gonna be interesting, as the Common Clock Framework does not
support 64-bit clock rates on 32-bit platforms yet...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-03-04 11:34:13

by Maxime Ripard

[permalink] [raw]

Subject: Re: [PATCH 1/3] kci-gitlab: Introducing GitLab-CI Pipeline for Kernel Testing

On Mon, Mar 04, 2024 at 12:12:47PM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> On Mon, Mar 4, 2024 at 11:20 AM Maxime Ripard <[email protected]> wrote:
> > On Mon, Mar 04, 2024 at 11:07:22AM +0100, Geert Uytterhoeven wrote:
> > > On Mon, Mar 4, 2024 at 10:15 AM Maxime Ripard <[email protected]> wrote:
> > > > On Mon, Mar 04, 2024 at 09:12:38AM +0100, Geert Uytterhoeven wrote:
> > > > > On Sun, Mar 3, 2024 at 10:30 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > > ERROR: modpost: "__udivdi3" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
> > > > > make[3]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1
> > > > > make[2]: *** [Makefile:1871: modpost] Error 2
> > > > > make[1]: *** [Makefile:240: __sub-make] Error 2
> > > > > make: *** [Makefile:240: __sub-make] Error 2
> > > > >
> > > > > No warnings found in log.
> > > > > ------------------------------------------------------------------->8---
> > > >
> > > > The driver is meant for a controller featured in an SoC with a Cortex-A8
> > > > ARM CPU and less than a GiB/s memory bandwidth.
> > >
> > > Good, so the hardware cannot possibly need 64-bit pixel clock values ;-)
> >
> > This is an early patch to convert that function into a framework hook
> > implementation. HDMI 2.1 has a max TMDS character rate of slightly less
> > than 6GHz, so larger than 2^32 - 1.
> >
> > So yes, this driver doesn't need to. The framework does however.
>
> That's gonna be interesting, as the Common Clock Framework does not
> support 64-bit clock rates on 32-bit platforms yet...

As far as KMS goes, it doesn't really matter. The TMDS character rate
doesn't need to be backed by a CCF clock.

And realistically speaking, I don't expect any 32 bit platform to
support HDMI 2.1.

Maxime

Attachments:

(No filename) (1.78 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-04 16:07:15

On Mon, 2024-03-04 at 18:45 -0300, Helen Koike wrote:
> Hi Linus,
>
> Thank you for your reply and valuable inputs.
>
> On 01/03/2024 17:10, Linus Torvalds wrote:
> > On Fri, 1 Mar 2024 at 02:27, Nikolai Kondrashov <[email protected]> wrote:
> > >
> > > I agree, it's hard to imagine even a simple majority agreeing on how GitLab CI
> > > should be done. Still, we would like to help people, who are interested in
> > > this kind of thing, to set it up. How about we reframe this contribution as a
> > > sort of template, or a reference for people to start their setup with,
> > > assuming that most maintainers would want to tweak it? We would also be glad
> > > to stand by for questions and help, as people try to use it.
> >
> > Ack. I think seeing it as a library for various gitlab CI models would
> > be a lot more palatable. Particularly if you can then show that yes,
> > it is also relevant to our currently existing drm case.
>
> Having it as a library would certainly make my work as the DRM-CI
> maintainer easier and also simplify the process whenever we consider
> integrating tests into other subsystems.
>
> >
> > So I'm not objecting to having (for example) some kind of CI helper
> > templates - I think a logical place would be in tools/ci/ which is
> > kind of alongside our tools/testing subdirectory.
>
> Works for me.
>
> We can skip having a default .gitlab-ci.yml in the root directory and
> instead include clear instructions in our documentation for using these
> templates.

From previous experience[1], I recommend this approach.

This way it does not bother current Gitlab mirrors / personal repos, while
allowing anyone to setup the CI from Gitlab menus just by changing:

Repo -> Settings -> CI/CD -> General Pipelines -> CI/CD configuration file

Thanks!
Leo

[1] Last year I implemented Gitlab-CI for the perfbook repo, and I came across
some issues, including the disruption of .gitlab-ci.yml in the root of a repo.
https://lore.kernel.org/perfbook/[email protected]/

>
> Thanks,
> Helen Koike
>
> >
> > (And then perhaps have a 'gitlab' directory under that. I'm not sure
> > whether - and how much - commonality there might be between the
> > different CI models of different hosts).
> >
> > Just to clarify: when I say "a logical place", I very much want to
> > emphasize the "a" - maybe there are better places, and I'm not saying
> > that is the only possible place. But it sounds more logical to me than
> > some.
> >
> > Linus

2024-03-11 08:40:19

On Mon, Mar 04, 2024 at 06:45:33PM -0300, Helen Koike wrote:
> Hi Linus,
>
> Thank you for your reply and valuable inputs.
>
> On 01/03/2024 17:10, Linus Torvalds wrote:
> > On Fri, 1 Mar 2024 at 02:27, Nikolai Kondrashov <[email protected]> wrote:
> > >
> > > I agree, it's hard to imagine even a simple majority agreeing on how GitLab CI
> > > should be done. Still, we would like to help people, who are interested in
> > > this kind of thing, to set it up. How about we reframe this contribution as a
> > > sort of template, or a reference for people to start their setup with,
> > > assuming that most maintainers would want to tweak it? We would also be glad
> > > to stand by for questions and help, as people try to use it.
> >
> > Ack. I think seeing it as a library for various gitlab CI models would
> > be a lot more palatable. Particularly if you can then show that yes,
> > it is also relevant to our currently existing drm case.
>
> Having it as a library would certainly make my work as the DRM-CI maintainer
> easier and also simplify the process whenever we consider integrating tests
> into other subsystems.

Kinda ignored this thread, just wanted to throw my +1 in here.

To spin it positively, the kernel CI space is wide open (more negatively,
it's a fractured mess). And I think there's just no way to force top-down
unification. Imo the only way is to land subsystem CI support in upstream,
figure out what exactly that should look like (I sketched a lot of open
questions in the DRM CI PR around what should and should not be in
upstream).

Then, once we have a few of those, extract common scripts and tools into
tools/ci/ or scripts/ci or whatever.

And only then, best case years down the road, dare to have some common
top-level CI, once it's clear what the actual common pieces and test
stages even are.

> > So I'm not objecting to having (for example) some kind of CI helper
> > templates - I think a logical place would be in tools/ci/ which is
> > kind of alongside our tools/testing subdirectory.
>
> Works for me.
>
> We can skip having a default .gitlab-ci.yml in the root directory and
> instead include clear instructions in our documentation for using these
> templates.

I'd go a few steps more back and start with trying to get more subsystem
CI into upstream. And then once that dust has settled, figure out what the
common pieces actually are. Because I'm pretty sure that what we have for
drm ci or kernelci right now won't be it, but likely just a local optimum.

Cheers, Sima

>
> Thanks,
> Helen Koike
>
> >
> > (And then perhaps have a 'gitlab' directory under that. I'm not sure
> > whether - and how much - commonality there might be between the
> > different CI models of different hosts).
> >
> > Just to clarify: when I say "a logical place", I very much want to
> > emphasize the "a" - maybe there are better places, and I'm not saying
> > that is the only possible place. But it sounds more logical to me than
> > some.
> >
> > Linus

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