All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@verge.net.au>
To: "M. Mohan Kumar" <mohan@in.ibm.com>
Cc: linuxppc-dev@ozlabs.org, kexec@lists.infradead.org, miltonm@bga.com
Subject: Re: [PATCH] Do not inline putprops function
Date: Wed, 17 Jun 2009 21:45:52 +1000	[thread overview]
Message-ID: <20090617114551.GA5672@verge.net.au> (raw)
In-Reply-To: <20090617113456.GC31595@in.ibm.com>

On Wed, Jun 17, 2009 at 05:04:56PM +0530, M. Mohan Kumar wrote:
> Do not inline putprops function
> 
> With the recent kexec-tools git tree, both kexec and kdump kernels hang (i.e
> kexec -l and kexec -p respectively). This happened after the patch "ppc64:
> cleanups" commit b43a84a31a4be6ed025c1bdef3bb1c3c12e01b16. I tried
> reverting each hunk and then found out that retaining following lines in
> fs2dt.c makes kexec/kdump work.
> 
> -static unsigned *dt_len; /* changed len of modified cmdline
> -                           in flat device-tree */
> 
> [....]
> 
> -               dt_len = dt;
> 
> I don't have any clue why removing a unused variable would cause the kexec
> kernel to hang. After further investigation, I observed that if the putprops
> function is not inlined, kexec/kdump kernel would work even after removing
> the above lines.
> 
> This patch directs gcc to not inline the putprops function. Now we could
> invoke kexec and kdump kernels.

I'm happy to apply this if you provide a sign-off,
but do you know why not inlining makes a difference here?

> ---
>  kexec/arch/ppc64/fs2dt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
> index 1f551fd..1e01f74 100644
> --- a/kexec/arch/ppc64/fs2dt.c
> +++ b/kexec/arch/ppc64/fs2dt.c
> @@ -259,7 +259,7 @@ static void add_usable_mem_property(int fd, int len)
>  }
>  
>  /* put all properties (files) in the property structure */
> -static void putprops(char *fn, struct dirent **nlist, int numlist)
> +__attribute__ ((noinline)) static void putprops(char *fn, struct dirent **nlist, int numlist)
>  {
>  	struct dirent *dp;
>  	int i = 0, fd, len;
> -- 
> 1.6.0.6

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@verge.net.au>
To: "M. Mohan Kumar" <mohan@in.ibm.com>
Cc: linuxppc-dev@ozlabs.org, kexec@lists.infradead.org, miltonm@bga.com
Subject: Re: [PATCH] Do not inline putprops function
Date: Wed, 17 Jun 2009 21:45:52 +1000	[thread overview]
Message-ID: <20090617114551.GA5672@verge.net.au> (raw)
In-Reply-To: <20090617113456.GC31595@in.ibm.com>

On Wed, Jun 17, 2009 at 05:04:56PM +0530, M. Mohan Kumar wrote:
> Do not inline putprops function
> 
> With the recent kexec-tools git tree, both kexec and kdump kernels hang (i.e
> kexec -l and kexec -p respectively). This happened after the patch "ppc64:
> cleanups" commit b43a84a31a4be6ed025c1bdef3bb1c3c12e01b16. I tried
> reverting each hunk and then found out that retaining following lines in
> fs2dt.c makes kexec/kdump work.
> 
> -static unsigned *dt_len; /* changed len of modified cmdline
> -                           in flat device-tree */
> 
> [....]
> 
> -               dt_len = dt;
> 
> I don't have any clue why removing a unused variable would cause the kexec
> kernel to hang. After further investigation, I observed that if the putprops
> function is not inlined, kexec/kdump kernel would work even after removing
> the above lines.
> 
> This patch directs gcc to not inline the putprops function. Now we could
> invoke kexec and kdump kernels.

I'm happy to apply this if you provide a sign-off,
but do you know why not inlining makes a difference here?

> ---
>  kexec/arch/ppc64/fs2dt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
> index 1f551fd..1e01f74 100644
> --- a/kexec/arch/ppc64/fs2dt.c
> +++ b/kexec/arch/ppc64/fs2dt.c
> @@ -259,7 +259,7 @@ static void add_usable_mem_property(int fd, int len)
>  }
>  
>  /* put all properties (files) in the property structure */
> -static void putprops(char *fn, struct dirent **nlist, int numlist)
> +__attribute__ ((noinline)) static void putprops(char *fn, struct dirent **nlist, int numlist)
>  {
>  	struct dirent *dp;
>  	int i = 0, fd, len;
> -- 
> 1.6.0.6

  reply	other threads:[~2009-06-17 11:46 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-17 11:34 [PATCH] Do not inline putprops function M. Mohan Kumar
2009-06-17 11:34 ` M. Mohan Kumar
2009-06-17 11:45 ` Simon Horman [this message]
2009-06-17 11:45   ` Simon Horman
2009-06-17 11:59   ` M. Mohan Kumar
2009-06-17 11:59     ` M. Mohan Kumar
2009-06-17 12:26     ` Michael Ellerman
2009-06-17 12:26       ` Michael Ellerman
2009-06-17 13:04       ` Neil Horman
2009-06-17 13:04         ` Neil Horman
2009-06-17 13:34         ` M. Mohan Kumar
2009-06-17 13:34           ` M. Mohan Kumar
2009-06-17 14:05           ` Neil Horman
2009-06-17 14:05             ` Neil Horman
2009-06-17 14:26             ` M. Mohan Kumar
2009-06-17 14:26               ` M. Mohan Kumar
2009-06-17 14:40               ` Neil Horman
2009-06-17 14:40                 ` Neil Horman
2009-06-23 12:55                 ` M. Mohan Kumar
2009-06-23 12:55                   ` M. Mohan Kumar
2009-06-23 13:56                   ` Neil Horman
2009-06-23 13:56                     ` Neil Horman
2009-06-24  0:27                     ` Michael Ellerman
2009-06-24  0:27                       ` Michael Ellerman
2009-08-03  5:49                       ` M. Mohan Kumar
2009-08-03  5:49                         ` M. Mohan Kumar
2009-08-05 16:49                         ` M. Mohan Kumar
2009-08-05 16:49                           ` M. Mohan Kumar
2009-08-06 14:24                           ` Michael Ellerman
2009-08-06 14:24                             ` Michael Ellerman
2009-08-07 14:35                             ` M. Mohan Kumar
2009-08-07 14:35                               ` M. Mohan Kumar
2009-08-07 14:54                               ` M. Mohan Kumar
2009-08-07 14:54                                 ` M. Mohan Kumar
2009-08-10  1:51                                 ` Michael Ellerman
2009-08-10  1:51                                   ` Michael Ellerman
2009-08-10  7:37                           ` Milton Miller
2009-08-10  7:37                             ` Milton Miller

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=20090617114551.GA5672@verge.net.au \
    --to=horms@verge.net.au \
    --cc=kexec@lists.infradead.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=miltonm@bga.com \
    --cc=mohan@in.ibm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.