Skip to contentSkip to conversation

Add Linux kernel selftests building

PATCH New Cyril Bur 3 participants 12 messages Nov 24, 2015 – Dec 21, 2015

lore
Share & tools
lore

Cyril Bur

author
#0 lore raw

PATCH 1/1 Add Linux kernel selftests building

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
Signature · 1 line
2.6.2
author
#1 lore raw

PATCH 1/1 linux: Build and install kernel selftests

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 <[email protected]>
---
 linux/Config.tools.in         | 12 ++++++++++++
 linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 linux/linux-tool-selftests.mk
linux/Config.tools.in +12 -0
index 24ef8cd..68655dc 100644
@@ -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
linux/linux-tool-selftests.mk new file +40 -0
new file mode 100644
index 0000000..573ba0c
@@ -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
Signature · 1 line
2.6.2
#2 lore raw

PATCH 1/1 linux: Build and install kernel selftests

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:

Quoted 18 lines

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 <[email protected]>
---
 linux/Config.tools.in         | 12 ++++++++++++
 linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 linux/linux-tool-selftests.mk
linux/Config.tools.in +0 -0
index 24ef8cd..68655dc 100644

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 5 lines
linux/Config.tools.in +2 -0
@@ -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

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 32

Quoted 2 lines
linux/Config.tools.in +2 -0
+	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

Quoted 14 lines
linux/Config.tools.in +7 -0
+	  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
linux/linux-tool-selftests.mk new file +0 -0
new file mode 100644
index 0000000..573ba0c

linux/linux-tool-selftests.mk line 7

Quoted 7 lines
linux/linux-tool-selftests.mk new file +7 -0
@@ -0,0 +1,40 @@
+################################################################################
+#
+# selftests
+#
+################################################################################
+
+LINUX_TOOLS += selftests

linux-tool infrastructure. Yeah! :-)

linux/linux-tool-selftests.mk line 13

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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.

linux/linux-tool-selftests.mk line 17

Quoted 3 lines
linux/linux-tool-selftests.mk new file +3 -0
+SELFTESTS_DEPENDENCIES = bash
+
+SELFTESTS_INSTALL_STAGING = YES

Why install it in staging?

linux/linux-tool-selftests.mk line 27

Quoted 10 lines
linux/linux-tool-selftests.mk new file +10 -0
+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?

linux/linux-tool-selftests.mk line 29

Quoted 2 lines
linux/linux-tool-selftests.mk new file +2 -0
+	$(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

linux/linux-tool-selftests.mk line 34

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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

linux/linux-tool-selftests.mk line 39

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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.

Quoted 8 lines
linux/linux-tool-selftests.mk new file +1 -0
+endef
Signature · 6 lines
2.6.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Hi Yann,

Quoted 7 lines · Yann E. MORIN

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 2 lines
linux/Config.tools.in +2 -0
+config BR2_LINUX_KERNEL_TOOL_SELFTESTS
+	bool "selftests"

Since the .mk has:
SELFTESTS_DEPENDENCIES = bash

Quoted 5 lines

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

Signature · 3 lines
     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 -
#4 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Baruch, Cyril, All,

On 2015-12-15 08:37 +0200, Baruch Siach spake thusly:

Quoted 8 lines · Yann E. MORIN
Quoted 7 lines

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 2 lines
linux/Config.tools.in +2 -0
+config BR2_LINUX_KERNEL_TOOL_SELFTESTS
+	bool "selftests"

Since the .mk has:
SELFTESTS_DEPENDENCIES = bash

Quoted 7 lines
Quoted 5 lines

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! :-)

Quoted 3 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#5 lore raw

PATCH 1/1 linux: Build and install kernel selftests

On Tue, 15 Dec 2015 18:31:17 +0100
"Yann E. MORIN" <[email protected]> wrote:

Hello Everyone,

Quoted 14 lines

Baruch, Cyril, All,

On 2015-12-15 08:37 +0200, Baruch Siach spake thusly:

Quoted 11 lines · Yann E. MORIN
Quoted 10 lines

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 2 lines
linux/Config.tools.in +2 -0
+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:

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

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!

Quoted 12 lines
Quoted 3 lines

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

Quoted 5 lines

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.

Quoted 3 lines

Regards,
Yann E. MORIN.

author
#6 lore raw

PATCH 1/1 linux: Build and install kernel selftests

In reply to Yann E. MORIN
Quoted 4 lines · On Mon, 14 Dec 2015 23:18:46 +0100 "Yann E. MORIN" <[email protected]>…

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.

Quoted 31 lines

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

Quoted 18 lines

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 <[email protected]>
---
 linux/Config.tools.in         | 12 ++++++++++++
 linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 linux/linux-tool-selftests.mk
linux/Config.tools.in +0 -0
index 24ef8cd..68655dc 100644

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 5 lines
linux/Config.tools.in +2 -0
@@ -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:

Quoted 8 lines

depends on BR2_USE_MMU # bash
select BR2_PACKAGE_BASH

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 32

Quoted 2 lines
linux/Config.tools.in +2 -0
+	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.

Quoted 3 lines

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.

Quoted 16 lines
Quoted 16 lines
linux/Config.tools.in +7 -0
+	  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
linux/linux-tool-selftests.mk new file +1 -0
new file mode 100644
index 0000000..573ba0c
@@ -0,0 +1,40 @@
+################################################################################
Quoted 9 lines

linux/linux-tool-selftests.mk line 7

Quoted 6 lines
linux/linux-tool-selftests.mk new file +6 -0
+#
+# 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! :)

Quoted 9 lines

linux/linux-tool-selftests.mk line 13

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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.

Quoted 3 lines

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 :)

