On Thu, Nov 30, 2017 at 7:39 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Thu, Nov 30, 2017 at 02:17:11PM +0100, Jessica Yu wrote:
>> Just some quick questions - are there any plans to use these in-kernel
>> module aliases anywhere else? Or are you using them just for debugging?
>
> As-is for now just debugging, but this could also more easily enable folks to
> prototype further evaluation of its uses. IMHO just having this at least posted
> online should suffice the later aspect of enabling folks to prototype.
I confirm that, after the module auto-load discussion where it is
clear that we need to improve the infrastructure, this debug
information may save some time, maybe someone can automate a script go
through modules and then on filesystem, however these patches may show
which module lead to load another one, right ? on userspace if there
are multiple dependencies it can be difficult I think.
>
> You're right that one can find aliases in userspace. One of the benefits
> of having this dump things on the kernel log is just that you can easily
> get the aliases printed out for all modules actually loaded for your system
> without much effort. I did find this useful when debugging and found it much
> more convenient than scraping modules one by one by hand in userspace.
>
> I had this implemented since 2016, and I had some ideas to use them in a
> functional way, however I first had to knock out a series of of fixes for
> kernel/kmod.c and setting up a baseline test infrastructure for kmod
> (tools/testing/selftests/kmod/ and lib/test_kmod.c) as such I hadn't had time
> to yet come around and finish benchmarking the alias enhancement ideas I had
> started evaluating.
>
> As such having aliases in-kernel currently are only useful for debugging and
> prototyping.
I would say so, however no strong argument if it should be mainlined.
Luis in your commit log you say:
"Obviously userspace can be buggy though, and it can lie to us. We
currently have no easy way to determine this."
Could you please share some info here ? how userspace can be buggy ?
Thank you!
> Luis
--
tixxdz
+++ Djalal Harouni [04/12/17 10:01 +0100]:
>On Thu, Nov 30, 2017 at 7:39 PM, Luis R. Rodriguez <[email protected]> wrote:
>> On Thu, Nov 30, 2017 at 02:17:11PM +0100, Jessica Yu wrote:
>>> Just some quick questions - are there any plans to use these in-kernel
>>> module aliases anywhere else? Or are you using them just for debugging?
>>
>> As-is for now just debugging, but this could also more easily enable folks to
>> prototype further evaluation of its uses. IMHO just having this at least posted
>> online should suffice the later aspect of enabling folks to prototype.
>
>I confirm that, after the module auto-load discussion where it is
>clear that we need to improve the infrastructure, this debug
>information may save some time, maybe someone can automate a script go
>through modules and then on filesystem,
>however these patches may show
>which module lead to load another one, right ? on userspace if there
>are multiple dependencies it can be difficult I think.
Hm? I'm confused by what you mean here. The patchset just saves and
prints a module's aliases on module load if the debug option is
enabled. There's no dependency tracking here; that's modprobe's job.
And if you need to see which additional modules are being loaded as a
result of a module load there's already modprobe --verbose and
modules.dep..
>> You're right that one can find aliases in userspace. One of the benefits
>> of having this dump things on the kernel log is just that you can easily
>> get the aliases printed out for all modules actually loaded for your system
>> without much effort. I did find this useful when debugging and found it much
>> more convenient than scraping modules one by one by hand in userspace.
>>
>> I had this implemented since 2016, and I had some ideas to use them in a
>> functional way, however I first had to knock out a series of of fixes for
>> kernel/kmod.c and setting up a baseline test infrastructure for kmod
>> (tools/testing/selftests/kmod/ and lib/test_kmod.c) as such I hadn't had time
>> to yet come around and finish benchmarking the alias enhancement ideas I had
>> started evaluating.
>>
>> As such having aliases in-kernel currently are only useful for debugging and
>> prototyping.
>
>I would say so, however no strong argument if it should be mainlined.
>Luis in your commit log you say:
>
>"Obviously userspace can be buggy though, and it can lie to us. We
>currently have no easy way to determine this."
>
>Could you please share some info here ? how userspace can be buggy ?
Ditto, I agree that a concrete debugging example would be helpful in
understanding why having this in-kernel is better than using existing
userspace tools to look up module aliases. AFAIK the debug prints only
on module load, but you could also easily get the aliases for all
modules loaded on your system by doing something like -
for mod in $(awk '{print $1}' /proc/modules) ; do echo $mod; modinfo -F alias $mod ; done
(Assuming they are all modules that modprobe knows about) Is there a
debugging scenario you ran into where the above script wouldn't
suffice?
Jessica
On Mon, Dec 4, 2017 at 2:58 PM, Jessica Yu <[email protected]> wrote:
> +++ Djalal Harouni [04/12/17 10:01 +0100]:
>>
>> On Thu, Nov 30, 2017 at 7:39 PM, Luis R. Rodriguez <[email protected]>
>> wrote:
>>>
>>> On Thu, Nov 30, 2017 at 02:17:11PM +0100, Jessica Yu wrote:
>>>>
>>>> Just some quick questions - are there any plans to use these in-kernel
>>>> module aliases anywhere else? Or are you using them just for debugging?
>>>
>>>
>>> As-is for now just debugging, but this could also more easily enable
>>> folks to
>>> prototype further evaluation of its uses. IMHO just having this at least
>>> posted
>>> online should suffice the later aspect of enabling folks to prototype.
>>
>>
>> I confirm that, after the module auto-load discussion where it is
>> clear that we need to improve the infrastructure, this debug
>> information may save some time, maybe someone can automate a script go
>> through modules and then on filesystem,
>
>
>> however these patches may show
>> which module lead to load another one, right ? on userspace if there
>> are multiple dependencies it can be difficult I think.
>
>
> Hm? I'm confused by what you mean here. The patchset just saves and
> prints a module's aliases on module load if the debug option is
> enabled. There's no dependency tracking here; that's modprobe's job.
> And if you need to see which additional modules are being loaded as a
> result of a module load there's already modprobe --verbose and
> modules.dep..
Yes I was referring by the printing or kernel logs order, if two
modules depend on same one, we know which first one triggered it, and
in that context it will be a bit easier in the auto-loading context,
maybe like crypto ones that can be triggered from anywhere.
Thanks!
--
tixxdz
On Mon, Dec 04, 2017 at 10:01:02AM +0100, Djalal Harouni wrote:
> Luis in your commit log you say:
>
> "Obviously userspace can be buggy though, and it can lie to us. We
> currently have no easy way to determine this."
>
> Could you please share some info here ? how userspace can be buggy ?
Sure, so kmod <= v19 was broken -- it returns 0 to modprobe calls, assuming
the kernel module is built-in, where really we have a race as the module starts
forming. kmod <= v19 has incorrect userspace heuristics, a userspace kmod fix
is available for it:
http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4
Although this is fixed now, in theory userspace can regress again, but debugging
these sorts of issue is not so easy at all.
Technically, with aliases parsed (as this patch series proposed it's just
debugging data), it should be possible for request_module() to also issue a
finished_kmod_load() so that when request_module() completes and returns 0 if
we know for certain the module is loaded. This could be useful either debugging
purposes, for instance to catch when userspace lies once again, or if we do
find value in it, to make a pedantic and patient new request_module_load(),
which for instance would want to just wait for the module to really be loaded.
What I mean is that some request_module() calls uses *assume* that if it
returns 0 that the module is loaded, and this is incorrect. Each
request_module() users today *should* in theory vet and ensure each module is
loaded correctly. Since there is no generic form to do this today,
try_then_request_module() was added to help with that.
try_then_request_module() solves two issues really, one is it prevents a
request to userspace if the module is already loaded, and two, it double checks
again if the module is loaded at the end using whatever heuristic the caller
wishes.
AFAICT its subjective whether or not each caller should open code its
own try_then_request_module() form, or if we should have a generic validator,
as such even though I have some dev code to use aliases to do something like
finished_kmod_load(), its just debug private code I use right now, and AFAICT
we have no formal justification for a change. Perhaps one could argue that
having a generic validator is much easier than expecting all callers to use
it correctly, but last time I checked I found only one buggy users of
request_module() and I fixed it (see 41124db869b ("fs: warn in case userspace
lied about modprobe return") so the point was moot.
You might think that expanding uses of aliases might benefit from a validator,
but the only thing I can think of right now is that avoiding to call
userspace if the module is already loaded, but this can *already* be
done by using try_then_request_module() or open coding your own checker
as get_fs_type() does, its just none of this is in a very generic form.
If you do come up with a valid argument for a generic form or for a full
generic wait as you expand use of aliases using request_module() do let me
know and we can revisit separately.
In the meantime I just noticed I do have a valid small optimization in my dev
code which does make sense upstream which I'll send in for review.
Luis