[PATCH] init: Properly placing noinline keyword. - Kernel

This is a discussion on [PATCH] init: Properly placing noinline keyword. - Kernel ; 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, ...

+ Reply to Thread
Results 1 to 7 of 7

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

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

    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();
    --
    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/

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

    On 10/17/08, Alexey Dobriyan 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)

    >

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

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

    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)

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

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

    On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
    >On 10/17/08, Alexey Dobriyan 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."

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

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

    On 10/17/08, Américo Wang wrote:
    > On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
    > >On 10/17/08, Alexey Dobriyan 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/
    >

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

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

    On Fri, Oct 17, 2008 at 09:10:07PM +0600, Rakib Mullick wrote:
    >On 10/17/08, Américo Wang wrote:
    >> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
    >> >On 10/17/08, Alexey Dobriyan 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."

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

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

    On Fri, 17 Oct 2008 18:31:23 +0100
    Am__rico Wang wrote:

    > On Fri, Oct 17, 2008 at 09:10:07PM +0600, Rakib Mullick wrote:
    > >On 10/17/08, Am__rico Wang wrote:
    > >> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
    > >> >On 10/17/08, Alexey Dobriyan 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.

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

+ Reply to Thread