Quoted 6 lines

linux/linux-tool-selftests.mk line 17

Quoted 3 lines
linux/linux-tool-selftests.mk new file +3 -0
+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.

Quoted 5 lines
Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+SELFTESTS_MAKE_FLAGS = \
+	$(LINUX_MAKE_FLAGS) \
+	ARCH=$(SELFTESTS_ARCH)
+
+# O must be redefined here to overwrite the one used by Buildroot for
Quoted 9 lines

linux/linux-tool-selftests.mk line 27

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+# 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.

Quoted 2 lines

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.

Quoted 10 lines

linux/linux-tool-selftests.mk line 29

Quoted 2 lines
linux/linux-tool-selftests.mk new file +2 -0
+	$(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.

Quoted 9 lines

linux/linux-tool-selftests.mk line 34

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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:

Quoted 6 lines
    $(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!

Quoted 11 lines

linux/linux-tool-selftests.mk line 39

Quoted 5 lines
linux/linux-tool-selftests.mk new file +5 -0
+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.

Quoted 9 lines
Quoted 8 lines
linux/linux-tool-selftests.mk new file +1 -0
+endef
Signature · 6 lines
2.6.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
#7 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Cyril, All,

On 2015-12-17 12:45 +1100, Cyril Bur spake thusly:

Quoted 53 lines
Quoted 31 lines · On Mon, 14 Dec 2015 23:18:46 +0100 "Yann E. MORIN" <[email protected]>…

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

Quoted 18 lines

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 <[email protected]>
---
 linux/Config.tools.in         | 12 ++++++++++++
 linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 linux/linux-tool-selftests.mk
linux/Config.tools.in +0 -0
index 24ef8cd..68655dc 100644

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 5 lines
linux/Config.tools.in +2 -0
@@ -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:

Quoted 8 lines

depends on BR2_USE_MMU # bash
select BR2_PACKAGE_BASH

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 32

Quoted 2 lines
linux/Config.tools.in +2 -0
+	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.

Quoted 3 lines

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.

Quoted 5 lines

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--]

Quoted 9 lines
Quoted 9 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 37

Quoted 5 lines
linux/Config.tools.in +5 -0
+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.

Quoted 3 lines
Quoted 2 lines

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". ;-)

Quoted 8 lines
Quoted 5 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 40

Quoted 3 lines
linux/Config.tools.in +3 -0
+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?

Quoted 10 lines
Quoted 10 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF

Quoted 10 lines
linux/Config.tools.in +10 -0
+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--]

Quoted 6 lines
Quoted 1 line

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#8 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Quoted 2 lines · On Thu, 17 Dec 2015 19:07:35 +0100 "Yann E. MORIN" <[email protected]>…

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,

Quoted 95 lines

On 2015-12-17 12:45 +1100, Cyril Bur spake thusly:

Quoted 53 lines
Quoted 31 lines · On Mon, 14 Dec 2015 23:18:46 +0100 "Yann E. MORIN" <[email protected]>…

On 2015-11-25 10:53 +1100, Cyril Bur spake thusly:

Quoted 18 lines

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 <[email protected]>
---
 linux/Config.tools.in         | 12 ++++++++++++
 linux/linux-tool-selftests.mk | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 linux/linux-tool-selftests.mk
linux/Config.tools.in +0 -0
index 24ef8cd..68655dc 100644

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 30

Quoted 5 lines
linux/Config.tools.in +2 -0
@@ -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:

Quoted 8 lines

depends on BR2_USE_MMU # bash
select BR2_PACKAGE_BASH

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 32

Quoted 2 lines
linux/Config.tools.in +2 -0
+	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.

Quoted 3 lines

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.

Quoted 5 lines

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--]

Quoted 9 lines
Quoted 9 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 37

Quoted 5 lines
linux/Config.tools.in +5 -0
+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.

Quoted 3 lines
Quoted 2 lines

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". ;-)

Quoted 6 lines
Quoted 5 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF · line 40

Quoted 3 lines
linux/Config.tools.in +3 -0
+SELFTESTS_DEPENDENCIES = bash
+
+SELFTESTS_INSTALL_STAGING = YES

Why install it in staging?

Quoted 5 lines
Quoted 2 lines

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

Quoted 33 lines
Quoted 10 lines
Quoted 10 lines

linux/Config.tools.in config BR2_LINUX_KERNEL_TOOL_PERF

Quoted 10 lines
linux/Config.tools.in +10 -0
+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--]

Quoted 6 lines
Quoted 1 line

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.

#9 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Cyril, All,

On 2015-12-21 16:44 +1100, Cyril Bur spake thusly:

Quoted 5 lines

On Thu, 17 Dec 2015 19:07:35 +0100
"Yann E. MORIN" <[email protected]> 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--]

Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Quoted 1 line

+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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#10 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Quoted 18 lines · On Mon, 21 Dec 2015 10:25:13 +0100 "Yann E. MORIN" <[email protected]>…

Cyril, All,

On 2015-12-21 16:44 +1100, Cyril Bur spake thusly:

Quoted 5 lines

On Thu, 17 Dec 2015 19:07:35 +0100
"Yann E. MORIN" <[email protected]> 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--]

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 1 line

+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.

Quoted 9 lines
Quoted 4 lines
Quoted 1 line

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

Quoted 8 lines

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.

#11 lore raw

PATCH 1/1 linux: Build and install kernel selftests

Cyril, All,

On 2015-12-22 09:18 +1100, Cyril Bur spake thusly:

Quoted 2 lines

On Mon, 21 Dec 2015 10:25:13 +0100
"Yann E. MORIN" <[email protected]> wrote:

[--SNIP--]

Quoted 5 lines
Quoted 3 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'