Gcc 16 switch - #502
Conversation
6295789 to
e2d82c0
Compare
|
Hi @vineetgarc . The error highlighted in the github UI is red herring. If you download the raw logs you'll get 469M log with this at the end: Please make sure the CI here is green. I suspect you need to adjust the denylist more. It's ok for it to be big at this point. |
|
btw, if you could come up with an (independent) fix for the misleading error that would be great too. The problem is that the script running the test_progs in a VM was killed due to the timeout, and so output post-processing didn't run, which is why we get this error downstream. The code doing all this is here: https://github.com/libbpf/ci/blob/main/run-vmtest/run.sh |
| # See: https://lore.kernel.org/bpf/87bjw6qpje.fsf@oracle.com/ | ||
| # if self.arch == Arch.X86_64: | ||
| # tests_list.append("test_progs-bpf_gcc") | ||
| if self.arch == Arch.X86_64: |
There was a problem hiding this comment.
We need to preserve the "if netdev" check here. We don't need to run gcc-bpf for netdev. The job that you deleted enforced !inputs.is_netdev. With this change every x86 netdev configuration now builds and schedules test_progs-bpf_gcc.
| MAX_MAKE_JOBS: 32 | ||
| RELEASE: ${{ inputs.release && '1' || '' }} | ||
| # Build test_progs-bpf_gcc on x86_64 so the vmtest runner has a binary to run. | ||
| BPF_GCC: ${{ inputs.arch == 'x86_64' && format('{0}/gcc-bpf', github.workspace) || '' }} |
There was a problem hiding this comment.
There is a problem with moving gcc-bpf path within the main kernel-build workflow: if something breaks with the scripts or selftests build, we now failing the "root" build job that every other test run depends on. Separate gcc-bpf workflow avoided that.
I think we can still delete gcc-bpf.yml, but then gcc-specific steps need to be allowed to fail and produce a separate artifact or something like that.
Think about how to do this. The main point is to keep gcc-bpf isolated: if it's broken, don't fail other independent jobs.
There was a problem hiding this comment.
I don't have a strong preference for deleting it - Claude suggested it was redundant after the changes. we can keep it and rework the patches along those lines.
There was a problem hiding this comment.
Ok so there are two plausible approaches:
Option 1 — self-contained gcc-bpf job (build and run in one job)
gcc-bpf.yml already downloads the vmlinux artifact, fetches the compiler, and builds test_progs-bpf_gcc. It just
throws the binary away. This option adds a run-vmtest step so the same job runs it.
Option 2 — gcc-bpf.yml builds and uploads; the test job downloads it
Keeps today's job split, but the binary is published instead of discarded.
Second option causes a delay of every job behind gcc-bpf it so not preferable.
But option 1 then wants a KVM runner (vs. builder runner) for gcc-bpf which is not the case currently.
Its a trade-off as always, which one do we prefer.
| # latest theihor/gcc-bpf release of that series is used. Override with | ||
| # the repo variable GCC_BPF_VERSION (Settings -> Secrets and variables | ||
| # -> Actions -> Variables) to try another series without a code change. | ||
| GCC_BPF_VERSION: ${{ vars.GCC_BPF_VERSION || '16' }} |
There was a problem hiding this comment.
I'd avoid having a default value here. TBH I don't think we need to track different releases for gcc-bpf at all: we only care about testing the "latest" one, so why bother? It's just an additional surface for bugs.
There was a problem hiding this comment.
I think it is still useful, even if only occasionally needed, to build something with a specific GCC release driven by a UI toggle.
| if [[ -n "$TAG_PREFIX" ]]; then | ||
| # Releases are listed newest first, so the first match is the latest one. | ||
| tag=$(gh release list -L 100 -R "${GH_REPO}" --json tagName -q '.[].tagName' \ | ||
| | grep -m1 -- "^${TAG_PREFIX}" || true) |
There was a problem hiding this comment.
looks like TAG_PREFIX is read as a regex
I think it should either be called "_REGEX" or be matched strictly as prefix
The traffic monitor names its logs after the test and subtest, so they can contain characters that actions/upload-artifact rejects. A subtest called "INET4: bpf_timestamping" makes the upload fail as below (see [1]) Error: The path for one of the files in artifact is not valid: /packets-125-15-net_timestamping__INET4:_bpf_timestamping-net_timestamping_ns.log. Contains the following character: Colon : run-vmtest already renames these, but only after the VM exits, so a step timeout or a dead VM kills it before that point. The upload step runs with if: always() and then trips over the names that were left behind. Rename them in a step of its own, also guarded by if: always(), so it happens whatever the outcome of the test step. It is a no-op when the directory is absent, which is the case for every test that does not enable the traffic monitor, and re-running it is harmless. Link: https://github.com/kernel-patches/vmtest/actions/runs/30710914503/job/9139901138 [1] Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
OK that's addressed via [1] and also a fix [2] in selftests to not generate those names in the first place. [1] #504 |
...
Yep added this test to denylist ! |
e2d82c0 to
40ee737
Compare
Pick the GCC BPF compiler by major version rather than taking whatever release
happens to be newest:
- download-gh-release.sh: accept an optional tag prefix and return the most
recent release matching it, so "gcc-16-" tracks the latest GCC 16 build. An
empty prefix keeps the previous latest-release behaviour, and a full tag
still works since a tag is a prefix of itself.
- gcc-bpf.yml: pass "gcc-${GCC_BPF_VERSION}-", defaulting to 15.
Plain "latest release" became ambiguous once more than one GCC series was
published to the same repo: the weekly job builds GCC 15, so a GCC 16 release
is only ever the newest one for as long as it takes the next GCC 15 snapshot to
land. Selecting by prefix makes the choice explicit and reproducible.
The prefix is matched literally rather than as a regular expression. Tags
contain dots, so passing a full tag through grep would let those dots match any
character and could pick a different release than the one asked for.
Defaulting to 15 keeps the current toolchain, so this changes nothing on its
own. Switching series is a one line change, or the repo variable
GCC_BPF_VERSION for an ad-hoc run.
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
Change the default GCC_BPF_VERSION from 15 to 16, so test_progs-bpf_gcc is built with the latest GCC 16 release, and regenerate the denylist to match what GCC 16 actually fails. The denylist shrinks from 904 to 193 entries (61 whole tests, 132 subtests). A test is denied outright only when nothing in it passes, otherwise just the failing subtests are listed so the rest keep running. Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
The GCC BPF workflow compiles test_progs-bpf_gcc and then throws it away, so it only ever checked that the selftests still build with GCC. Nothing runs them: the runner was disabled in f7504ae ("Disable test_progs-bpf_gcc") because too many tests were failing with GCC 15, and it has stayed off since. Split the workflow into a build job and a test job, in the same shape as test-progs-asan.yml. The build job keeps doing what it did and now packages test_progs-bpf_gcc as an artifact; the test job unpacks it over the selftests directory from the main build and runs it under vmtest. Keeping this in its own workflow rather than folding the GCC BPF build into kernel-build.yml is deliberate. Every test and veristat job depends on the root build job, so a break in the GCC BPF compiler download, in the scripts, or in the GCC selftests build would take out the whole matrix leg and the artifact it produces. Here it can only fail the GCC BPF jobs. The test job needs /dev/kvm, so kernel-build-test.yml now passes the test runners; the build job continues to pick its own builder image. Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
40ee737 to
c6a65cb
Compare
|
Superseded by #505 |
2 participants
No description provided.