Skip to contentSkip to conversation

pkg-kernel-module: Die when kernel module are disabled

PATCH New Noé Rubinstein 4 participants 7 messages Aug 17, 2015 – Aug 21, 2015

lore
Share & tools
lore

Noé Rubinstein

author
#0 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Test the config of the kernel to see if loadable module support is enabled, and error out otherwise. This makes the build failure less confusing.

Signed-off-by No? Rubinstein <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>
---
 package/pkg-kernel-module.mk | 4 ++++
 1 file changed, 4 insertions(+)
package/pkg-kernel-module.mk +4 -0
index 5fb19be..6fb7704 100644
@@ -60,6 +60,10 @@ $(2)_MODULE_SUBDIRS ?= .
 # includes and other support files (Booo!)
 define $(2)_KERNEL_MODULES_BUILD
 	@$$(call MESSAGE,"Building kernel module(s)")
+	@if ! grep -Fqx 'CONFIG_MODULES=y' $(LINUX_DIR)/.config; then \
+		echo "ERROR: Kernel does not support loadable modules"; \
+		exit 1; \
+	fi
 	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
 		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
 			-C $$(LINUX_DIR) \
Signature · 1 line
2.1.4
#1 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Dear No? Rubinstein,

Quoted 9 lines · No? Rubinstein

Test the config of the kernel to see if loadable module support is enabled, and error out otherwise. This makes the build failure less confusing.

Signed-off-by No? Rubinstein <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>
---
 package/pkg-kernel-module.mk | 4 ++++
 1 file changed, 4 insertions(+)

Applied after tweaking the commit log, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Dear No? Rubinstein,

Quoted 4 lines · No? Rubinstein

Test the config of the kernel to see if loadable module support is enabled, and error out otherwise. This makes the build failure less confusing.

Quoted 7 lines
Quoted 5 lines
Signed-off-by No? Rubinstein <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>

---
package/pkg-kernel-module.mk | 4 ++++

1 file changed, 4 insertions(+)

Applied after tweaking the commit log, thanks.

Couldn't we instead just force modules support on in the kernel if the pkg-kernel-module infrastructure is used?

Signature · 1 line
Bye, Peter Korsgaard
#3 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Hello,

Quoted 14 lines · Peter Korsgaard
Quoted 13 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Dear No? Rubinstein,

Quoted 9 lines · No? Rubinstein

Test the config of the kernel to see if loadable module support is enabled, and error out otherwise. This makes the build failure less confusing.

Signed-off-by No? Rubinstein <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>

---
package/pkg-kernel-module.mk | 4 ++++

1 file changed, 4 insertions(+)
Quoted 4 lines
Quoted 1 line

Applied after tweaking the commit log, thanks.

Couldn't we instead just force modules support on in the kernel if the pkg-kernel-module infrastructure is used?

That's another option, yes. No??

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

In reply to Peter Korsgaard

Peter, All,

On 2015-08-18 07:41 +0200, Peter Korsgaard spake thusly:

Quoted 14 lines
Quoted 13 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Dear No? Rubinstein,

Quoted 9 lines · No? Rubinstein

Test the config of the kernel to see if loadable module support is enabled, and error out otherwise. This makes the build failure less confusing.

Signed-off-by No? Rubinstein <[email protected]>
Acked-by "Yann E. MORIN" <[email protected]>

---
package/pkg-kernel-module.mk | 4 ++++

1 file changed, 4 insertions(+)
Quoted 4 lines
Quoted 1 line

Applied after tweaking the commit log, thanks.

Couldn't we instead just force modules support on in the kernel if the pkg-kernel-module infrastructure is used?

And how do you suggest we test whether the kernel-module infra is used?

1) it *is* used already: we do have packages that do call $(eval $(kernel-module)) , but I guess that's not what you meant. ;-)

2) so you probably meant: if we actually need to build a kernel module.

But that's not so easy:

- we need to know whether to enable moduls when parsing linux/linux.mk

- external packages may call kernel-module

- external packages are parsed after linux.mk

So, too late...

We can't enable kernel modules just from the .mk, we can only check for them.

Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that packages may select if they want to build modules. Of a visible knob (user-selectable) that packages can depend on if they need to build kernel modules...

Needless to say, for 2015.08, No?'s patch is good to go in, while the alternatives are not; they can however be done post-2015.08.

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.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 14 lines
Quoted 2 lines

