main.c has initcall_level_names[] for parse_args to print in debug messages,
add comments to keep them in sync with initcalls defined in init.h.
Also tweak comment re not using *_initcall macros in loadable modules.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/init.h | 3 ++-
init/main.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/init.h b/include/linux/init.h
index 6b95109..da6f6f9 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -191,6 +191,7 @@ extern bool initcall_debug;
* initializes variables that couldn't be statically initialized.
*
* This only exists for built-in code, not for modules.
+ * Keep main.c:initcall_level_names[] in sync. */
*/
#define pure_initcall(fn) __define_initcall("0",fn,0)
@@ -280,7 +281,7 @@ void __init parse_early_options(char *cmdline);
#else /* MODULE */
-/* Don't use these in modules, but some people do... */
+/* Don't use these in loadable modules, but some people do... */
#define early_initcall(fn) module_init(fn)
#define core_initcall(fn) module_init(fn)
#define postcore_initcall(fn) module_init(fn)
diff --git a/init/main.c b/init/main.c
index b5cc0a7..7a74087 100644
--- a/init/main.c
+++ b/init/main.c
@@ -724,6 +724,7 @@ static initcall_t *initcall_levels[] __initdata = {
__initcall_end,
};
+/* Keep these in sync with initcalls in include/linux/init.h */
static char *initcall_level_names[] __initdata = {
"early",
"core",
--
1.7.10.2
On Thu, Jun 14, 2012 at 12:51:02PM -0600, Jim Cromie wrote:
> main.c has initcall_level_names[] for parse_args to print in debug messages,
> add comments to keep them in sync with initcalls defined in init.h.
> Also tweak comment re not using *_initcall macros in loadable modules.
>
> Signed-off-by: Jim Cromie <[email protected]>
Acked-by: Borislav Petkov <[email protected]>.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On Thu, 2012-06-14 at 12:51 -0600, Jim Cromie wrote:
> main.c has initcall_level_names[] for parse_args to print in debug messages,
> add comments to keep them in sync with initcalls defined in init.h.
> Also tweak comment re not using *_initcall macros in loadable modules.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> include/linux/init.h | 3 ++-
> init/main.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 6b95109..da6f6f9 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -191,6 +191,7 @@ extern bool initcall_debug;
> * initializes variables that couldn't be statically initialized.
> *
> * This only exists for built-in code, not for modules.
> + * Keep main.c:initcall_level_names[] in sync. */
> */
This comment now ends with "*/" twice. Perhaps that's legal (I haven't
even bothered to check) but it is really too ugly.
> #define pure_initcall(fn) __define_initcall("0",fn,0)
>
> @@ -280,7 +281,7 @@ void __init parse_early_options(char *cmdline);
>
> #else /* MODULE */
>
> -/* Don't use these in modules, but some people do... */
> +/* Don't use these in loadable modules, but some people do... */
What problem does this solve?
> #define early_initcall(fn) module_init(fn)
> #define core_initcall(fn) module_init(fn)
> #define postcore_initcall(fn) module_init(fn)
> diff --git a/init/main.c b/init/main.c
> index b5cc0a7..7a74087 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -724,6 +724,7 @@ static initcall_t *initcall_levels[] __initdata = {
> __initcall_end,
> };
>
> +/* Keep these in sync with initcalls in include/linux/init.h */
> static char *initcall_level_names[] __initdata = {
> "early",
> "core",
Paul Bolle
On Thu, Jun 14, 2012 at 09:23:03PM +0200, Paul Bolle wrote:
> On Thu, 2012-06-14 at 12:51 -0600, Jim Cromie wrote:
> > main.c has initcall_level_names[] for parse_args to print in debug messages,
> > add comments to keep them in sync with initcalls defined in init.h.
> > Also tweak comment re not using *_initcall macros in loadable modules.
> >
> > Signed-off-by: Jim Cromie <[email protected]>
> > ---
> > include/linux/init.h | 3 ++-
> > init/main.c | 1 +
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 6b95109..da6f6f9 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -191,6 +191,7 @@ extern bool initcall_debug;
> > * initializes variables that couldn't be statically initialized.
> > *
> > * This only exists for built-in code, not for modules.
> > + * Keep main.c:initcall_level_names[] in sync. */
> > */
>
> This comment now ends with "*/" twice. Perhaps that's legal
Of course it isn't, good catch Paul:
In file included from include/linux/printk.h:4:0,
from include/linux/kernel.h:22,
from include/asm-generic/bug.h:5,
from /usr/src/linux-2.6/arch/x86/include/asm/bug.h:38,
from include/linux/bug.h:4,
from include/linux/page-flags.h:9,
from kernel/bounds.c:9:
include/linux/init.h:195:3: error: expected identifier or ‘(’ before ‘/’ token
make[1]: *** [kernel/bounds.s] Error 1
make: *** [prepare0] Error 2
Jim, you need to test-build your patches no matter how trivial they are.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
On Thu, Jun 14, 2012 at 1:23 PM, Paul Bolle <[email protected]> wrote:
> On Thu, 2012-06-14 at 12:51 -0600, Jim Cromie wrote:
>> *
>> * This only exists for built-in code, not for modules.
>> + * Keep main.c:initcall_level_names[] in sync. */
>> */
> This comment now ends with "*/" twice. Perhaps that's legal (I haven't
> even bothered to check) but it is really too ugly.
Guilty as charged, revision forthcoming.
>>
>> -/* Don't use these in modules, but some people do... */
>> +/* Don't use these in loadable modules, but some people do... */
>
> What problem does this solve?
>
kernel/params.c and other builtins are also modules - at least wrt
how theyre reported by dynamic_debug:
kernel/params.c:121 [params]parse_one =_ "Unknown argument `%s'\012"
kernel/params.c:117 [params]parse_one =_ "Unknown argument: calling %p\012"
kernel/params.c:108 [params]parse_one =_ "They are equal! Calling %p\012"
kernel/params.c:188 [params]parse_args =_ "Parsing ARGS: %s\012"
The advice to avoid those macros does not apply to builtin "modules"
On Thu, 2012-06-14 at 14:32 -0600, Jim Cromie wrote:
> On Thu, Jun 14, 2012 at 1:23 PM, Paul Bolle <[email protected]> wrote:
> > On Thu, 2012-06-14 at 12:51 -0600, Jim Cromie wrote:
> >>
> >> -/* Don't use these in modules, but some people do... */
> >> +/* Don't use these in loadable modules, but some people do... */
> >
> > What problem does this solve?
>
> kernel/params.c and other builtins are also modules - at least wrt
> how theyre reported by dynamic_debug:
>
> kernel/params.c:121 [params]parse_one =_ "Unknown argument `%s'\012"
> kernel/params.c:117 [params]parse_one =_ "Unknown argument: calling %p\012"
> kernel/params.c:108 [params]parse_one =_ "They are equal! Calling %p\012"
> kernel/params.c:188 [params]parse_args =_ "Parsing ARGS: %s\012"
>
> The advice to avoid those macros does not apply to builtin "modules"
I don't think I use dynamic_debug, but still, a pair of square brackets
doesn't make that some part of the kernel is considered to be a module,
does it? And more importantly, even if there's a difference between
"module" and "loadable module", which I rather doubt, aren't the people
who are expected to read this comment also expected to understand the
relevance of the preceding
#else /* MODULE */
line?
Paul Bolle
On Thu, Jun 14, 2012 at 11:21:53PM +0200, Paul Bolle wrote:
> > > What problem does this solve?
> >
> > kernel/params.c and other builtins are also modules - at least wrt
> > how theyre reported by dynamic_debug:
> >
> > kernel/params.c:121 [params]parse_one =_ "Unknown argument `%s'\012"
> > kernel/params.c:117 [params]parse_one =_ "Unknown argument: calling %p\012"
> > kernel/params.c:108 [params]parse_one =_ "They are equal! Calling %p\012"
> > kernel/params.c:188 [params]parse_args =_ "Parsing ARGS: %s\012"
> >
> > The advice to avoid those macros does not apply to builtin "modules"
>
> I don't think I use dynamic_debug, but still, a pair of square brackets
> doesn't make that some part of the kernel is considered to be a module,
> does it? And more importantly, even if there's a difference between
> "module" and "loadable module", which I rather doubt, aren't the people
> who are expected to read this comment also expected to understand the
> relevance of the preceding
> #else /* MODULE */
>
> line?
I don't understand one thing: what's wrong with adding another word to
the comment so that it explicitly states what "modules" this comment is
referring to?
Can you give me at least one technical reason against the comment being
as precise as possible, even to the point of tautology. So what if it
says "loadable modules"? I don't see anything wrong with that.
So please, let's drop the bikeshedding and get on with our lives :-)
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551