* [Buildroot] [PATCH] Add Linux kernel selftests building @ 2015-11-24 23:53 Cyril Bur 2015-11-24 23:53 ` [Buildroot] [PATCH] linux: Build and install kernel selftests Cyril Bur 0 siblings, 1 reply; 12+ messages in thread From: Cyril Bur @ 2015-11-24 23:53 UTC (permalink / raw) To: buildroot Hi, Being able to boot and run the kernel selftests on development machines has proven very useful recently. The one caveat in this patch is that the kernel selftests and associated build infrastructure leaves a bit to be desired so enabling the option makes the build messy, nothing much we can do here and I'm assuming that anyone turns it on, they know what they're doing anyway. Cyril Bur (1): linux: Build and install kernel selftests linux/Config.tools.in | 12 ++++++++++++ linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 linux/linux-tool-selftests.mk -- 2.6.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-11-24 23:53 [Buildroot] [PATCH] Add Linux kernel selftests building Cyril Bur @ 2015-11-24 23:53 ` Cyril Bur 2015-12-14 22:18 ` Yann E. MORIN 0 siblings, 1 reply; 12+ messages in thread From: Cyril Bur @ 2015-11-24 23:53 UTC (permalink / raw) To: buildroot This patch simply adds the ability to compile and install the kernel selftests into the target. This is likely to be a rarely used debugging/performance feature for development and unlikely to be used in a production configuration. Signed-off-by: Cyril Bur <cyrilbur@gmail.com> --- linux/Config.tools.in | 12 ++++++++++++ linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 linux/linux-tool-selftests.mk diff --git a/linux/Config.tools.in b/linux/Config.tools.in index 24ef8cd..68655dc 100644 --- a/linux/Config.tools.in +++ b/linux/Config.tools.in @@ -26,4 +26,16 @@ config BR2_LINUX_KERNEL_TOOL_PERF https://perf.wiki.kernel.org/ +config BR2_LINUX_KERNEL_TOOL_SELFTESTS + bool "selftests" + help + Build and install (to /usr/lib/selftests) kernel selftests. + + Use of this option implies you know the process using and compiling + the kernel selftests. The Makefile to build and install these is very + noisy and may appear to cause your build to fail for strange reasons. + + This is very much a use at your risk option and may not work for + every setup or every architecture. + endmenu diff --git a/linux/linux-tool-selftests.mk b/linux/linux-tool-selftests.mk new file mode 100644 index 0000000..573ba0c --- /dev/null +++ b/linux/linux-tool-selftests.mk @@ -0,0 +1,40 @@ +################################################################################ +# +# selftests +# +################################################################################ + +LINUX_TOOLS += selftests + +ifeq ($(KERNEL_ARCH),x86_64) +SELFTESTS_ARCH=x86 +else +SELFTESTS_ARCH=$(KERNEL_ARCH) +endif + +SELFTESTS_DEPENDENCIES = bash + +SELFTESTS_INSTALL_STAGING = YES +SELFTESTS_MAKE_FLAGS = \ + $(LINUX_MAKE_FLAGS) \ + ARCH=$(SELFTESTS_ARCH) + +# O must be redefined here to overwrite the one used by Buildroot for +# out of tree build. We build the selftests in $(@D)/tools/selftests and +# not just $(@D) so that it isn't built in the root directory of the kernel +# sources. +define SELFTESTS_BUILD_CMDS + $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) headers_install + $(TARGET_MAKE_ENV) $(MAKE1) $(SELFTESTS_MAKE_FLAGS) \ + -C $(@D)/tools/testing/selftests O=$(@D)/tools/testing/selftests +endef + +define SELFTESTS_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(STAGING_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ + -C $(@D)/tools/testing/selftests install +endef + +define SELFTESTS_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(TARGET_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ + -C $(@D)/tools/testing/selftests install +endef -- 2.6.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-11-24 23:53 ` [Buildroot] [PATCH] linux: Build and install kernel selftests Cyril Bur @ 2015-12-14 22:18 ` Yann E. MORIN 2015-12-15 6:37 ` Baruch Siach 2015-12-17 1:45 ` Cyril Bur 0 siblings, 2 replies; 12+ messages in thread From: Yann E. MORIN @ 2015-12-14 22:18 UTC (permalink / raw) To: buildroot Cyril, All, Sorry for the delay, but I'm now looking at this patch... On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > This patch simply adds the ability to compile and install the kernel > selftests into the target. > > This is likely to be a rarely used debugging/performance feature for > development and unlikely to be used in a production configuration. > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > --- > linux/Config.tools.in | 12 ++++++++++++ > linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > create mode 100644 linux/linux-tool-selftests.mk > > diff --git a/linux/Config.tools.in b/linux/Config.tools.in > index 24ef8cd..68655dc 100644 > --- a/linux/Config.tools.in > +++ b/linux/Config.tools.in > @@ -26,4 +26,16 @@ config BR2_LINUX_KERNEL_TOOL_PERF > > https://perf.wiki.kernel.org/ > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > + bool "selftests" Since the .mk has: SELFTESTS_DEPENDENCIES = bash then you must either depend on bash or select it here. I think a select is better, so: depends on BR2_USE_MMU # bash select BR2_PACKAGE_BASH > + help > + Build and install (to /usr/lib/selftests) kernel selftests. Why do you install in there? I'm not sure what the kernel selftests are, but two options: - if they are executables, they should go into /usr/sbin - if they are libraries, they should go in /usr/lib > + Use of this option implies you know the process using and compiling > + the kernel selftests. The Makefile to build and install these is very > + noisy and may appear to cause your build to fail for strange reasons. > + > + This is very much a use at your risk option and may not work for > + every setup or every architecture. > + > endmenu > diff --git a/linux/linux-tool-selftests.mk b/linux/linux-tool-selftests.mk > new file mode 100644 > index 0000000..573ba0c > --- /dev/null > +++ b/linux/linux-tool-selftests.mk > @@ -0,0 +1,40 @@ > +################################################################################ > +# > +# selftests > +# > +################################################################################ > + > +LINUX_TOOLS += selftests linux-tool infrastructure. Yeah! :-) > +ifeq ($(KERNEL_ARCH),x86_64) > +SELFTESTS_ARCH=x86 > +else > +SELFTESTS_ARCH=$(KERNEL_ARCH) > +endif Not related to your patch, but I wonder if we should not make that a common conditional, so that all tools do not have to repeat it. Since there are only two such tool (with selftests) that do it, it can be put aside for now, and we can revisit it later. > +SELFTESTS_DEPENDENCIES = bash > + > +SELFTESTS_INSTALL_STAGING = YES Why install it in staging? > +SELFTESTS_MAKE_FLAGS = \ > + $(LINUX_MAKE_FLAGS) \ > + ARCH=$(SELFTESTS_ARCH) > + > +# O must be redefined here to overwrite the one used by Buildroot for > +# out of tree build. We build the selftests in $(@D)/tools/selftests and > +# not just $(@D) so that it isn't built in the root directory of the kernel > +# sources. > +define SELFTESTS_BUILD_CMDS > + $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) headers_install Headers are target dependent, so I think you also need to pass $(SELFTESTS_MAKE_FLAGS) in there, no? Why are headers needed? > + $(TARGET_MAKE_ENV) $(MAKE1) $(SELFTESTS_MAKE_FLAGS) \ > + -C $(@D)/tools/testing/selftests O=$(@D)/tools/testing/selftests I prefer when the -C args are passed early (if possible first) because it is then much obvious where the build is taking place: $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \ $(SELFTESTS_MAKE_FLAGS) \ O=$(@D)/tools/testing/selftests > +endef > + > +define SELFTESTS_INSTALL_STAGING_CMDS > + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(STAGING_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ > + -C $(@D)/tools/testing/selftests install Line too long, split it to below ~72 chars; also, I prefer we keep the generic flags early; probably you also have to repeat the O arg: $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \ $(SELFTESTS_MAKE_FLAGS) \ O=$(@D)/tools/testing/selftests \ INSTALL_PATH=$(STAGING_DIR)/usr/lib/selftests \ install > +endef > + > +define SELFTESTS_INSTALL_TARGET_CMDS > + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(TARGET_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ > + -C $(@D)/tools/testing/selftests install Ditto. Regards, Yann E. MORIN. > +endef > -- > 2.6.2 > > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-14 22:18 ` Yann E. MORIN @ 2015-12-15 6:37 ` Baruch Siach 2015-12-15 17:31 ` Yann E. MORIN 2015-12-17 1:45 ` Cyril Bur 1 sibling, 1 reply; 12+ messages in thread From: Baruch Siach @ 2015-12-15 6:37 UTC (permalink / raw) To: buildroot Hi Yann, On Mon, Dec 14, 2015 at 11:18:46PM +0100, Yann E. MORIN wrote: > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > + bool "selftests" > > Since the .mk has: > SELFTESTS_DEPENDENCIES = bash > > then you must either depend on bash or select it here. I think a select > is better, so: > > depends on BR2_USE_MMU # bash > select BR2_PACKAGE_BASH But BR2_PACKAGE_BASH depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS. Should BR2_LINUX_KERNEL_TOOL_SELFTESTS also depend on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-15 6:37 ` Baruch Siach @ 2015-12-15 17:31 ` Yann E. MORIN 2015-12-17 1:25 ` Cyril Bur 0 siblings, 1 reply; 12+ messages in thread From: Yann E. MORIN @ 2015-12-15 17:31 UTC (permalink / raw) To: buildroot Baruch, Cyril, All, On 2015-12-15 08:37 +0200, Baruch Siach spake thusly: > On Mon, Dec 14, 2015 at 11:18:46PM +0100, Yann E. MORIN wrote: > > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > > + bool "selftests" > > > > Since the .mk has: > > SELFTESTS_DEPENDENCIES = bash > > > > then you must either depend on bash or select it here. I think a select > > is better, so: > > > > depends on BR2_USE_MMU # bash > > select BR2_PACKAGE_BASH > > But BR2_PACKAGE_BASH depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS. Gah... I missed that... Thanks for spotting! :-) > Should > BR2_LINUX_KERNEL_TOOL_SELFTESTS also depend on > BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? Yes. With a comment stating so, like: config BR2_LINUX_KERNEL_TOOL_SELFTESTS bool "selftests" depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # bash depends on BR2_USE_MMU # bash select BR2_PACKAGE_BASH comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS (not so nice, but we already have it for openvmtools.) Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-15 17:31 ` Yann E. MORIN @ 2015-12-17 1:25 ` Cyril Bur 0 siblings, 0 replies; 12+ messages in thread From: Cyril Bur @ 2015-12-17 1:25 UTC (permalink / raw) To: buildroot On Tue, 15 Dec 2015 18:31:17 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: Hello Everyone, > Baruch, Cyril, All, > > On 2015-12-15 08:37 +0200, Baruch Siach spake thusly: > > On Mon, Dec 14, 2015 at 11:18:46PM +0100, Yann E. MORIN wrote: > > > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > > > + bool "selftests" > > > > > > Since the .mk has: > > > SELFTESTS_DEPENDENCIES = bash > > > > > > then you must either depend on bash or select it here. I think a select > > > is better, so: > > > > > > depends on BR2_USE_MMU # bash > > > select BR2_PACKAGE_BASH > > > > But BR2_PACKAGE_BASH depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS. > > Gah... I missed that... Thanks for spotting! :-) > Ah I would totally have missed that too, thanks Yann and Baruch! > > Should > > BR2_LINUX_KERNEL_TOOL_SELFTESTS also depend on > > BR2_PACKAGE_BUSYBOX_SHOW_OTHERS? > > Yes. With a comment stating so, like: > > config BR2_LINUX_KERNEL_TOOL_SELFTESTS > bool "selftests" > depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # bash > depends on BR2_USE_MMU # bash > select BR2_PACKAGE_BASH > > comment "selftests needs BR2_PACKAGE_BUSYBOX_SHOW_OTHERS" > depends on !BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > > (not so nice, but we already have it for openvmtools.) > Doesn't matter, I don't think we'll ever get the selftests looking 'nice' anyway but that is an improvement. On the note of dependencies - when I wrote the patch I was in two minds about dependencies. Either this does the bare minimum to get the tests in and if the user wants more tests to work/pass then they'll need to add things, or the selftests depends on as much as possible to give as many tests passing as possible. Based off this and discussion with a colleague, it might be better that ticking the option to just select everything, if you're building the selftests into your image it's because you want to be able to run them. I haven't gone through and determined what the biggest set is but will do and post v2. > Regards, > Yann E. MORIN. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-14 22:18 ` Yann E. MORIN 2015-12-15 6:37 ` Baruch Siach @ 2015-12-17 1:45 ` Cyril Bur 2015-12-17 18:07 ` Yann E. MORIN 1 sibling, 1 reply; 12+ messages in thread From: Cyril Bur @ 2015-12-17 1:45 UTC (permalink / raw) To: buildroot On Mon, 14 Dec 2015 23:18:46 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Cyril, All, > > Sorry for the delay, but I'm now looking at this patch... > Hi Yann, No worries, thanks for getting around to it and thanks for review. > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > This patch simply adds the ability to compile and install the kernel > > selftests into the target. > > > > This is likely to be a rarely used debugging/performance feature for > > development and unlikely to be used in a production configuration. > > > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > > --- > > linux/Config.tools.in | 12 ++++++++++++ > > linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 52 insertions(+) > > create mode 100644 linux/linux-tool-selftests.mk > > > > diff --git a/linux/Config.tools.in b/linux/Config.tools.in > > index 24ef8cd..68655dc 100644 > > --- a/linux/Config.tools.in > > +++ b/linux/Config.tools.in > > @@ -26,4 +26,16 @@ config BR2_LINUX_KERNEL_TOOL_PERF > > > > https://perf.wiki.kernel.org/ > > > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > + bool "selftests" > > Since the .mk has: > SELFTESTS_DEPENDENCIES = bash > > then you must either depend on bash or select it here. I think a select > is better, so: > > depends on BR2_USE_MMU # bash > select BR2_PACKAGE_BASH > > > + help > > + Build and install (to /usr/lib/selftests) kernel selftests. > > Why do you install in there? > I was really quite a bit random, don't have any preferred place myself. > I'm not sure what the kernel selftests are, but two options: > - if they are executables, they should go into /usr/sbin > - if they are libraries, they should go in /usr/lib So I think I initially put them in /bin for testing but its odd because they have quite a directory structure to them, so /bin/selftests/x/y/z felt odd. Theres a runtests script that expects that too otherwise all the binaries *could* be dumped into /usr/sbin. They aren't libraries either though I agree but then its not like the executables really do anything, in a way they are more like a 'library of tests' Happy to defer to you on that one. > > > + Use of this option implies you know the process using and compiling > > + the kernel selftests. The Makefile to build and install these is very > > + noisy and may appear to cause your build to fail for strange reasons. > > + > > + This is very much a use at your risk option and may not work for > > + every setup or every architecture. > > + > > endmenu > > diff --git a/linux/linux-tool-selftests.mk b/linux/linux-tool-selftests.mk > > new file mode 100644 > > index 0000000..573ba0c > > --- /dev/null > > +++ b/linux/linux-tool-selftests.mk > > @@ -0,0 +1,40 @@ > > +################################################################################ > > +# > > +# selftests > > +# > > +################################################################################ > > + > > +LINUX_TOOLS += selftests > > linux-tool infrastructure. Yeah! :-) > I thought doing this was going to be a massive headache and then I found linux-tool infrastructure and it's exactly what I need, long live buildroot! :) > > +ifeq ($(KERNEL_ARCH),x86_64) > > +SELFTESTS_ARCH=x86 > > +else > > +SELFTESTS_ARCH=$(KERNEL_ARCH) > > +endif > > Not related to your patch, but I wonder if we should not make that a > common conditional, so that all tools do not have to repeat it. > > Since there are only two such tool (with selftests) that do it, it can > be put aside for now, and we can revisit it later. > That does make sense, totally agreed. I'll ponder too :) > > +SELFTESTS_DEPENDENCIES = bash > > + > > +SELFTESTS_INSTALL_STAGING = YES > > Why install it in staging? > I actually hadn't initially but I did get to a point in my work where I had to objdump a test binary. So, since I needed it, it stayed. > > +SELFTESTS_MAKE_FLAGS = \ > > + $(LINUX_MAKE_FLAGS) \ > > + ARCH=$(SELFTESTS_ARCH) > > + > > +# O must be redefined here to overwrite the one used by Buildroot for > > +# out of tree build. We build the selftests in $(@D)/tools/selftests and > > +# not just $(@D) so that it isn't built in the root directory of the kernel > > +# sources. > > +define SELFTESTS_BUILD_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) headers_install > > Headers are target dependent, so I think you also need to pass > $(SELFTESTS_MAKE_FLAGS) in there, no? > Quite possible that you're right, this might explain somethings I was seeing. Thanks. > Why are headers needed? > Unfortunately a few of the Makefiles do: CFLAGS += -I../../../../usr/include/ There are other ways around the problem but this was the easiest. > > + $(TARGET_MAKE_ENV) $(MAKE1) $(SELFTESTS_MAKE_FLAGS) \ > > + -C $(@D)/tools/testing/selftests O=$(@D)/tools/testing/selftests > > I prefer when the -C args are passed early (if possible first) because > it is then much obvious where the build is taking place: > > $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \ > $(SELFTESTS_MAKE_FLAGS) \ > O=$(@D)/tools/testing/selftests > Sure, will fix. > > +endef > > + > > +define SELFTESTS_INSTALL_STAGING_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(STAGING_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ > > + -C $(@D)/tools/testing/selftests install > > Line too long, split it to below ~72 chars; also, I prefer we keep the > generic flags early; probably you also have to repeat the O arg: > > $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D)/tools/testing/selftests \ > $(SELFTESTS_MAKE_FLAGS) \ > O=$(@D)/tools/testing/selftests \ > INSTALL_PATH=$(STAGING_DIR)/usr/lib/selftests \ > install > Thanks! > > +endef > > + > > +define SELFTESTS_INSTALL_TARGET_CMDS > > + $(TARGET_MAKE_ENV) $(MAKE1) INSTALL_PATH=$(TARGET_DIR)/usr/lib/selftests $(SELFTESTS_MAKE_FLAGS) \ > > + -C $(@D)/tools/testing/selftests install > > Ditto. > > Regards, > Yann E. MORIN. > Thanks for the review, Cyril. > > +endef > > -- > > 2.6.2 > > > > _______________________________________________ > > buildroot mailing list > > buildroot at busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-17 1:45 ` Cyril Bur @ 2015-12-17 18:07 ` Yann E. MORIN 2015-12-21 5:44 ` Cyril Bur 0 siblings, 1 reply; 12+ messages in thread From: Yann E. MORIN @ 2015-12-17 18:07 UTC (permalink / raw) To: buildroot Cyril, All, On 2015-12-17 12:45 +1100, Cyril Bur spake thusly: > On Mon, 14 Dec 2015 23:18:46 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > > This patch simply adds the ability to compile and install the kernel > > > selftests into the target. > > > > > > This is likely to be a rarely used debugging/performance feature for > > > development and unlikely to be used in a production configuration. > > > > > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > > > --- > > > linux/Config.tools.in | 12 ++++++++++++ > > > linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 52 insertions(+) > > > create mode 100644 linux/linux-tool-selftests.mk > > > > > > diff --git a/linux/Config.tools.in b/linux/Config.tools.in > > > index 24ef8cd..68655dc 100644 > > > --- a/linux/Config.tools.in > > > +++ b/linux/Config.tools.in > > > @@ -26,4 +26,16 @@ config BR2_LINUX_KERNEL_TOOL_PERF > > > > > > https://perf.wiki.kernel.org/ > > > > > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > > + bool "selftests" > > > > Since the .mk has: > > SELFTESTS_DEPENDENCIES = bash > > > > then you must either depend on bash or select it here. I think a select > > is better, so: > > > > depends on BR2_USE_MMU # bash > > select BR2_PACKAGE_BASH > > > > > + help > > > + Build and install (to /usr/lib/selftests) kernel selftests. > > > > Why do you install in there? > > > > I was really quite a bit random, don't have any preferred place myself. > > > I'm not sure what the kernel selftests are, but two options: > > - if they are executables, they should go into /usr/sbin > > - if they are libraries, they should go in /usr/lib > > So I think I initially put them in /bin for testing but its odd because they > have quite a directory structure to them, so /bin/selftests/x/y/z felt odd. > Theres a runtests script that expects that too otherwise all the binaries > *could* be dumped into /usr/sbin. > > They aren't libraries either though I agree but then its not like the > executables really do anything, in a way they are more like a 'library of > tests' > > Happy to defer to you on that one. OK, your explanations is good for me. Still, the directory is not completely to my taste, since it does not refer to the kernel. What about either of: /usr/lib/linux-selftests /usr/lib/kselftests My preference would be for the second, but if you have a better idea, feel free to improvise! ;-) That should probably be explained in the commit log. [--SNIP--] > > > +ifeq ($(KERNEL_ARCH),x86_64) > > > +SELFTESTS_ARCH=x86 > > > +else > > > +SELFTESTS_ARCH=$(KERNEL_ARCH) > > > +endif > > > > Not related to your patch, but I wonder if we should not make that a > > common conditional, so that all tools do not have to repeat it. > > > > Since there are only two such tool (with selftests) that do it, it can > > be put aside for now, and we can revisit it later. > That does make sense, totally agreed. I'll ponder too :) Really, just let that as-is for now. Two packages doing exactly the same think is OK. We try to commonalise stuff when a lot of packages want to do the same thing. 2 is not "a lot". ;-) > > > +SELFTESTS_DEPENDENCIES = bash > > > + > > > +SELFTESTS_INSTALL_STAGING = YES > > > > Why install it in staging? > > I actually hadn't initially but I did get to a point in my work where I had > to objdump a test binary. So, since I needed it, it stayed. But can't you objdump the binaries that are installed in target/ instead? > > > +SELFTESTS_MAKE_FLAGS = \ > > > + $(LINUX_MAKE_FLAGS) \ > > > + ARCH=$(SELFTESTS_ARCH) > > > + > > > +# O must be redefined here to overwrite the one used by Buildroot for > > > +# out of tree build. We build the selftests in $(@D)/tools/selftests and > > > +# not just $(@D) so that it isn't built in the root directory of the kernel > > > +# sources. > > > +define SELFTESTS_BUILD_CMDS > > > + $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) headers_install [--SNIP--] > > Why are headers needed? > > Unfortunately a few of the Makefiles do: > CFLAGS += -I../../../../usr/include/ > > There are other ways around the problem but this was the easiest. OK, your solution is acceptable. But please add a comment just above, tha texplains why this is needed, like so; # Lots of selftests use hard-coded CFLAGS to find the headers # like: CFLAGS += -I../../../../usr/include/ # So we insall them locally *inside* the kernel source tree make blabla headers_install Basically, add a comment whenever what you do, or the reason why you do it, is not obvious. And it also probably warrants some explanations in the commit log as well. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-17 18:07 ` Yann E. MORIN @ 2015-12-21 5:44 ` Cyril Bur 2015-12-21 9:25 ` Yann E. MORIN 0 siblings, 1 reply; 12+ messages in thread From: Cyril Bur @ 2015-12-21 5:44 UTC (permalink / raw) To: buildroot On Thu, 17 Dec 2015 19:07:35 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Cyril, All, > Hi Yann, Totally agreed will address everything you mention. I should have mentioned a few in more detail, my bad for not going over it in more detail before posting, you're correct that a few things in there are actually non obvious. Just one more thing below, > On 2015-12-17 12:45 +1100, Cyril Bur spake thusly: > > On Mon, 14 Dec 2015 23:18:46 +0100 > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > On 2015-11-25 10:53 +1100, Cyril Bur spake thusly: > > > > This patch simply adds the ability to compile and install the kernel > > > > selftests into the target. > > > > > > > > This is likely to be a rarely used debugging/performance feature for > > > > development and unlikely to be used in a production configuration. > > > > > > > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > > > > --- > > > > linux/Config.tools.in | 12 ++++++++++++ > > > > linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 52 insertions(+) > > > > create mode 100644 linux/linux-tool-selftests.mk > > > > > > > > diff --git a/linux/Config.tools.in b/linux/Config.tools.in > > > > index 24ef8cd..68655dc 100644 > > > > --- a/linux/Config.tools.in > > > > +++ b/linux/Config.tools.in > > > > @@ -26,4 +26,16 @@ config BR2_LINUX_KERNEL_TOOL_PERF > > > > > > > > https://perf.wiki.kernel.org/ > > > > > > > > +config BR2_LINUX_KERNEL_TOOL_SELFTESTS > > > > + bool "selftests" > > > > > > Since the .mk has: > > > SELFTESTS_DEPENDENCIES = bash > > > > > > then you must either depend on bash or select it here. I think a select > > > is better, so: > > > > > > depends on BR2_USE_MMU # bash > > > select BR2_PACKAGE_BASH > > > > > > > + help > > > > + Build and install (to /usr/lib/selftests) kernel selftests. > > > > > > Why do you install in there? > > > > > > > I was really quite a bit random, don't have any preferred place myself. > > > > > I'm not sure what the kernel selftests are, but two options: > > > - if they are executables, they should go into /usr/sbin > > > - if they are libraries, they should go in /usr/lib > > > > So I think I initially put them in /bin for testing but its odd because they > > have quite a directory structure to them, so /bin/selftests/x/y/z felt odd. > > Theres a runtests script that expects that too otherwise all the binaries > > *could* be dumped into /usr/sbin. > > > > They aren't libraries either though I agree but then its not like the > > executables really do anything, in a way they are more like a 'library of > > tests' > > > > Happy to defer to you on that one. > > OK, your explanations is good for me. Still, the directory is not > completely to my taste, since it does not refer to the kernel. What > about either of: > /usr/lib/linux-selftests > /usr/lib/kselftests > > My preference would be for the second, but if you have a better idea, > feel free to improvise! ;-) > > That should probably be explained in the commit log. > > [--SNIP--] > > > > +ifeq ($(KERNEL_ARCH),x86_64) > > > > +SELFTESTS_ARCH=x86 > > > > +else > > > > +SELFTESTS_ARCH=$(KERNEL_ARCH) > > > > +endif > > > > > > Not related to your patch, but I wonder if we should not make that a > > > common conditional, so that all tools do not have to repeat it. > > > > > > Since there are only two such tool (with selftests) that do it, it can > > > be put aside for now, and we can revisit it later. > > That does make sense, totally agreed. I'll ponder too :) > > Really, just let that as-is for now. Two packages doing exactly the same > think is OK. We try to commonalise stuff when a lot of packages want to > do the same thing. 2 is not "a lot". ;-) > > > > > +SELFTESTS_DEPENDENCIES = bash > > > > + > > > > +SELFTESTS_INSTALL_STAGING = YES > > > > > > Why install it in staging? > > > > I actually hadn't initially but I did get to a point in my work where I had > > to objdump a test binary. So, since I needed it, it stayed. > > But can't you objdump the binaries that are installed in target/ instead? > I think I tried that initially but IIRC they get stripped when put into target/ which proved annoying when objdumping. Thanks, Cyril > > > > +SELFTESTS_MAKE_FLAGS = \ > > > > + $(LINUX_MAKE_FLAGS) \ > > > > + ARCH=$(SELFTESTS_ARCH) > > > > + > > > > +# O must be redefined here to overwrite the one used by Buildroot for > > > > +# out of tree build. We build the selftests in $(@D)/tools/selftests and > > > > +# not just $(@D) so that it isn't built in the root directory of the kernel > > > > +# sources. > > > > +define SELFTESTS_BUILD_CMDS > > > > + $(TARGET_MAKE_ENV) $(MAKE1) -C $(@D) headers_install > [--SNIP--] > > > Why are headers needed? > > > > Unfortunately a few of the Makefiles do: > > CFLAGS += -I../../../../usr/include/ > > > > There are other ways around the problem but this was the easiest. > > OK, your solution is acceptable. But please add a comment just above, > tha texplains why this is needed, like so; > > # Lots of selftests use hard-coded CFLAGS to find the headers > # like: CFLAGS += -I../../../../usr/include/ > # So we insall them locally *inside* the kernel source tree > make blabla headers_install > > Basically, add a comment whenever what you do, or the reason why you do > it, is not obvious. And it also probably warrants some explanations in > the commit log as well. > > Regards, > Yann E. MORIN. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-21 5:44 ` Cyril Bur @ 2015-12-21 9:25 ` Yann E. MORIN 2015-12-21 22:18 ` Cyril Bur 0 siblings, 1 reply; 12+ messages in thread From: Yann E. MORIN @ 2015-12-21 9:25 UTC (permalink / raw) To: buildroot Cyril, All, On 2015-12-21 16:44 +1100, Cyril Bur spake thusly: > On Thu, 17 Dec 2015 19:07:35 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Totally agreed will address everything you mention. I should have mentioned a > few in more detail, my bad for not going over it in more detail before posting, > you're correct that a few things in there are actually non obvious. No problem! Things just happen to be obvious when you actually do the job, because everything is fresh in your memory. Patch reviews are there to eventually highlight that fact. So, really, that's not a problem. [--SNIP--] > > > > > +SELFTESTS_INSTALL_STAGING = YES > > > > Why install it in staging? > > > I actually hadn't initially but I did get to a point in my work where I had > > > to objdump a test binary. So, since I needed it, it stayed. > > But can't you objdump the binaries that are installed in target/ instead? > > I think I tried that initially but IIRC they get stripped when put into target/ > which proved annoying when objdumping. But then, the 'original' binaries are still present in the build directory, where they are definitely not stripped (at least, not by Builroot). Isn't that enough? Staging is really for libraries and headers that are to be used by another package. Kernel selftests are not headers, they are not libraries, so nothing will include/link them. So, I still believe there's no reason to put them in staging. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-21 9:25 ` Yann E. MORIN @ 2015-12-21 22:18 ` Cyril Bur 2015-12-21 22:25 ` Yann E. MORIN 0 siblings, 1 reply; 12+ messages in thread From: Cyril Bur @ 2015-12-21 22:18 UTC (permalink / raw) To: buildroot On Mon, 21 Dec 2015 10:25:13 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Cyril, All, > > On 2015-12-21 16:44 +1100, Cyril Bur spake thusly: > > On Thu, 17 Dec 2015 19:07:35 +0100 > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > Totally agreed will address everything you mention. I should have mentioned a > > few in more detail, my bad for not going over it in more detail before posting, > > you're correct that a few things in there are actually non obvious. > > No problem! Things just happen to be obvious when you actually do the > job, because everything is fresh in your memory. Patch reviews are there > to eventually highlight that fact. So, really, that's not a problem. > > [--SNIP--] > > > > > > +SELFTESTS_INSTALL_STAGING = YES > > > > > Why install it in staging? > > > > I actually hadn't initially but I did get to a point in my work where I had > > > > to objdump a test binary. So, since I needed it, it stayed. > > > But can't you objdump the binaries that are installed in target/ instead? > > > > I think I tried that initially but IIRC they get stripped when put into target/ > > which proved annoying when objdumping. > > But then, the 'original' binaries are still present in the build > directory, where they are definitely not stripped (at least, not by > Builroot). Isn't that enough? > Doh, you're right of course they'll be in the linux build dir, I'll change that too - I'll be sure to comment that one, even I totally forgot that haha. Thanks > Staging is really for libraries and headers that are to be used by > another package. Kernel selftests are not headers, they are not > libraries, so nothing will include/link them. So, I still believe > there's no reason to put them in staging. > > Regards, > Yann E. MORIN. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Buildroot] [PATCH] linux: Build and install kernel selftests 2015-12-21 22:18 ` Cyril Bur @ 2015-12-21 22:25 ` Yann E. MORIN 0 siblings, 0 replies; 12+ messages in thread From: Yann E. MORIN @ 2015-12-21 22:25 UTC (permalink / raw) To: buildroot Cyril, All, On 2015-12-22 09:18 +1100, Cyril Bur spake thusly: > On Mon, 21 Dec 2015 10:25:13 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: [--SNIP--] > > But then, the 'original' binaries are still present in the build > > directory, where they are definitely not stripped (at least, not by > > Builroot). Isn't that enough? > Doh, you're right of course they'll be in the linux build dir, I'll change that > too - I'll be sure to comment that one, even I totally forgot that haha. If you were planning on adding a comment like "if you need to objdump the binaries, they are still present unstripped in the build directory" then you should not: it's the case for all and every packages; there's nothing special about this for the kselftests. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-21 22:25 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-24 23:53 [Buildroot] [PATCH] Add Linux kernel selftests building Cyril Bur 2015-11-24 23:53 ` [Buildroot] [PATCH] linux: Build and install kernel selftests Cyril Bur 2015-12-14 22:18 ` Yann E. MORIN 2015-12-15 6:37 ` Baruch Siach 2015-12-15 17:31 ` Yann E. MORIN 2015-12-17 1:25 ` Cyril Bur 2015-12-17 1:45 ` Cyril Bur 2015-12-17 18:07 ` Yann E. MORIN 2015-12-21 5:44 ` Cyril Bur 2015-12-21 9:25 ` Yann E. MORIN 2015-12-21 22:18 ` Cyril Bur 2015-12-21 22:25 ` Yann E. MORIN
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.