From: Harish Jenny Kandiga Nagaraj <harish_kandiga@mentor.com>
To: Lucas De Marchi <lucas.de.marchi@gmail.com>,
Rusty Russell <rusty@rustcorp.com.au>
Cc: linux-modules <linux-modules@vger.kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN
Date: Thu, 19 Feb 2015 11:19:16 +0530 [thread overview]
Message-ID: <54E5795C.5050804@mentor.com> (raw)
In-Reply-To: <CAKi4VAKc_zrPptt2ZKT_Jj8bLdL1bt0emhtEuodV2WKkas9ZOw@mail.gmail.com>
On Thursday 19 February 2015 06:49 AM, Lucas De Marchi wrote:
> On Wed, Feb 18, 2015 at 8:40 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>>> On Wed, Feb 18, 2015 at 2:07 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>> Yeah, I just thought (an wanted that) the attributes were being
>>> created first and then hooked up in the sysfs tree under
>>> /sys/module/<modulename>. I.e. if the directory exists and there's no
>>> initstate this is because it's a builtin module. I don't want to
>>> wait/sleep on the file to appear because users of
>>> kmod_module_get_initstate() may not tolerate this behavior.
>>>
>>> Looking up at the old module-init-tools, it used an ugly loop with
>>> usleep() before trying to read the file again :-/
>>>
>>> Can we change kernel side guaranteeing the initstate file appears
>>> together with the directory?
>> Greg? The core problem is that kmod looks for
>> /sys/module/<name>/initstate; if it's not there, it assumes a builtin
>> module.
> Just to make it clear:
>
> We try to open /sys/module/<name>/initstate. If it fails we stat
> /sys/module/<name> checking if it exists and is a directory. If it
> does then we assume the module is builtin.
>
>> However, this is racy when a module is being inserted. Is there a way
>> to create this sysfs file and dir atomically?
> Greg, the question is still valid since it'd be nice to have this
> guarantee and be able to correctly reply the state with whatever is in
> initstate file, but...
>
> Rusty, thinking again if we fallback to "coming" instead of "builtin"
> everything should be fine, no? Because the decision about builtin has
> already been taken by looking at the modules.builtin index. If we
> return "coming" here the second call to modprobe would call
> init_module() again which would wait for the first one to complete (or
> return EEXIST if it's already live) since we only shortcut the
> init_module() call if the module is live or builtin
>
> what do you think?
>
>
> Harrish, in your patch if you just change the "return
> KMOD_MODULE_BUILTIN;" to "return KMOD_MODULE_COMING;" does it work?
>
Yes. Returning KMOD_MODULE_COMING instead of KMOD_MODULE_BUILTIN works. The built-in modules are handled by looking at the modules.builtin index file. Is there any chance of returning KMOD_MODULE_COMING for builti-in modules? If it does not have any impact, then the fix should be fine.
Do I need to send a separate patch ?
next prev parent reply other threads:[~2015-02-19 5:49 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-17 12:56 [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN Harish Jenny K N
2015-02-17 17:30 ` Lucas De Marchi
2015-02-18 4:07 ` Rusty Russell
2015-02-18 6:10 ` Harish Jenny Kandiga Nagaraj
2015-02-18 16:50 ` Lucas De Marchi
2015-02-18 22:40 ` Rusty Russell
2015-02-19 1:19 ` Lucas De Marchi
2015-02-19 2:25 ` greg KH
2015-02-19 3:46 ` Lucas De Marchi
2015-02-19 2:25 ` Rusty Russell
2015-02-19 3:34 ` Lucas De Marchi
2015-02-19 5:49 ` Harish Jenny Kandiga Nagaraj [this message]
2015-02-19 10:30 ` Lucas De Marchi
2015-02-19 12:32 ` Harish Jenny Kandiga Nagaraj
2015-02-19 12:43 ` Lucas De Marchi
2015-02-19 14:02 ` Harish Jenny Kandiga Nagaraj
2015-02-19 14:35 ` Harish Jenny Kandiga Nagaraj
2015-02-28 17:28 ` Lucas De Marchi
2015-03-02 4:52 ` Harish Jenny Kandiga Nagaraj
2015-02-19 12:33 ` Harish Jenny Kandiga Nagaraj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54E5795C.5050804@mentor.com \
--to=harish_kandiga@mentor.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-modules@vger.kernel.org \
--cc=lucas.de.marchi@gmail.com \
--cc=rusty@rustcorp.com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).