Couldn't we instead just force modules support on in the kernel if the pkg-kernel-module infrastructure is used?

And how do you suggest we test whether the kernel-module infra is used?

1) it *is* used already: we do have packages that do call $(eval $(kernel-module)) , but I guess that's not what you meant. ;-)

2) so you probably meant: if we actually need to build a kernel module.

But that's not so easy:

Quoted 7 lines

- we need to know whether to enable moduls when parsing linux/linux.mk

- external packages may call kernel-module

- external packages are parsed after linux.mk

So, too late...

If we can come up with a solution that only works for internal packages then that is imho still better than what we have today. We can still leave the check in the pkg-kernel-module infrastructure to atleast warn BR2_EXTERNAL users.

Quoted 7 lines

We can't enable kernel modules just from the .mk, we can only check for them.

Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that packages may select if they want to build modules. Of a visible knob (user-selectable) that packages can depend on if they need to build kernel modules...

Or the pkg-kernel-module infrastructure can just set a variable that gets checked by LINUX_KCONFIG_FIXUP_CMDS, E.G. something like:

package/pkg-kernel-module.mk +4 -0
index 6fb7704..436a721 100644
@@ -91,6 +91,10 @@ define $(2)_KERNEL_MODULES_INSTALL
 endef
 $(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
 
+ifeq ($(BR2_PACKAGE_$(2)),y)
+LINUX_NEEDS_MODULES = YES
+endif
+
 endef
 
 ################################################################################

And then:

linux/linux.mk +2 -0
index d91dbb2..eb7ce7e 100644
@@ -219,6 +219,8 @@ define LINUX_KCONFIG_FIXUP_CMDS
                $(call KCONFIG_ENABLE_OPT,CONFIG_NF_CONNTRACK_MARK,$(@D)/.config))
        $(if $(BR2_LINUX_KERNEL_APPENDED_DTB),
                $(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config))
+       $(if $(LINUX_NEEDS_MODULES),
+               $(call KCONFIG_ENABLE_OPT,CONFIG_MODULES,$(@D)/.config))
 endef
 
 ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)
Quoted 2 lines

Needless to say, for 2015.08, No?'s patch is good to go in, while the alternatives are not; they can however be done post-2015.08.

Agreed, this is not a new regression.

Signature · 2 lines
Venlig hilsen,
Peter Korsgaard
#6 lore raw

PATCH v2 1/1 pkg-kernel-module: Die when kernel module are disabled

Peter, All,

On 2015-08-18 12:41 +0200, Peter Korsgaard spake thusly:

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

[--SNIP--]

Quoted 7 lines
Quoted 4 lines

Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that packages may select if they want to build modules. Of a visible knob (user-selectable) that packages can depend on if they need to build kernel modules...

Or the pkg-kernel-module infrastructure can just set a variable that gets checked by LINUX_KCONFIG_FIXUP_CMDS, E.G. something like:

OK, I don;t know why I missed that mail of yours (except it is not in the same thread we were "battling" in).

Quoted 24 lines
package/pkg-kernel-module.mk +4 -0
index 6fb7704..436a721 100644
@@ -91,6 +91,10 @@ define $(2)_KERNEL_MODULES_INSTALL
 endef
 $(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
 
+ifeq ($(BR2_PACKAGE_$(2)),y)
+LINUX_NEEDS_MODULES = YES
+endif
+
 endef
 
 ################################################################################

And then:

linux/linux.mk +0 -0
index d91dbb2..eb7ce7e 100644

linux/linux.mk define LINUX_KCONFIG_FIXUP_CMDS · line 226

Quoted 8 lines
linux/linux.mk +2 -0
@@ -219,6 +219,8 @@ define LINUX_KCONFIG_FIXUP_CMDS
                $(call KCONFIG_ENABLE_OPT,CONFIG_NF_CONNTRACK_MARK,$(@D)/.config))
        $(if $(BR2_LINUX_KERNEL_APPENDED_DTB),
                $(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config))
+       $(if $(LINUX_NEEDS_MODULES),
+               $(call KCONFIG_ENABLE_OPT,CONFIG_MODULES,$(@D)/.config))
 endef
 
 ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)

Yes, that should work.

Of course, you are right, we'd only use that variable after all .mk are parsed, being in the kconfig-fixup commands.

I'll see to implement and test that.

Thanks! :-)

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.  |
'------------------------------^-------^------------------^--------------------'