[PATCH] init: add comments to keep initcall-names in sync with initcall levels

[PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Jim Cromie]

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 add "loadable" into comment re not using *_initcall macros in
modules, to disambiguate from kernel/params.c and other builtins.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 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..5e664f6 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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Borislav Petkov]

On Thu, Jun 14, 2012 at 04:00:59PM -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 add "loadable" into comment re not using *_initcall macros in
> modules, to disambiguate from kernel/params.c and other builtins.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

This time I actually build-tested it.

Acked-by: Borislav Petkov <borislav.petkov@amd.com>

Thanks.

-- 
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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Rusty Russell]

On Fri, 15 Jun 2012 12:29:06 +0200, Borislav Petkov <bp@amd64.org> wrote:
> On Thu, Jun 14, 2012 at 04:00:59PM -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 add "loadable" into comment re not using *_initcall macros in
> > modules, to disambiguate from kernel/params.c and other builtins.
> > 
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> 
> This time I actually build-tested it.
> 
> Acked-by: Borislav Petkov <borislav.petkov@amd.com>
> 
> Thanks.

Applied.

Thanks,
Rusty.

[PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Jim Cromie]

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 <jim.cromie@gmail.com>
---
 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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Borislav Petkov]

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 <jim.cromie@gmail.com>

Acked-by: Borislav Petkov <borislav.petkov@amd.com>.

-- 
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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Paul Bolle]

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 <jim.cromie@gmail.com>
> ---
>  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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Borislav Petkov]

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 <jim.cromie@gmail.com>
> > ---
> >  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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Jim Cromie]

On Thu, Jun 14, 2012 at 1:23 PM, Paul Bolle <pebolle@tiscali.nl> 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"

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Paul Bolle]

On Thu, 2012-06-14 at 14:32 -0600, Jim Cromie wrote:
> On Thu, Jun 14, 2012 at 1:23 PM, Paul Bolle <pebolle@tiscali.nl> 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

Re: [PATCH] init: add comments to keep initcall-names in sync with initcall levels

[Borislav Petkov]

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