With allnoconfig, bp becomes unused. So put bp into defination of
CONFIG_FRAME_POINTER.
arch/x86/kernel/dumpstack.c: In function ?dump_stack?:
arch/x86/kernel/dumpstack.c:200: warning: unused variable ?bp?
Signed-off-by: Rakib Mullick <[email protected]>
---
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 8474c99..fc5a253 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -197,10 +197,10 @@ void show_stack(struct task_struct *task,
unsigned long *sp)
*/
void dump_stack(void)
{
- unsigned long bp = 0;
unsigned long stack;
#ifdef CONFIG_FRAME_POINTER
+ unsigned long bp = 0;
if (!bp)
get_bp(bp);
#endif
On Wed, 24 Nov 2010, Rakib Mullick wrote:
> With allnoconfig, bp becomes unused. So put bp into defination of
> CONFIG_FRAME_POINTER.
>
> arch/x86/kernel/dumpstack.c: In function ?dump_stack?:
> arch/x86/kernel/dumpstack.c:200: warning: unused variable ?bp?
>
> Signed-off-by: Rakib Mullick <[email protected]>
> ---
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 8474c99..fc5a253 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -197,10 +197,10 @@ void show_stack(struct task_struct *task,
> unsigned long *sp)
> */
> void dump_stack(void)
> {
> - unsigned long bp = 0;
> unsigned long stack;
>
> #ifdef CONFIG_FRAME_POINTER
> + unsigned long bp = 0;
> if (!bp)
> get_bp(bp);
> #endif
So, now the bp variable does not exist at all if CONFIG_FRAME_POINTER is
not defined.
That's going to make this line :
show_trace(NULL, NULL, &stack, bp);
found further down in the dump_stack() function, quite unhappy.
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
On Wed, Nov 24, 2010 at 3:57 PM, Jesper Juhl <[email protected]> wrote:
> On Wed, 24 Nov 2010, Rakib Mullick wrote:
>
>
> So, now the bp variable does not exist at all if CONFIG_FRAME_POINTER is
> not defined.
Yes - right. And its not used outside CONFIG_FRAME_POINTER.
> That's going to make this line :
>
> ? ? ? ? ?show_trace(NULL, NULL, &stack, bp);
>
Not clear what are you trying to say. Would you please, clear it up a bit?
> found further down in the dump_stack() function, quite unhappy.
>
>
> --
> Jesper Juhl <[email protected]> ? ? ? ? ? ?http://www.chaosbits.net/
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please.
>
On Wed, Nov 24, 2010 at 04:26:01PM +0600, Rakib Mullick wrote:
> On Wed, Nov 24, 2010 at 3:57 PM, Jesper Juhl <[email protected]> wrote:
> > On Wed, 24 Nov 2010, Rakib Mullick wrote:
> >
> >
> > So, now the bp variable does not exist at all if CONFIG_FRAME_POINTER is
> > not defined.
>
> Yes - right. And its not used outside CONFIG_FRAME_POINTER.
>
> > That's going to make this line :
> >
> > ? ? ? ? ?show_trace(NULL, NULL, &stack, bp);
> >
> Not clear what are you trying to say. Would you please, clear it up a bit?
I think Jesper is confused because your patch applies on -tip (tip/master or
tip/perf/core) where we have zapped the bp argument from show_trace() lately.
But in the mainline, show_trace() still has the bp argument.
2010/11/24 Rakib Mullick <[email protected]>:
> With allnoconfig, bp becomes unused. So put bp into defination of
> CONFIG_FRAME_POINTER.
>
> arch/x86/kernel/dumpstack.c: In function ?dump_stack?:
> arch/x86/kernel/dumpstack.c:200: warning: unused variable ?bp?
>
> Signed-off-by: Rakib Mullick <[email protected]>
> ---
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 8474c99..fc5a253 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -197,10 +197,10 @@ void show_stack(struct task_struct *task,
> unsigned long *sp)
> ?*/
> ?void dump_stack(void)
> ?{
> - ? ? ? unsigned long bp = 0;
> ? ? ? ?unsigned long stack;
>
> ?#ifdef CONFIG_FRAME_POINTER
> + ? ? ? unsigned long bp = 0;
> ? ? ? ?if (!bp)
> ? ? ? ? ? ? ? ?get_bp(bp);
> ?#endif
>
Looks like you can even remove that block. bp is not used any more in
this function.
get_bp() was only loading bp but did not make any use of it.
Thanks.
On Wed, 24 Nov 2010, Frederic Weisbecker wrote:
> On Wed, Nov 24, 2010 at 04:26:01PM +0600, Rakib Mullick wrote:
> > On Wed, Nov 24, 2010 at 3:57 PM, Jesper Juhl <[email protected]> wrote:
> > > On Wed, 24 Nov 2010, Rakib Mullick wrote:
> > >
> > >
> > > So, now the bp variable does not exist at all if CONFIG_FRAME_POINTER is
> > > not defined.
> >
> > Yes - right. And its not used outside CONFIG_FRAME_POINTER.
> >
> > > That's going to make this line :
> > >
> > > ? ? ? ? ?show_trace(NULL, NULL, &stack, bp);
> > >
> > Not clear what are you trying to say. Would you please, clear it up a bit?
>
>
> I think Jesper is confused because your patch applies on -tip (tip/master or
> tip/perf/core) where we have zapped the bp argument from show_trace() lately.
>
> But in the mainline, show_trace() still has the bp argument.
Ahh yes, my mistake. I checked the code in mainline.
Sorry about the confusion.
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.
On Wed, 24 Nov 2010 10:57:10 +0100, Jesper Juhl said:
> On Wed, 24 Nov 2010, Rakib Mullick wrote:
> > diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> > index 8474c99..fc5a253 100644
> > --- a/arch/x86/kernel/dumpstack.c
> > +++ b/arch/x86/kernel/dumpstack.c
> > @@ -197,10 +197,10 @@ void show_stack(struct task_struct *task,
> > unsigned long *sp)
> > */
> > void dump_stack(void)
> > {
> > - unsigned long bp = 0;
> > unsigned long stack;
> >
> > #ifdef CONFIG_FRAME_POINTER
> > + unsigned long bp = 0;
> > if (!bp)
> > get_bp(bp);
> > #endif
>
> So, now the bp variable does not exist at all if CONFIG_FRAME_POINTER is
> not defined.
> That's going to make this line :
>
> show_trace(NULL, NULL, &stack, bp);
>
> found further down in the dump_stack() function, quite unhappy.
OK, I'll bite. Why does the original say 'unsigned long bp = 0;' and then turns
around and has an 'if (!bp)' check? Why is the conditional there?
On Wed, Nov 24, 2010 at 7:19 PM, Frederic Weisbecker <[email protected]> wrote:
> 2010/11/24 Rakib Mullick <[email protected]>:
>
> Looks like you can even remove that block. bp is not used any more in
> this function.
> get_bp() was only loading bp but did not make any use of it.
Yes, right. Here is the updated one.
In dump_stack function, bp isn't used anymore, which is introduced by
commit 9c0729dc8062bed96189bd14ac6d4920f3958743. This patch removes bp
completely.
Signed-off-by: Rakib Mullick <[email protected]>
---
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 8474c99..d6fb146 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -197,14 +197,8 @@ void show_stack(struct task_struct *task,
unsigned long *sp)
*/
void dump_stack(void)
{
- unsigned long bp = 0;
unsigned long stack;
-#ifdef CONFIG_FRAME_POINTER
- if (!bp)
- get_bp(bp);
-#endif
-
printk("Pid: %d, comm: %.20s %s %s %.*s\n",
current->pid, current->comm, print_tainted(),
init_utsname()->release,
>
> Thanks.
>
On Wed, Nov 24, 2010 at 7:01 PM, Frederic Weisbecker <[email protected]> wrote:
> On Wed, Nov 24, 2010 at 04:26:01PM +0600, Rakib Mullick wrote:
>> On Wed, Nov 24, 2010 at 3:57 PM, Jesper Juhl <[email protected]> wrote:
>
> I think Jesper is confused because your patch applies on -tip (tip/master or
> tip/perf/core) where we have zapped the bp argument from show_trace() lately.
>
Yes, the patch was made against tip/master.
> But in the mainline, show_trace() still has the bp argument.
>
Le Wed, 24 Nov 2010 23:07:47 -0500,
[email protected] a ?crit :
> On Wed, 24 Nov 2010 10:57:10 +0100, Jesper Juhl said:
>
> > On Wed, 24 Nov 2010, Rakib Mullick wrote:
>
> > > diff --git a/arch/x86/kernel/dumpstack.c
> > > b/arch/x86/kernel/dumpstack.c index 8474c99..fc5a253 100644
> > > --- a/arch/x86/kernel/dumpstack.c
> > > +++ b/arch/x86/kernel/dumpstack.c
> > > @@ -197,10 +197,10 @@ void show_stack(struct task_struct *task,
> > > unsigned long *sp)
> > > */
> > > void dump_stack(void)
> > > {
> > > - unsigned long bp = 0;
> > > unsigned long stack;
> > >
> > > #ifdef CONFIG_FRAME_POINTER
> > > + unsigned long bp = 0;
> > > if (!bp)
> > > get_bp(bp);
> > > #endif
> >
> > So, now the bp variable does not exist at all if
> > CONFIG_FRAME_POINTER is not defined.
> > That's going to make this line :
> >
> > show_trace(NULL, NULL, &stack, bp);
> >
> > found further down in the dump_stack() function, quite unhappy.
>
> OK, I'll bite. Why does the original say 'unsigned long bp = 0;' and
> then turns around and has an 'if (!bp)' check? Why is the
> conditional there?
The original check was indeed not necessary. But now the whole block
should be removed anyway.
Le Thu, 25 Nov 2010 11:35:16 +0600,
Rakib Mullick <[email protected]> a ?crit :
> On Wed, Nov 24, 2010 at 7:19 PM, Frederic Weisbecker
> <[email protected]> wrote:
> > 2010/11/24 Rakib Mullick <[email protected]>:
> >
> > Looks like you can even remove that block. bp is not used any more
> > in this function.
> > get_bp() was only loading bp but did not make any use of it.
>
> Yes, right. Here is the updated one.
>
> In dump_stack function, bp isn't used anymore, which is introduced by
> commit 9c0729dc8062bed96189bd14ac6d4920f3958743. This patch removes bp
> completely.
>
>
> Signed-off-by: Rakib Mullick <[email protected]>
> ---
>
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index 8474c99..d6fb146 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -197,14 +197,8 @@ void show_stack(struct task_struct *task,
> unsigned long *sp)
> */
> void dump_stack(void)
> {
> - unsigned long bp = 0;
> unsigned long stack;
>
> -#ifdef CONFIG_FRAME_POINTER
> - if (!bp)
> - get_bp(bp);
> -#endif
> -
> printk("Pid: %d, comm: %.20s %s %s %.*s\n",
> current->pid, current->comm, print_tainted(),
> init_utsname()->release,
>
Queued, thanks!