diff mbox series

linux-module: make sure out-of-tree modules win over built-in

Message ID 20220825204052.9151-1-henning.schild@siemens.com
State Superseded, archived
Headers show
Series linux-module: make sure out-of-tree modules win over built-in | expand

Commit Message

Henning Schild Aug. 25, 2022, 12:40 p.m. UTC 
If we built an out of tree module that happens to be already part of the
kernel, we want to make sure depmod prefers the external one. In order
to do so simply include a config file into every module package, by
default using the kernels default INSTALL_MOD_DIR which is "extra".

Should there be multiple modules using different such directories, all
recipes should set INSTALL_MOD_DIR listing them all in the desired
order. That interface for multiple such very special modules is not nice
but since they are pretty special it might be just good enough.

Reported-by: Pingfang Liao <Pingfang.Liao@siemens.com>
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 1 +
 meta/recipes-kernel/linux-module/module.inc              | 3 +++
 2 files changed, 4 insertions(+)

Comments

Florian Bezdeka Aug. 26, 2022, 3:34 a.m. UTC | #1 
On Thu, 2022-08-25 at 22:40 +0200, Henning Schild wrote:
> If we built an out of tree module that happens to be already part of the
> kernel, 
> 

This hopefully does not happen in the wild and is likely a very special
situation we're facing internally while working on upstreaming some
driver stuff.

I'm not sure if this is really relevant for Isar itself. If the
decision is "let's take it", I would vote for "disabled per default".

