| Message ID | 1446201872-17474-1-git-send-email-toke@toke.dk |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2015-10-30 11:44, Toke Høiland-Jørgensen wrote: > This change adds support for specifying that a particular kernel module > wants to keep its build ID debug symbol (.note.gnu.build-id). This > symbol is exported in sysfs by the kernel (if the kernel is compiled > with CONFIG_KALLSYMS) and so can be used to uniquely identify a version > of a kernel module in a running kernel (if the module is built with > suitable linker options). This is useful for keeping track of different > versions of a module when doing experiments and development. > > A kernel module Makefile can specify that the build ID should be kept by > exporting PKG_KEEP_BUILD_ID in the Build/Exports section. This will add > ~100 bytes to the size of the .ko (depending on the length of the build > ID specified). > > The default is to strip the build ID (as before), so there is no size > difference for kernel modules that do not export this variable. > > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> How is this more useful than simply checking a hash of the kernel module in /lib/modules and comparing that with the corresponding file on the host? - Felix
Felix Fietkau <nbd@openwrt.org> writes: > How is this more useful than simply checking a hash of the kernel > module in /lib/modules and comparing that with the corresponding file > on the host? Because the value in /sys/module/ is from the *loaded* module. So it catches the case where the module file is updated but not reloaded (was bitten by this a couple of times). Also, you can put the module source git hash as the build id, so it can be referred back to the source, rather than the compiled file. -Toke
On 2015-10-30 13:54, Toke Høiland-Jørgensen wrote: > Felix Fietkau <nbd@openwrt.org> writes: > >> How is this more useful than simply checking a hash of the kernel >> module in /lib/modules and comparing that with the corresponding file >> on the host? > > Because the value in /sys/module/ is from the *loaded* module. So it > catches the case where the module file is updated but not reloaded (was > bitten by this a couple of times). > > Also, you can put the module source git hash as the build id, so it can > be referred back to the source, rather than the compiled file. I'm still not convinced that this is very useful - if you have issues that you sometimes reinstall modules, but don't reload them and have to check the id, why not just fix your workflow instead? You could just make a simple wrapper script that reinstalls and reloads your module and complains if reloading fails. When I do wifi driver development, I have to reload a chain of modules frequently. Since this is tedious and error prone, I have a script that simply pulls the latest package from my laptop via HTTP, unloads all relevant modules, then reloads them. Either way, if this is really necessary for you, I'm okay with adding support for keeping the build-id, but setting this in the package makefile (especially in the Build/Exports section) seems rather quirky to me. Since this is a rather exotic debugging-only feature, why not just add a global config option for it? - Felix
Felix Fietkau <nbd@openwrt.org> writes: > I'm still not convinced that this is very useful - if you have issues > that you sometimes reinstall modules, but don't reload them and have > to check the id, why not just fix your workflow instead? I did script it, but I'm trying to have a way to make this available to others as well. Let me give a bit more context: I'm using this for the 'cake' shaper module, which is built out-of-tree as kmod-sched-cake, available in the ceropackages feed. Other people are testing this, with their own builds; and I want to be able to keep track of which versions they are testing with, without having to impose a specific workflow for them: With this change, I can just add the build-id linker option in the package Makefile, and it will be available on the router running the code, where it can be automatically extracted by the Flent tool as part of the test. > Either way, if this is really necessary for you, I'm okay with adding > support for keeping the build-id, but setting this in the package > makefile (especially in the Build/Exports section) seems rather quirky > to me. Since this is a rather exotic debugging-only feature, why not > just add a global config option for it? Well I was trying to find a way to make it possible to set this for one module (package) only, in order to keep the impact as low as possible. And this was the only I could figure out to make it work (since the RSTRIP variable in rules.mk seems to be expanded before the contents of the package Makefile, so I can't add a variable there). But since this requires a specific kernel configuration anyway (CONFIG_KALLSYMS), global config changes are needed anyway, so am fine with making it a global switch. Can resubmit is as such. Should I just add a new switch under "Global build settings"? Or would it be okay to condition it on CONFIG_KERNEL_KALLSYMS (which it needs to work anyway)? Incidentally, is there a way to depend on these options from the package Makefile? -Toke
On 2015-10-30 14:37, Toke Høiland-Jørgensen wrote: > Felix Fietkau <nbd@openwrt.org> writes: > >> I'm still not convinced that this is very useful - if you have issues >> that you sometimes reinstall modules, but don't reload them and have >> to check the id, why not just fix your workflow instead? > > I did script it, but I'm trying to have a way to make this available to > others as well. Let me give a bit more context: > > I'm using this for the 'cake' shaper module, which is built out-of-tree > as kmod-sched-cake, available in the ceropackages feed. Other people are > testing this, with their own builds; and I want to be able to keep track > of which versions they are testing with, without having to impose a > specific workflow for them: With this change, I can just add the > build-id linker option in the package Makefile, and it will be available > on the router running the code, where it can be automatically extracted > by the Flent tool as part of the test. As far as I know, the build-id is build host specific, so I don't think it will really help you with tracking versions of other people's builds. >> Either way, if this is really necessary for you, I'm okay with adding >> support for keeping the build-id, but setting this in the package >> makefile (especially in the Build/Exports section) seems rather quirky >> to me. Since this is a rather exotic debugging-only feature, why not >> just add a global config option for it? > > Well I was trying to find a way to make it possible to set this for one > module (package) only, in order to keep the impact as low as possible. > And this was the only I could figure out to make it work (since the > RSTRIP variable in rules.mk seems to be expanded before the contents of > the package Makefile, so I can't add a variable there). > > But since this requires a specific kernel configuration anyway > (CONFIG_KALLSYMS), global config changes are needed anyway, so am fine > with making it a global switch. Can resubmit is as such. > > Should I just add a new switch under "Global build settings"? Or would > it be okay to condition it on CONFIG_KERNEL_KALLSYMS (which it needs to > work anyway)? > > Incidentally, is there a way to depend on these options from the package > Makefile? I think for what you're trying to do, it might be better to use something else. You could just embed a custom section in your module containing a git hash of the source tree, or something like that. - Felix
Felix Fietkau <nbd@openwrt.org> writes: > As far as I know, the build-id is build host specific, so I don't think > it will really help you with tracking versions of other people's > builds. Yup. But you can pass it explicitly to the linker: LDFLAGS_MODULE=--build-id=0x$(PKG_SOURCE_VERSION) where PKG_SOURCE_VERSION is the git hash. > I think for what you're trying to do, it might be better to use > something else. You could just embed a custom section in your module > containing a git hash of the source tree, or something like that. Well the nice thing about this is that it requires very little change in other places: The facility to export it in sysfs is already in the kernel, and it's a one-line change to the build invocation (as above) to get it included. If you're opposed to adding this change, I guess that adding a custom note section to the module might be an alternative way to go about it; if I can figure out how to do that... -Toke
On 2015-10-30 15:03, Toke Høiland-Jørgensen wrote: > Felix Fietkau <nbd@openwrt.org> writes: > >> As far as I know, the build-id is build host specific, so I don't think >> it will really help you with tracking versions of other people's >> builds. > > Yup. But you can pass it explicitly to the linker: > > LDFLAGS_MODULE=--build-id=0x$(PKG_SOURCE_VERSION) > > where PKG_SOURCE_VERSION is the git hash. > >> I think for what you're trying to do, it might be better to use >> something else. You could just embed a custom section in your module >> containing a git hash of the source tree, or something like that. > > Well the nice thing about this is that it requires very little change in > other places: The facility to export it in sysfs is already in the > kernel, and it's a one-line change to the build invocation (as above) to > get it included. > > If you're opposed to adding this change, I guess that adding a custom > note section to the module might be an alternative way to go about it; > if I can figure out how to do that... How about this: add it per-package, but add it in a way that you can set PKG_BUILD_ID:=$(PKG_SOURCE_VERSION) and this will add the linker command and disable the build-id stripping. - Felix
Felix Fietkau <nbd@openwrt.org> writes: > How about this: add it per-package, but add it in a way that you can set > PKG_BUILD_ID:=$(PKG_SOURCE_VERSION) and this will add the linker command > and disable the build-id stripping. Would definitely work. The problem is I'm not sure I quite grok the openwrt build system sufficiently to do this correctly. Guess I need to get the linker flag into KERNEL_MAKEOPTS? Can go looking for that, but that still leaves the problem of signaling the strip-kmod.sh script correctly -- can I get at per-package variables in rules.mk where the RSTRIP invocation is defined? -Toke
On 2015-10-30 15:17, Toke Høiland-Jørgensen wrote: > Felix Fietkau <nbd@openwrt.org> writes: > >> How about this: add it per-package, but add it in a way that you can set >> PKG_BUILD_ID:=$(PKG_SOURCE_VERSION) and this will add the linker command >> and disable the build-id stripping. > > Would definitely work. The problem is I'm not sure I quite grok the > openwrt build system sufficiently to do this correctly. Guess I need to > get the linker flag into KERNEL_MAKEOPTS? Yes. It's in kernel-defaults.mk > Can go looking for that, but > that still leaves the problem of signaling the strip-kmod.sh script > correctly -- can I get at per-package variables in rules.mk where the > RSTRIP invocation is defined? You can add it wrapped in $(if ...) to rules.mk to the other exports in the RSTRIP variable. - Felix
diff --git a/scripts/strip-kmod.sh b/scripts/strip-kmod.sh index 13e6b58..d6fa10d 100755 --- a/scripts/strip-kmod.sh +++ b/scripts/strip-kmod.sh @@ -18,11 +18,14 @@ else ARGS="-x -G __this_module --strip-unneeded" fi +if [ -z "$PKG_KEEP_BUILD_ID" ]; then + ARGS="$ARGS -R .note.gnu.build-id" +fi + ${CROSS}objcopy \ -R .comment \ -R .pdr \ -R .mdebug.abi32 \ - -R .note.gnu.build-id \ -R .gnu.attributes \ -R .reginfo \ $ARGS \
This change adds support for specifying that a particular kernel module wants to keep its build ID debug symbol (.note.gnu.build-id). This symbol is exported in sysfs by the kernel (if the kernel is compiled with CONFIG_KALLSYMS) and so can be used to uniquely identify a version of a kernel module in a running kernel (if the module is built with suitable linker options). This is useful for keeping track of different versions of a module when doing experiments and development. A kernel module Makefile can specify that the build ID should be kept by exporting PKG_KEEP_BUILD_ID in the Build/Exports section. This will add ~100 bytes to the size of the .ko (depending on the length of the build ID specified). The default is to strip the build ID (as before), so there is no size difference for kernel modules that do not export this variable. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- scripts/strip-kmod.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)