2013-07-24 07:18:08

by Herbert Xu

[permalink] [raw]
Subject: [PATCH] modules: add support for soft module dependencies

Hi Rusty:

I don't know why this patch never went into the kernel, even
though the corresponding features have been added to modprobe
in most if not all distros.

This is required for dependencies on crypto modules such as
crc32c where the dependency is only visible at run-time, which
means that depmod fails to list the necessary dependencies
causing modules to go missing in the initrd.

Acked-by: Herbert Xu <[email protected]>

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Attachments:
(No filename) (607.00 B)
(No filename) (1.01 kB)
Download all attachments

2013-07-25 02:03:41

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
> Herbert Xu <[email protected]> writes:
> > Hi Rusty:
> >
> > I don't know why this patch never went into the kernel, even
> > though the corresponding features have been added to modprobe
> > in most if not all distros.
>
> Because Andreas never sent me the patch? This is the first I've *heard*
> of this feature. Looks like it didn't hit lkml either. And what was
> 2/2?

2/2 was the patch to actually use this in crc32c.

> It's not how I would have done this: post-deps are more flexibly done at
> runtime, because the module may have to do work to figure out what to
> pull in. But since it already exists, I'll apply this patch: it doesn't
> cost the kernel anything.
>
> And I've fixed the example, so we know how to list multiple modules!

Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-09-10 14:01:38

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
<[email protected]> wrote:
> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>> Herbert Xu <[email protected]> writes:
>> > Hi Rusty:
>> >
>> > I don't know why this patch never went into the kernel, even
>> > though the corresponding features have been added to modprobe
>> > in most if not all distros.
>>
>> Because Andreas never sent me the patch? This is the first I've *heard*
>> of this feature. Looks like it didn't hit lkml either. And what was
>> 2/2?
>
> 2/2 was the patch to actually use this in crc32c.
>
>> It's not how I would have done this: post-deps are more flexibly done at
>> runtime, because the module may have to do work to figure out what to
>> pull in. But since it already exists, I'll apply this patch: it doesn't
>> cost the kernel anything.

But it did cause boot failures. The file modules.softdep file was
supposed to be informational until now. That's why depmod put a
comment saying to "copy on user's discretion to /etc/modules.d"
instead of parsing it directly.

If dependencies expressed with softdeps can be required dependencies
now, depmod needs to be updated to work it out otherwise we will get
missing dependencies as happened with the crypto stuff after this
patch. I'm CC'ing Tom who had the boot failure; it looks like the same
boot failure that caused this feature to get reverted the first time
(the deps being required by sd_mod).

Even if it is an optional module, it would be great to have depmod
updated so it parses the file directly now that we are exporting this
info. I can do that once we agree what we want to do with softdeps
coming directly from kernel itself.

However looking at the only user right now, crct10dif: couldn't we
detect at runtime if this module can be used instead of just trying to
load it as a pre softdep and possibly failing?


Lucas De Marchi

2013-09-10 14:18:16

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Tue, Sep 10, 2013 at 4:01 PM, Lucas De Marchi
<[email protected]> wrote:
> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
> <[email protected]> wrote:
>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>> Herbert Xu <[email protected]> writes:
>>> > Hi Rusty:
>>> >
>>> > I don't know why this patch never went into the kernel, even
>>> > though the corresponding features have been added to modprobe
>>> > in most if not all distros.
>>>
>>> Because Andreas never sent me the patch? This is the first I've *heard*
>>> of this feature. Looks like it didn't hit lkml either. And what was
>>> 2/2?
>>
>> 2/2 was the patch to actually use this in crc32c.
>>
>>> It's not how I would have done this: post-deps are more flexibly done at
>>> runtime, because the module may have to do work to figure out what to
>>> pull in. But since it already exists, I'll apply this patch: it doesn't
>>> cost the kernel anything.
>
> But it did cause boot failures. The file modules.softdep file was
> supposed to be informational until now. That's why depmod put a
> comment saying to "copy on user's discretion to /etc/modules.d"
> instead of parsing it directly.
>
> If dependencies expressed with softdeps can be required dependencies
> now, depmod needs to be updated to work it out otherwise we will get
> missing dependencies as happened with the crypto stuff after this
> patch. I'm CC'ing Tom who had the boot failure; it looks like the same
> boot failure that caused this feature to get reverted the first time
> (the deps being required by sd_mod).

For the record: what happened was that I didn't get the "softdep'ed"
modules in my initrd and when trying to modprobe sd_mod in the initrd
I got an error message about missing symbols (I forget the precise
wording). We cold certainly fix the initrd generation to include the
softdep modules, but it seems unexpected that missing softdeps should
cause failures like this.

