[PATCH] init: Properly placing noinline keyword.

[PATCH] init: Properly placing noinline keyword.

[Rakib Mullick]

Here, noinline keyword should be placed between storage class and type.
Thanks.

Signed-off-by: Md.Rakib H. Mullick(rakib.mullick@gmail.com)

--- linux-2.6-stable.orig/init/main.c	2008-10-16 20:04:15.000000000 +0600
+++ linux-2.6-stable/init/main.c	2008-10-16 20:59:29.562096784 +0600
@@ -457,7 +457,7 @@ static void __init setup_command_line(ch
  * gcc-3.4 accidentally inlines this function, so use noinline.
  */

-static void noinline __init_refok rest_init(void)
+static noinline void __init_refok rest_init(void)
 	__releases(kernel_lock)
 {
 	int pid;
@@ -792,7 +792,7 @@ static void run_init_process(char *init_
 /* This is a non __init function. Force it to be noinline otherwise gcc
  * makes it inline to init() and it becomes part of init.text section
  */
-static int noinline init_post(void)
+static noinline int init_post(void)
 {
 	free_initmem();
 	unlock_kernel();

Re: [PATCH] init: Properly placing noinline keyword.

[Alexey Dobriyan]

On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
> Here, noinline keyword should be placed between storage class and type.

Why?

> --- linux-2.6-stable.orig/init/main.c
> +++ linux-2.6-stable/init/main.c
> @@ -457,7 +457,7 @@ static void __init setup_command_line(ch
>   * gcc-3.4 accidentally inlines this function, so use noinline.
>   */
> 
> -static void noinline __init_refok rest_init(void)
> +static noinline void __init_refok rest_init(void)
>  	__releases(kernel_lock)
>  {
>  	int pid;
> @@ -792,7 +792,7 @@ static void run_init_process(char *init_
>  /* This is a non __init function. Force it to be noinline otherwise gcc
>   * makes it inline to init() and it becomes part of init.text section
>   */
> -static int noinline init_post(void)
> +static noinline int init_post(void)

Re: [PATCH] init: Properly placing noinline keyword.

[Rakib Mullick]

On 10/17/08, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
>  > Here, noinline keyword should be placed between storage class and type.
>
>
> Why?
Because, scripts/checkpatch.pl warned with following warning:
     ERROR: inline keyword should sit between storage class and type
>
>
>  > --- linux-2.6-stable.orig/init/main.c
>
> > +++ linux-2.6-stable/init/main.c
>
> > @@ -457,7 +457,7 @@ static void __init setup_command_line(ch
>  >   * gcc-3.4 accidentally inlines this function, so use noinline.
>  >   */
>  >
>  > -static void noinline __init_refok rest_init(void)
>  > +static noinline void __init_refok rest_init(void)
>  >       __releases(kernel_lock)
>  >  {
>  >       int pid;
>  > @@ -792,7 +792,7 @@ static void run_init_process(char *init_
>  >  /* This is a non __init function. Force it to be noinline otherwise gcc
>  >   * makes it inline to init() and it becomes part of init.text section
>  >   */
>  > -static int noinline init_post(void)
>  > +static noinline int init_post(void)
>

Re: [PATCH] init: Properly placing noinline keyword.

[Américo Wang]

On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
>On 10/17/08, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
>>  > Here, noinline keyword should be placed between storage class and type.
>>
>>
>> Why?
>Because, scripts/checkpatch.pl warned with following warning:
>     ERROR: inline keyword should sit between storage class and type

Well, 'noinline' is different from 'inline'.

'noinline' is defined as:

#define  noinline                       __attribute__((noinline))

in include/linux/compiler-gcc.h. But 'inline' is a _keyword_ defined
by C standard. If checkpatch.pl complains about 'noinline', you should
fix checkpatch.pl. :)


-- 
"Sometimes the only way to stay sane is to go a little crazy."

Re: [PATCH] init: Properly placing noinline keyword.

[Rakib Mullick]

On 10/17/08, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
>  >On 10/17/08, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>  >> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
>  >>  > Here, noinline keyword should be placed between storage class and type.
>  >>
>  >>
>  >> Why?
>  >Because, scripts/checkpatch.pl warned with following warning:
>  >     ERROR: inline keyword should sit between storage class and type
>
>
> Well, 'noinline' is different from 'inline'.
>
>  'noinline' is defined as:
>
>  #define  noinline                       __attribute__((noinline))
>
>  in include/linux/compiler-gcc.h. But 'inline' is a _keyword_ defined
>  by C standard. If checkpatch.pl complains about 'noinline', you should
>  fix checkpatch.pl. :)
Thanks, for explanation. But isn't it nice to place it between storage
class and type ?
>
>
>  --
>  "Sometimes the only way to stay sane is to go a little crazy."
>
>
>  --
>  To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>  Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] init: Properly placing noinline keyword.

[Américo Wang]

On Fri, Oct 17, 2008 at 09:10:07PM +0600, Rakib Mullick wrote:
>On 10/17/08, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
>>  >On 10/17/08, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>>  >> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
>>  >>  > Here, noinline keyword should be placed between storage class and type.
>>  >>
>>  >>
>>  >> Why?
>>  >Because, scripts/checkpatch.pl warned with following warning:
>>  >     ERROR: inline keyword should sit between storage class and type
>>
>>
>> Well, 'noinline' is different from 'inline'.
>>
>>  'noinline' is defined as:
>>
>>  #define  noinline                       __attribute__((noinline))
>>
>>  in include/linux/compiler-gcc.h. But 'inline' is a _keyword_ defined
>>  by C standard. If checkpatch.pl complains about 'noinline', you should
>>  fix checkpatch.pl. :)
>Thanks, for explanation. But isn't it nice to place it between storage
>class and type ?

I don't think so, I don't know why checkpatch.pl prefers that style.
I think probably only because that is more readable?

Anyway, gcc attribute is another different thing.

-- 
"Sometimes the only way to stay sane is to go a little crazy."

Re: [PATCH] init: Properly placing noinline keyword.

[Andrew Morton]

On Fri, 17 Oct 2008 18:31:23 +0100
Am__rico Wang <xiyou.wangcong@gmail.com> wrote:

> On Fri, Oct 17, 2008 at 09:10:07PM +0600, Rakib Mullick wrote:
> >On 10/17/08, Am__rico Wang <xiyou.wangcong@gmail.com> wrote:
> >> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
> >>  >On 10/17/08, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >>  >> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
> >>  >>  > Here, noinline keyword should be placed between storage class and type.
> >>  >>
> >>  >>
> >>  >> Why?
> >>  >Because, scripts/checkpatch.pl warned with following warning:
> >>  >     ERROR: inline keyword should sit between storage class and type
> >>
> >>
> >> Well, 'noinline' is different from 'inline'.
> >>
> >>  'noinline' is defined as:
> >>
> >>  #define  noinline                       __attribute__((noinline))
> >>
> >>  in include/linux/compiler-gcc.h. But 'inline' is a _keyword_ defined
> >>  by C standard. If checkpatch.pl complains about 'noinline', you should
> >>  fix checkpatch.pl. :)
> >Thanks, for explanation. But isn't it nice to place it between storage
> >class and type ?
> 
> I don't think so, I don't know why checkpatch.pl prefers that style.
> I think probably only because that is more readable?
> 

I think it's good for consistency reasons.  Yes, we _could_ have a
random sprinkling of different keyword orderings, but what benefit is
there in that?  In the great majority of places the kernel uses `static
inline void' and `static noinline void' ordering, and that's a good
thing, no?

So I merged the patch and I'd support retaining the checkpatch warning.

My one concern is that the patch is too small.  Probably there are
other codesites which get the keywords in a non-standard order, and I'd
rather fix them up in a single big pass rather than in a long series of
little patches.