Upstream changes from perf-event-open-sys2#42
Merged
jimblandy merged 15 commits intojimblandy:masterfrom Apr 9, 2025
Merged
Conversation
This commit contains two main changes: - regenerate.sh has been rewritten to generate bindings from a local copy of the linux kernel source instead of using the headers installed on the current system. - The bindings for x86 and arm64 have been regenerated from kernel version 6.2.10. The benefit of this change is that it is easier to update the generated bindings. It is not necessary to get a live install of whatever the newest kernel version is to generate bindings for a new kernel version. This is just barely doable with x86_64 and aarch64 but is rather impractical for any one person to do once we think about potentially bringing in other architectures. After this change, all that needs to be done is update regenerate.sh with the new kernel verison and run it.
Phantomical
force-pushed
the
upstream-sys-changes
branch
from
9fc2db1 to
9731b10
Compare
Owner
|
This looks amazing. As I've said elsewhere, we definitely want the changes for regenerating the bindings directly from the kernel sources. And certainly our bindings should not include non-perf types and constants. I am not sure about the I'd also like the chance to understand the anonymous union trick a little better. Would it be possible to split that out into a separate PR as well? |
The asm/perf_event.h includes definitions for the registers for the current architecture. If you want to tell perf_event_open to sample CPU registers then you need these definitions.
It is not necessary since we can generically cast to the right value by doing `ioctl as _`.
The current way of generating bindings via bindgen creates entries for any type or constant that is reachable when including the headers listed in wrapper.h. In addition to the perf-related structs we care about, this includes: - A variety of unrelated kernel API types - A bunch of constants used as implementation details for making ioctls - A constant for every single syscall number - ... and more besides There is no reason to include these in the generated bindings. Once present, however, removing or changing them becomes a breaking change. This is a one-shot change that changes the generation to only keep the relevant structs. This is a breaking change and would require a perf-event-open-sys2 v6.0.0.
Phantomical
force-pushed
the
upstream-sys-changes
branch
from
9731b10 to
a191795
Compare
Phantomical
force-pushed
the
upstream-sys-changes
branch
from
a191795 to
58b96ad
Compare
Collaborator
Author
|
Sure. I've stripped out those commits. Once this one is merged I'll make more PRs with those changes. |
Owner
|
These files seem to be generated by an older version of bindgen. Could you try regenerating them with 0.71.1? |
Collaborator
Author
|
Done. To be honest, I was not expecting it to have that many changes going from 0.69 to 0.71. |
Owner
|
Thanks! |
This was referenced Apr 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a (mostly) straight upstream of the various changes I have made to perf-event-open-sys2. These are few enough that I figure I can upstream them all at once. I don't intend to do this with changes to the
perf-eventcrate, which are more involved.Here's the list of relevant changes:
regenerate.shscript from Generateperf_event_open_sysbindings directly from the linux kernel source #32.perf_.*structs are now annotated with annotated with#[non_exhaustive].new_bitfieldvariants).perf_event_attrwhich allows accessing the fields in unnamed union fields as if they were regular fields on the struct.perf_event_open.This isn't a straight upstream. I have made a few small tweaks where I think appropriate. These tweaks are:
memoffsetcrate anymore.std::mem::offset_of!has been stable for long enough now that it is fine to just use it.src/versionno longer includes an extra period at the end, this is instead added in the docs viaconcat!cargo semver-checksfor versioning.perf_event_open, since theaux_actionfield was turned into an unnamed union in the kernel headers. I have not handled the unnamed bitfield in the union yet, I'll do that in a follow-up PR.Otherwise the main theme of these changes (other than the regenerate script) is this: if it is not a breaking change in the C header, that should generally not be a breaking change for the bindings here. In practice I found that major version bumps in
perf-event-open-sys2required a corresponding major version bump inperf-event2, which is a bit of a pain for everyone involved.There are some tradeoffs here. It is no longer possible to use the struct update syntax with
perf_event_attr. That is basically the only real loss though, and I think that is a fair price to pay for avoiding making a breaking release every time somebody makes an update toperf_event_attron the kernel side.None of this is final though! We can make changes (either here or in a follow up) to better address certain use cases. I'm happy to hear feedback on this :)