> Even if it is an optional module, it would be great to have depmod
> updated so it parses the file directly now that we are exporting this
> info. I can do that once we agree what we want to do with softdeps
> coming directly from kernel itself.

I just submitted a patch to do this, I think it is much more useful
than expecting the packager/admin to copy modules.softdep to the
correct location.

> However looking at the only user right now, crct10dif: couldn't we
> detect at runtime if this module can be used instead of just trying to
> load it as a pre softdep and possibly failing?

Having softdeps statically specified is useful for inird generation.
Even if it turns out that the modules are truly optional, having the
possibility of automatically including all optional modules would
surely be useful.

Cheers,

Tom

2013-09-11 04:40:03

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Tue, Sep 10, 2013 at 11:01:38AM -0300, Lucas De Marchi wrote:
>
> However looking at the only user right now, crct10dif: couldn't we
> detect at runtime if this module can be used instead of just trying to
> load it as a pre softdep and possibly failing?

No as sd_mod requires crct10dif to be present. The use of softdep
as opposed to just a symbolic dependency is due to there being two
modules for crct10dif, a generic version and an arch-specific one.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2013-09-13 00:40:55

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

Lucas De Marchi <[email protected]> writes:
> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
> <[email protected]> wrote:
>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>> Herbert Xu <[email protected]> writes:
>>> > Hi Rusty:
>>> >
>>> > I don't know why this patch never went into the kernel, even
>>> > though the corresponding features have been added to modprobe
>>> > in most if not all distros.
>>>
>>> Because Andreas never sent me the patch? This is the first I've *heard*
>>> of this feature. Looks like it didn't hit lkml either. And what was
>>> 2/2?
>>
>> 2/2 was the patch to actually use this in crc32c.
>>
>>> It's not how I would have done this: post-deps are more flexibly done at
>>> runtime, because the module may have to do work to figure out what to
>>> pull in. But since it already exists, I'll apply this patch: it doesn't
>>> cost the kernel anything.
>
> But it did cause boot failures. The file modules.softdep file was
> supposed to be informational until now. That's why depmod put a
> comment saying to "copy on user's discretion to /etc/modules.d"
> instead of parsing it directly.

I'm happy to change this macro to create a modinfo line like
"softdep:<modname>"

ie. tools like mkinitrd could pick it up and try to find a matching
module, but depmod would ignore it.

It's really up to Lucas, since this affects him.

Cheers,
Rusty.

2013-09-17 22:27:49

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <[email protected]> wrote:
> Lucas De Marchi <[email protected]> writes:
>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>> <[email protected]> wrote:
>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>> Herbert Xu <[email protected]> writes:
>>>> > Hi Rusty:
>>>> >
>>>> > I don't know why this patch never went into the kernel, even
>>>> > though the corresponding features have been added to modprobe
>>>> > in most if not all distros.
>>>>
>>>> Because Andreas never sent me the patch? This is the first I've *heard*
>>>> of this feature. Looks like it didn't hit lkml either. And what was
>>>> 2/2?
>>>
>>> 2/2 was the patch to actually use this in crc32c.
>>>
>>>> It's not how I would have done this: post-deps are more flexibly done at
>>>> runtime, because the module may have to do work to figure out what to
>>>> pull in. But since it already exists, I'll apply this patch: it doesn't
>>>> cost the kernel anything.
>>
>> But it did cause boot failures. The file modules.softdep file was
>> supposed to be informational until now. That's why depmod put a
>> comment saying to "copy on user's discretion to /etc/modules.d"
>> instead of parsing it directly.
>
> I'm happy to change this macro to create a modinfo line like
> "softdep:<modname>"

