2015-02-22 15:24:30

by Adrien Schildknecht

[permalink] [raw]
Subject: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

i386 is already using kstack_end() in dumpstack_32.c.
We should also use it to make the code clearer and unify the stack
printing logic some more.

This patch depends on patch "x86: fix output of show_stack_log_lvl()"

Signed-off-by: Adrien Schildknecht <[email protected]>
---
arch/x86/kernel/dumpstack_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 553573b..5f1c626 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -280,7 +280,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
pr_cont(" <EOI> ");
}
} else {
- if (((long) stack & (THREAD_SIZE-1)) == 0)
+ if (kstack_end(stack))
break;
}
if ((i % STACKSLOTS_PER_LINE) == 0) {
--
2.2.1


2015-02-23 17:17:56

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

On Sun, 22 Feb 2015 16:23:58 +0100
Adrien Schildknecht <[email protected]> wrote:

> i386 is already using kstack_end() in dumpstack_32.c.
> We should also use it to make the code clearer and unify the stack
> printing logic some more.

Looks fine to me.

>
> This patch depends on patch "x86: fix output of show_stack_log_lvl()"

I'm curious to what the dependency is? What would break if we apply
this without that patch?

-- Steve

>
> Signed-off-by: Adrien Schildknecht <[email protected]>
> ---
> arch/x86/kernel/dumpstack_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
> index 553573b..5f1c626 100644
> --- a/arch/x86/kernel/dumpstack_64.c
> +++ b/arch/x86/kernel/dumpstack_64.c
> @@ -280,7 +280,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
> pr_cont(" <EOI> ");
> }
> } else {
> - if (((long) stack & (THREAD_SIZE-1)) == 0)
> + if (kstack_end(stack))
> break;
> }
> if ((i % STACKSLOTS_PER_LINE) == 0) {

2015-02-23 17:21:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

On Mon, Feb 23, 2015 at 12:17:51PM -0500, Steven Rostedt wrote:
> I'm curious to what the dependency is? What would break if we apply
> this without that patch?

Nah, no dependency - it causes just a little fuzz when the previous one
is missing.

I'll apply it to the proper tree and fixup the commit message :)

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-02-23 17:29:11

by Adrien Schildknecht

[permalink] [raw]
Subject: Re: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

> > This patch depends on patch "x86: fix output of
> > show_stack_log_lvl()"
>
> I'm curious to what the dependency is? What would break if we apply
> this without that patch?

Maybe I misunderstood "dependency".
I mean that this patch won't apply properly since a line of the context
was changed by the previous patch.
"if ((i % STACKSLOTS_PER_LINE) == 0) {"

--
Adrien Schildknecht

2015-02-23 17:54:16

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

On Mon, 23 Feb 2015 18:29:05 +0100
Adrien Schildknecht <[email protected]> wrote:

> > > This patch depends on patch "x86: fix output of
> > > show_stack_log_lvl()"
> >
> > I'm curious to what the dependency is? What would break if we apply
> > this without that patch?
>
> Maybe I misunderstood "dependency".
> I mean that this patch won't apply properly since a line of the context
> was changed by the previous patch.
> "if ((i % STACKSLOTS_PER_LINE) == 0) {"
>

Yeah, "dependency" is a bit ambiguous here. It can mean both "this
patch wont work without said patch" as well as "this patch wont apply
without said patch". Probably would have been better to say that it
won't apply to avoid the ambiguity.

Also, these types of notes are best stated after the '---' line. As
it's good information for the maintainer pulling in the patch, but
should not go in the change log. Same for comments like "Changes since
version 1 of this series".

That is:

i386 is already using kstack_end() in dumpstack_32.c.
We should also use it to make the code clearer and unify the stack
printing logic some more.

Signed-off-by: Adrien Schildknecht <[email protected]>
---
Note: This patch depends on patch "x86: fix output of show_stack_log_lvl()"
or it might not apply properly.

arch/x86/kernel/dumpstack_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


The change log for the above stops at the '---'.

-- Steve

2015-02-23 18:03:28

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86_64: use kstack_end() in dumpstack_64.c

On Sun, Feb 22, 2015 at 04:23:58PM +0100, Adrien Schildknecht wrote:
> i386 is already using kstack_end() in dumpstack_32.c.
> We should also use it to make the code clearer and unify the stack
> printing logic some more.
>
> This patch depends on patch "x86: fix output of show_stack_log_lvl()"
>
> Signed-off-by: Adrien Schildknecht <[email protected]>

Applied, thanks.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--