* [PATCH] sh: dwarf: prefer _THIS_IP_ to current_text_addr
@ 2018-08-01 18:29 Nick Desaulniers
2018-08-01 18:50 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-08-01 18:29 UTC (permalink / raw)
To: ysato, dalias
Cc: akpm, alexander.levin, linux-sh, linux-kernel, Nick Desaulniers
As part of the effort to reduce the code duplication between _THIS_IP_
and current_text_addr(), let's consolidate callers of
current_text_addr() to use _THIS_IP_.
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
arch/sh/kernel/dwarf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sh/kernel/dwarf.c b/arch/sh/kernel/dwarf.c
index 1a2526676a87..bb511e2d9d68 100644
--- a/arch/sh/kernel/dwarf.c
+++ b/arch/sh/kernel/dwarf.c
@@ -599,7 +599,7 @@ struct dwarf_frame *dwarf_unwind_stack(unsigned long pc,
* time this function makes its first function call.
*/
if (!pc || !prev)
- pc = (unsigned long)current_text_addr();
+ pc = _THIS_IP_;
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
/*
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] sh: dwarf: prefer _THIS_IP_ to current_text_addr
2018-08-01 18:29 [PATCH] sh: dwarf: prefer _THIS_IP_ to current_text_addr Nick Desaulniers
@ 2018-08-01 18:50 ` Nick Desaulniers
2018-08-01 18:53 ` [PATCH v2] sh: " Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-08-01 18:50 UTC (permalink / raw)
To: Yoshinori Sato, dalias; +Cc: Andrew Morton, linux-sh, LKML
On Wed, Aug 1, 2018 at 11:29 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> As part of the effort to reduce the code duplication between _THIS_IP_
> and current_text_addr(), let's consolidate callers of
> current_text_addr() to use _THIS_IP_.
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> arch/sh/kernel/dwarf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/sh/kernel/dwarf.c b/arch/sh/kernel/dwarf.c
> index 1a2526676a87..bb511e2d9d68 100644
> --- a/arch/sh/kernel/dwarf.c
> +++ b/arch/sh/kernel/dwarf.c
> @@ -599,7 +599,7 @@ struct dwarf_frame *dwarf_unwind_stack(unsigned long pc,
> * time this function makes its first function call.
> */
> if (!pc || !prev)
> - pc = (unsigned long)current_text_addr();
> + pc = _THIS_IP_;
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> /*
> --
> 2.18.0.597.ga71716f1ad-goog
>
Sorry, there's another call site in the arch/sh/ tree
(arch/sh/include/asm/kexec.h#64). Let me fix both and send as a
single patch in v2.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] sh: prefer _THIS_IP_ to current_text_addr
2018-08-01 18:50 ` Nick Desaulniers
@ 2018-08-01 18:53 ` Nick Desaulniers
2018-08-01 21:55 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-08-01 18:53 UTC (permalink / raw)
To: ysato, dalias; +Cc: akpm, linux-sh, linux-kernel, Nick Desaulniers
As part of the effort to reduce the code duplication between _THIS_IP_
and current_text_addr(), let's consolidate callers of
current_text_addr() to use _THIS_IP_.
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
arch/sh/include/asm/kexec.h | 3 ++-
arch/sh/kernel/dwarf.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/sh/include/asm/kexec.h b/arch/sh/include/asm/kexec.h
index fd5f331a3912..927d80ba2332 100644
--- a/arch/sh/include/asm/kexec.h
+++ b/arch/sh/include/asm/kexec.h
@@ -4,6 +4,7 @@
#include <asm/ptrace.h>
#include <asm/string.h>
+#include <linux/kernel.h>
/*
* KEXEC_SOURCE_MEMORY_LIMIT maximum page get_free_page can return.
@@ -61,7 +62,7 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
__asm__ __volatile__ ("stc gbr, %0" : "=r" (newregs->gbr));
__asm__ __volatile__ ("stc sr, %0" : "=r" (newregs->sr));
- newregs->pc = (unsigned long)current_text_addr();
+ newregs->pc = _THIS_IP_;
}
}
#else
diff --git a/arch/sh/kernel/dwarf.c b/arch/sh/kernel/dwarf.c
index 1a2526676a87..bb511e2d9d68 100644
--- a/arch/sh/kernel/dwarf.c
+++ b/arch/sh/kernel/dwarf.c
@@ -599,7 +599,7 @@ struct dwarf_frame *dwarf_unwind_stack(unsigned long pc,
* time this function makes its first function call.
*/
if (!pc || !prev)
- pc = (unsigned long)current_text_addr();
+ pc = _THIS_IP_;
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
/*
--
2.18.0.597.ga71716f1ad-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] sh: prefer _THIS_IP_ to current_text_addr
2018-08-01 18:53 ` [PATCH v2] sh: " Nick Desaulniers
@ 2018-08-01 21:55 ` Andrew Morton
2018-08-01 22:06 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2018-08-01 21:55 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: ysato, dalias, linux-sh, linux-kernel
On Wed, 1 Aug 2018 11:53:31 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
> As part of the effort to reduce the code duplication between _THIS_IP_
> and current_text_addr(), let's consolidate callers of
> current_text_addr() to use _THIS_IP_.
Why not switch everything to current_text_addr()? _THIS_IP_ is ugly ;)
Several architectures (s390, sparc, sh, ...) do funky things in their
current_text_addr(). Does the generic kernel.h implementation of
current_text_addr() work OK on those architectures?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] sh: prefer _THIS_IP_ to current_text_addr
2018-08-01 21:55 ` Andrew Morton
@ 2018-08-01 22:06 ` Nick Desaulniers
2018-08-01 22:22 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-08-01 22:06 UTC (permalink / raw)
To: Andrew Morton; +Cc: Yoshinori Sato, dalias, linux-sh, LKML
On Wed, Aug 1, 2018 at 2:55 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 1 Aug 2018 11:53:31 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> > As part of the effort to reduce the code duplication between _THIS_IP_
> > and current_text_addr(), let's consolidate callers of
> > current_text_addr() to use _THIS_IP_.
>
> Why not switch everything to current_text_addr()? _THIS_IP_ is ugly ;)
>
> Several architectures (s390, sparc, sh, ...) do funky things in their
> current_text_addr(). Does the generic kernel.h implementation of
> current_text_addr() work OK on those architectures?
Good question, one I've been mulling over for a few days. That's
definitely something that I've considered. Mostly, it comes down to
there being fewer call sites of current_text_addr() than _THIS_IP_, so
it's IMO a smaller change that doesn't require coordinating tree-wide
changes.
What's interesting about current_text_addr() is that while there are
many arch specific definitions, there are only 5 call sites in the
whole kernel (and 4 of them are arch specific). So unless there's an
arch specific reason to prefer inline assembly over higher level C
(yes, returning the address of a label in a statement expression is
ugly), (it looks like parisc might have a reason, at its lone call
site, see: https://lkml.org/lkml/2018/8/1/1678, TBD), then I don't
think that current_text_addr() has a raison d'être. I suspect that
its use predates the GNU C extensions needed for the current generic C
implementation (statement expressions + addr of label) and has just
been reimplemented in every new arch's arch/*/include/asm/processor.h
simply because the other archs had it.
So the plan is to replace the 5 call sites of current_text_addr() with
_THIS_IP_ (if possible), then delete all definitions of the arch
specific current_text_addr()'s. This can be done on a per tree basis,
rather than a treewide patch for converting _THIS_IP_ (who has many
more call sites) to current_text_addr(). What do you think?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] sh: prefer _THIS_IP_ to current_text_addr
2018-08-01 22:06 ` Nick Desaulniers
@ 2018-08-01 22:22 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2018-08-01 22:22 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: Yoshinori Sato, dalias, linux-sh, LKML
On Wed, 1 Aug 2018 15:06:33 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote:
> So the plan is to replace the 5 call sites of current_text_addr() with
> _THIS_IP_ (if possible), then delete all definitions of the arch
> specific current_text_addr()'s. This can be done on a per tree basis,
> rather than a treewide patch for converting _THIS_IP_ (who has many
> more call sites) to current_text_addr(). What do you think?
Sounds good to me.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-01 22:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-01 18:29 [PATCH] sh: dwarf: prefer _THIS_IP_ to current_text_addr Nick Desaulniers
2018-08-01 18:50 ` Nick Desaulniers
2018-08-01 18:53 ` [PATCH v2] sh: " Nick Desaulniers
2018-08-01 21:55 ` Andrew Morton
2018-08-01 22:06 ` Nick Desaulniers
2018-08-01 22:22 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).