> we want to make sure depmod prefers the external one. In order
> to do so simply include a config file into every module package, by
> default using the kernels default INSTALL_MOD_DIR which is "extra".
> 
> Should there be multiple modules using different such directories, all
> recipes should set INSTALL_MOD_DIR listing them all in the desired
> order. That interface for multiple such very special modules is not nice
> but since they are pretty special it might be just good enough.
> 
> Reported-by: Pingfang Liao <Pingfang.Liao@siemens.com>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 1 +
>  meta/recipes-kernel/linux-module/module.inc              | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> index d3bd7dc30f21..abd3c88fac7e 100755
> --- a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> +++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> @@ -55,6 +55,7 @@ override_dh_auto_build:
>  
>  override_dh_auto_install:
>  	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
> +	install -D -m 0644 $(PWD)/debian/${PN}.conf $(PWD)/debian/${PN}/usr/lib/depmod.d/${PN}.conf
>  
>  %:
>  	CFLAGS= LDFLAGS= dh $@ --parallel
> diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
> index b20ec32ae20a..475e7aef05ea 100644
> --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -29,9 +29,12 @@ TEMPLATE_FILES = "debian/control.tmpl \
>                    debian/rules.tmpl"
>  TEMPLATE_VARS += "KERNEL_NAME KERNEL_TYPE KERNEL_IMAGE_PKG KERNEL_HEADERS_PKG DEBIAN_BUILD_DEPENDS PN"
>  
> +INSTALL_MOD_DIR ?= "extra"
> +
>  do_prepare_build() {
>      cp -r ${WORKDIR}/debian ${S}/
>  
> +    echo "search ${INSTALL_MOD_DIR} built-in" > ${S}/debian/${PN}.conf
>      for module in ${AUTOLOAD}; do
>          echo "echo $module >> /etc/modules" >> ${S}/debian/postinst
>      done
> -- 
> 2.35.1
>
Henning Schild Aug. 26, 2022, 4:32 a.m. UTC | #2 
Am Fri, 26 Aug 2022 13:34:22 +0200
schrieb "Bezdeka, Florian (T CED SES-DE)" <florian.bezdeka@siemens.com>:

> On Thu, 2022-08-25 at 22:40 +0200, Henning Schild wrote:
> > If we built an out of tree module that happens to be already part
> > of the kernel, 
> >   
> 
> This hopefully does not happen in the wild and is likely a very
> special situation we're facing internally while working on
> upstreaming some driver stuff.

That period is going to be very long, like years. And it is rare but
valid. Any board could require a slightly modified version of a driver
already in the kernel, and if it is just a backport of something new
not yet in your old kernel.

While the conflict could also be solved with recompiling that kernel,
with the conflicting driver disabled, that would be kind of a big deal
to solve such a small issue.

> I'm not sure if this is really relevant for Isar itself. If the
> decision is "let's take it", I would vote for "disabled per default".

I think it is relevant, that is why i propose it.

But yes, we could maybe not say "INSTALL_MOD_DIR ?= "extra"" but maybe
allow for an array of "EXTRA_FILES". That would also allow to ship
things like /etc/modprobe.d/ and /etc/udev.d/.

Or we could make the pattern that such EXTRA_FILES would have to come
from another BSP package which the module package itself would depend
on. All there would be to do is to write that extra package and allow
the modification of "Depends:" of an Isar built module package.

I think i might give that last approach a try, all Isar would need to
do is allow that Depends to change. Might feel a little obfuscated in
the end, especially if others might want to do similar things. But let
us see.

Henning

> > we want to make sure depmod prefers the external one. In order
> > to do so simply include a config file into every module package, by
> > default using the kernels default INSTALL_MOD_DIR which is "extra".
> > 
> > Should there be multiple modules using different such directories,
> > all recipes should set INSTALL_MOD_DIR listing them all in the
> > desired order. That interface for multiple such very special
> > modules is not nice but since they are pretty special it might be
> > just good enough.
> > 
> > Reported-by: Pingfang Liao <Pingfang.Liao@siemens.com>
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 1 +
> >  meta/recipes-kernel/linux-module/module.inc              | 3 +++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git
> > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl index
> > d3bd7dc30f21..abd3c88fac7e 100755 ---
> > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl +++
> > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl @@ -55,6
> > +55,7 @@ override_dh_auto_build: override_dh_auto_install:
> >  	$(MAKE) -C $(KDIR) M=$(PWD)
> > INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
> > +	install -D -m 0644 $(PWD)/debian/${PN}.conf
> > $(PWD)/debian/${PN}/usr/lib/depmod.d/${PN}.conf 
> >  %:
> >  	CFLAGS= LDFLAGS= dh $@ --parallel
> > diff --git a/meta/recipes-kernel/linux-module/module.inc
> > b/meta/recipes-kernel/linux-module/module.inc index
> > b20ec32ae20a..475e7aef05ea 100644 ---
> > a/meta/recipes-kernel/linux-module/module.inc +++
> > b/meta/recipes-kernel/linux-module/module.inc @@ -29,9 +29,12 @@
> > TEMPLATE_FILES = "debian/control.tmpl \ debian/rules.tmpl"
> >  TEMPLATE_VARS += "KERNEL_NAME KERNEL_TYPE KERNEL_IMAGE_PKG
> > KERNEL_HEADERS_PKG DEBIAN_BUILD_DEPENDS PN" 
> > +INSTALL_MOD_DIR ?= "extra"
> > +
> >  do_prepare_build() {
> >      cp -r ${WORKDIR}/debian ${S}/
> >  
> > +    echo "search ${INSTALL_MOD_DIR} built-in" >
> > ${S}/debian/${PN}.conf for module in ${AUTOLOAD}; do
> >          echo "echo $module >> /etc/modules" >> ${S}/debian/postinst
> >      done
> > -- 
> > 2.35.1
> >   
>
Henning Schild Aug. 26, 2022, 4:53 a.m. UTC | #3 
Am Fri, 26 Aug 2022 14:32:15 +0200
schrieb Henning Schild <henning.schild@siemens.com>:

> Am Fri, 26 Aug 2022 13:34:22 +0200
> schrieb "Bezdeka, Florian (T CED SES-DE)"
> <florian.bezdeka@siemens.com>:
> 
> > On Thu, 2022-08-25 at 22:40 +0200, Henning Schild wrote:  
> > > If we built an out of tree module that happens to be already part
> > > of the kernel, 
> > >     
> > 
> > This hopefully does not happen in the wild and is likely a very
> > special situation we're facing internally while working on
> > upstreaming some driver stuff.  
> 
> That period is going to be very long, like years. And it is rare but
> valid. Any board could require a slightly modified version of a driver
> already in the kernel, and if it is just a backport of something new
> not yet in your old kernel.
> 
> While the conflict could also be solved with recompiling that kernel,
> with the conflicting driver disabled, that would be kind of a big deal
> to solve such a small issue.
> 
> > I'm not sure if this is really relevant for Isar itself. If the
> > decision is "let's take it", I would vote for "disabled per
> > default".  
> 
> I think it is relevant, that is why i propose it.
> 
> But yes, we could maybe not say "INSTALL_MOD_DIR ?= "extra"" but maybe
> allow for an array of "EXTRA_FILES". That would also allow to ship
> things like /etc/modprobe.d/ and /etc/udev.d/.
> 
> Or we could make the pattern that such EXTRA_FILES would have to come
> from another BSP package which the module package itself would depend
> on. All there would be to do is to write that extra package and allow
> the modification of "Depends:" of an Isar built module package.
> 
> I think i might give that last approach a try, all Isar would need to
> do is allow that Depends to change. Might feel a little obfuscated in
> the end, especially if others might want to do similar things. But let
> us see.

Likely one could also just patch the modules sources to ship those
files on "make modules_install" and they would just like that become
part of the package.
Feels kind of hacky since "make install" would seem the better place,
but that does not exist in your average external module. I will play
with that idea, since it would allow that out-of-tree module to accept
a patch that would be useful for anyone using it, Isar or not.

Henning

> Henning
> 
> > > we want to make sure depmod prefers the external one. In order
> > > to do so simply include a config file into every module package,
> > > by default using the kernels default INSTALL_MOD_DIR which is
> > > "extra".
> > > 
> > > Should there be multiple modules using different such directories,
> > > all recipes should set INSTALL_MOD_DIR listing them all in the
> > > desired order. That interface for multiple such very special
> > > modules is not nice but since they are pretty special it might be
> > > just good enough.
> > > 
> > > Reported-by: Pingfang Liao <Pingfang.Liao@siemens.com>
> > > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > > ---
> > >  meta/recipes-kernel/linux-module/files/debian/rules.tmpl | 1 +
> > >  meta/recipes-kernel/linux-module/module.inc              | 3 +++
> > >  2 files changed, 4 insertions(+)
> > > 
> > > diff --git
> > > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
> > > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl index
> > > d3bd7dc30f21..abd3c88fac7e 100755 ---
> > > a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl +++
> > > b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl @@
> > > -55,6 +55,7 @@ override_dh_auto_build: override_dh_auto_install:
> > >  	$(MAKE) -C $(KDIR) M=$(PWD)
> > > INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
> > > +	install -D -m 0644 $(PWD)/debian/${PN}.conf
> > > $(PWD)/debian/${PN}/usr/lib/depmod.d/${PN}.conf 
> > >  %:
> > >  	CFLAGS= LDFLAGS= dh $@ --parallel
> > > diff --git a/meta/recipes-kernel/linux-module/module.inc
> > > b/meta/recipes-kernel/linux-module/module.inc index
> > > b20ec32ae20a..475e7aef05ea 100644 ---
> > > a/meta/recipes-kernel/linux-module/module.inc +++
> > > b/meta/recipes-kernel/linux-module/module.inc @@ -29,9 +29,12 @@
> > > TEMPLATE_FILES = "debian/control.tmpl \ debian/rules.tmpl"
> > >  TEMPLATE_VARS += "KERNEL_NAME KERNEL_TYPE KERNEL_IMAGE_PKG
> > > KERNEL_HEADERS_PKG DEBIAN_BUILD_DEPENDS PN" 
> > > +INSTALL_MOD_DIR ?= "extra"
> > > +
> > >  do_prepare_build() {
> > >      cp -r ${WORKDIR}/debian ${S}/
> > >  
> > > +    echo "search ${INSTALL_MOD_DIR} built-in" >
> > > ${S}/debian/${PN}.conf for module in ${AUTOLOAD}; do
> > >          echo "echo $module >> /etc/modules" >>
> > > ${S}/debian/postinst done
> > > -- 
> > > 2.35.1
> > >     
> >   
>
diff mbox series

Patch

diff --git a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
index d3bd7dc30f21..abd3c88fac7e 100755
--- a/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
+++ b/meta/recipes-kernel/linux-module/files/debian/rules.tmpl
@@ -55,6 +55,7 @@  override_dh_auto_build:
 
 override_dh_auto_install:
 	$(MAKE) -C $(KDIR) M=$(PWD) INSTALL_MOD_PATH=$(PWD)/debian/${PN} modules_install
+	install -D -m 0644 $(PWD)/debian/${PN}.conf $(PWD)/debian/${PN}/usr/lib/depmod.d/${PN}.conf
 
 %:
 	CFLAGS= LDFLAGS= dh $@ --parallel
diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index b20ec32ae20a..475e7aef05ea 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -29,9 +29,12 @@  TEMPLATE_FILES = "debian/control.tmpl \
                   debian/rules.tmpl"
 TEMPLATE_VARS += "KERNEL_NAME KERNEL_TYPE KERNEL_IMAGE_PKG KERNEL_HEADERS_PKG DEBIAN_BUILD_DEPENDS PN"
 
+INSTALL_MOD_DIR ?= "extra"
+
 do_prepare_build() {
     cp -r ${WORKDIR}/debian ${S}/
 
+    echo "search ${INSTALL_MOD_DIR} built-in" > ${S}/debian/${PN}.conf
     for module in ${AUTOLOAD}; do
         echo "echo $module >> /etc/modules" >> ${S}/debian/postinst
     done