how is that solving the issue that this macro can be used to designate
a mandatory or optional dependency
(https://lkml.org/lkml/2013/9/10/371)? If we decide the dependency is
mandatory we can very well let modprobe use that dependency during
module load

>
> ie. tools like mkinitrd could pick it up and try to find a matching
> module, but depmod would ignore it.

Some mkinitrd-like use whatever depmod/modprobe tells them it's
needed. So kmod still needs to know about it.


>
> It's really up to Lucas, since this affects him.

IMO saying "this is an optional dependency and we can work without"
doesn't buy us much. Distros will end up putting the soft dep in
/etc/modules.d, kmod will always use them anyway and failing to load
the soft dep will fail the module load. I'd like to have no distro
files in /etc/modules.d in future.


Lucas De Marchi

2013-09-18 04:10:13

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

Lucas De Marchi <[email protected]> writes:
> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <[email protected]> wrote:
>> Lucas De Marchi <[email protected]> writes:
>>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>>> <[email protected]> wrote:
>>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>>> Herbert Xu <[email protected]> writes:
>>>>> > Hi Rusty:
>>>>> >
>>>>> > I don't know why this patch never went into the kernel, even
>>>>> > though the corresponding features have been added to modprobe
>>>>> > in most if not all distros.
>>>>>
>>>>> Because Andreas never sent me the patch? This is the first I've *heard*
>>>>> of this feature. Looks like it didn't hit lkml either. And what was
>>>>> 2/2?
>>>>
>>>> 2/2 was the patch to actually use this in crc32c.
>>>>
>>>>> It's not how I would have done this: post-deps are more flexibly done at
>>>>> runtime, because the module may have to do work to figure out what to
>>>>> pull in. But since it already exists, I'll apply this patch: it doesn't
>>>>> cost the kernel anything.
>>>
>>> But it did cause boot failures. The file modules.softdep file was
>>> supposed to be informational until now. That's why depmod put a
>>> comment saying to "copy on user's discretion to /etc/modules.d"
>>> instead of parsing it directly.
>>
>> I'm happy to change this macro to create a modinfo line like
>> "softdep:<modname>"
>
> how is that solving the issue that this macro can be used to designate
> a mandatory or optional dependency
> (https://lkml.org/lkml/2013/9/10/371)? If we decide the dependency is
> mandatory we can very well let modprobe use that dependency during
> module load

I'm very close to sending Linus a revert commit at this point, since
there's no consensus on what it's for.

*Clearly* softdep shouldn't indicate a mandatory dependency. We already
have a way (several) to make mandatory dependencies!

And the "pre:" vs "post:" thing is just weird. If a module wants a post
dependency, you can request_module() it from a workqueue.

>> ie. tools like mkinitrd could pick it up and try to find a matching
>> module, but depmod would ignore it.
>
> Some mkinitrd-like use whatever depmod/modprobe tells them it's
> needed. So kmod still needs to know about it.

It sounds like we should create a separate tool, which takes a list of
modules and spits out the full pathname of all dependencies. *That*
tool should include soft dependencies.

>> It's really up to Lucas, since this affects him.
>
> IMO saying "this is an optional dependency and we can work without"
> doesn't buy us much. Distros will end up putting the soft dep in
> /etc/modules.d, kmod will always use them anyway and failing to load
> the soft dep will fail the module load. I'd like to have no distro
> files in /etc/modules.d in future.

I assumed modprobe would handle soft dependencies in modules and try to
pull them in, but *not* fail if they don't work.

The previous way of doing this was:
install foo modprobe foo_softdep 2>/dev/null; modprobe --ignore-install foo $CMDLINE_OPTS

I agree this logic belongs in the kernel, we just have to figure out
exactly how.

Cheers,
Rusty.

2013-09-18 05:33:06

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

On Tue, Sep 17, 2013 at 11:10 PM, Rusty Russell <[email protected]> wrote:
> Lucas De Marchi <[email protected]> writes:
>> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <[email protected]> wrote:
>>> Lucas De Marchi <[email protected]> writes:
>>>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>>>> <[email protected]> wrote:
>>>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>>>> Herbert Xu <[email protected]> writes:
>>>>>> > Hi Rusty:
>>>>>> >
>>>>>> > I don't know why this patch never went into the kernel, even
>>>>>> > though the corresponding features have been added to modprobe
>>>>>> > in most if not all distros.
>>>>>>
>>>>>> Because Andreas never sent me the patch? This is the first I've *heard*
>>>>>> of this feature. Looks like it didn't hit lkml either. And what was
>>>>>> 2/2?
>>>>>
>>>>> 2/2 was the patch to actually use this in crc32c.
>>>>>
>>>>>> It's not how I would have done this: post-deps are more flexibly done at
>>>>>> runtime, because the module may have to do work to figure out what to
>>>>>> pull in. But since it already exists, I'll apply this patch: it doesn't
>>>>>> cost the kernel anything.
>>>>
>>>> But it did cause boot failures. The file modules.softdep file was
>>>> supposed to be informational until now. That's why depmod put a
>>>> comment saying to "copy on user's discretion to /etc/modules.d"
>>>> instead of parsing it directly.
>>>
>>> I'm happy to change this macro to create a modinfo line like
>>> "softdep:<modname>"
>>
>> how is that solving the issue that this macro can be used to designate
>> a mandatory or optional dependency
>> (https://lkml.org/lkml/2013/9/10/371)? If we decide the dependency is
>> mandatory we can very well let modprobe use that dependency during
>> module load
>
> I'm very close to sending Linus a revert commit at this point, since
> there's no consensus on what it's for.
>
> *Clearly* softdep shouldn't indicate a mandatory dependency. We already
> have a way (several) to make mandatory dependencies!
>
> And the "pre:" vs "post:" thing is just weird. If a module wants a post
> dependency, you can request_module() it from a workqueue.
>
>>> ie. tools like mkinitrd could pick it up and try to find a matching
>>> module, but depmod would ignore it.
>>
>> Some mkinitrd-like use whatever depmod/modprobe tells them it's
>> needed. So kmod still needs to know about it.
>
> It sounds like we should create a separate tool, which takes a list of
> modules and spits out the full pathname of all dependencies. *That*
> tool should include soft dependencies.
>
>>> It's really up to Lucas, since this affects him.
>>
>> IMO saying "this is an optional dependency and we can work without"
>> doesn't buy us much. Distros will end up putting the soft dep in
>> /etc/modules.d, kmod will always use them anyway and failing to load
>> the soft dep will fail the module load. I'd like to have no distro
>> files in /etc/modules.d in future.
>
> I assumed modprobe would handle soft dependencies in modules and try to
> pull them in, but *not* fail if they don't work.

Iff the module doesn't *exist*. If it failed to load or failed for any
other reason then we will abort trying to load the other module.
However this is one thing we can change in modprobe to make it
consistent and more predictable. But we really need to reach a
consensus.

>
> The previous way of doing this was:
> install foo modprobe foo_softdep 2>/dev/null; modprobe --ignore-install foo $CMDLINE_OPTS

I just hope this is in no way an incentive for people using install commands ;-)


Lucas De Marchi

2013-09-18 07:17:10

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] modules: add support for soft module dependencies

Lucas De Marchi <[email protected]> writes:
> On Tue, Sep 17, 2013 at 11:10 PM, Rusty Russell <rusty-8n+1lVoiYb80n/[email protected]> wrote:
>> Lucas De Marchi <[email protected]> writes:
>>> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <rusty-8n+1lVoiYb80n/[email protected]> wrote:
>>>> I'm happy to change this macro to create a modinfo line like
>>>> "softdep:<modname>"
>>>
>>> how is that solving the issue that this macro can be used to designate
>>> a mandatory or optional dependency
>>> (https://lkml.org/lkml/2013/9/10/371)? If we decide the dependency is
>>> mandatory we can very well let modprobe use that dependency during
>>> module load
>>
>> I'm very close to sending Linus a revert commit at this point, since
>> there's no consensus on what it's for.
>>
>> *Clearly* softdep shouldn't indicate a mandatory dependency. We already
>> have a way (several) to make mandatory dependencies!
>>
>> And the "pre:" vs "post:" thing is just weird. If a module wants a post
>> dependency, you can request_module() it from a workqueue.
>>
>>>> ie. tools like mkinitrd could pick it up and try to find a matching
>>>> module, but depmod would ignore it.
>>>
>>> Some mkinitrd-like use whatever depmod/modprobe tells them it's
>>> needed. So kmod still needs to know about it.
>>
>> It sounds like we should create a separate tool, which takes a list of
>> modules and spits out the full pathname of all dependencies. *That*
>> tool should include soft dependencies.
>>
>>>> It's really up to Lucas, since this affects him.
>>>
>>> IMO saying "this is an optional dependency and we can work without"
>>> doesn't buy us much. Distros will end up putting the soft dep in
>>> /etc/modules.d, kmod will always use them anyway and failing to load
>>> the soft dep will fail the module load. I'd like to have no distro
>>> files in /etc/modules.d in future.
>>
>> I assumed modprobe would handle soft dependencies in modules and try to
>> pull them in, but *not* fail if they don't work.
>
> Iff the module doesn't *exist*. If it failed to load or failed for any
> other reason then we will abort trying to load the other module.
> However this is one thing we can change in modprobe to make it
> consistent and more predictable. But we really need to reach a
> consensus.

It is a bit of a corner case, but there are other conceivable reasons
why a module wouldn't load. The hardware it wants might not be present,
or busy, or it conflicts with an existing module, or needs user
configuration (eg. commandline params).

It might be worth reporting, but I wouldn't stop loading on softdep fails.

>> The previous way of doing this was:
>> install foo modprobe foo_softdep 2>/dev/null; modprobe --ignore-install foo $CMDLINE_OPTS
>
> I just hope this is in no way an incentive for people using install commands ;-)

Agreed. install commands were implemented because I had no idea what
people wanted to do in future. They definitely provide enough rope...

Cheers,
Rusty.