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

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

[Lucas De Marchi]

On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
<herbert@gondor.apana.org.au> wrote:
> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>> Herbert Xu <herbert@gondor.apana.org.au> 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

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

[Tom Gundersen]

On Tue, Sep 10, 2013 at 4:01 PM, Lucas De Marchi
<lucas.de.marchi@gmail.com> wrote:
> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
> <herbert@gondor.apana.org.au> wrote:
>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>> Herbert Xu <herbert@gondor.apana.org.au> 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

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Rusty Russell]

Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
> <herbert@gondor.apana.org.au> wrote:
>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>> Herbert Xu <herbert@gondor.apana.org.au> 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.

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

[Lucas De Marchi]

On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>> <herbert@gondor.apana.org.au> wrote:
>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>> Herbert Xu <herbert@gondor.apana.org.au> 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

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

[Rusty Russell]

Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>>> <herbert@gondor.apana.org.au> wrote:
>>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>>> Herbert Xu <herbert@gondor.apana.org.au> 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.

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

[Lucas De Marchi]

On Tue, Sep 17, 2013 at 11:10 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>>>> On Wed, Jul 24, 2013 at 11:03 PM, Herbert Xu
>>>> <herbert@gondor.apana.org.au> wrote:
>>>>> On Thu, Jul 25, 2013 at 09:32:02AM +0930, Rusty Russell wrote:
>>>>>> Herbert Xu <herbert@gondor.apana.org.au> 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

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

[Rusty Russell]

Lucas De Marchi <lucas.demarchi@profusion.mobi> writes:
> On Tue, Sep 17, 2013 at 11:10 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>>> On Thu, Sep 12, 2013 at 9:07 PM, Rusty Russell <rusty@rustcorp.com.au> 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.