Re: [PATCH v7 6/6] powerpc/perf: split callchain.c by bitness
On Fri, 30 Aug 2019 23:03:43 +0200
Michal Suchanek wrote:
> Building callchain.c with !COMPAT proved quite ugly with all the
> defines. Splitting out the 32bit and 64bit parts looks better.
>
> No code change intended.
valid_user_sp is broken in compat. It needs to be ifdefed for the 32bit
case.
>
> Signed-off-by: Michal Suchanek
> Reviewed-by: Christophe Leroy
> ---
> v6:
> - move current_is_64bit consolidetaion to earlier patch
> - move defines to the top of callchain_32.c
> - Makefile cleanup
> ---
> arch/powerpc/perf/Makefile | 5 +-
> arch/powerpc/perf/callchain.c| 365 +--
> arch/powerpc/perf/callchain.h| 11 +
> arch/powerpc/perf/callchain_32.c | 204 +
> arch/powerpc/perf/callchain_64.c | 185
> 5 files changed, 405 insertions(+), 365 deletions(-)
> create mode 100644 arch/powerpc/perf/callchain.h
> create mode 100644 arch/powerpc/perf/callchain_32.c
> create mode 100644 arch/powerpc/perf/callchain_64.c
>
> diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
> index c155dcbb8691..53d614e98537 100644
> --- a/arch/powerpc/perf/Makefile
> +++ b/arch/powerpc/perf/Makefile
> @@ -1,6 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -obj-$(CONFIG_PERF_EVENTS)+= callchain.o perf_regs.o
> +obj-$(CONFIG_PERF_EVENTS)+= callchain.o callchain_$(BITS).o perf_regs.o
> +ifdef CONFIG_COMPAT
> +obj-$(CONFIG_PERF_EVENTS)+= callchain_32.o
> +endif
>
> obj-$(CONFIG_PPC_PERF_CTRS) += core-book3s.o bhrb.o
> obj64-$(CONFIG_PPC_PERF_CTRS)+= ppc970-pmu.o power5-pmu.o \
> diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
> index 9b8dc822f531..8f30f1b47c78 100644
> --- a/arch/powerpc/perf/callchain.c
> +++ b/arch/powerpc/perf/callchain.c
> @@ -15,11 +15,9 @@
> #include
> #include
> #include
> -#ifdef CONFIG_COMPAT
> -#include "../kernel/ppc32.h"
> -#endif
> #include
>
> +#include "callchain.h"
>
> /*
> * Is sp valid as the address of the next kernel stack frame after prev_sp?
> @@ -102,367 +100,6 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx
> *entry, struct pt_regs *re
> }
> }
>
> -#ifdef CONFIG_PPC64
> -/*
> - * On 64-bit we don't want to invoke hash_page on user addresses from
> - * interrupt context, so if the access faults, we read the page tables
> - * to find which page (if any) is mapped and access it directly.
> - */
> -static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
> -{
> - int ret = -EFAULT;
> - pgd_t *pgdir;
> - pte_t *ptep, pte;
> - unsigned shift;
> - unsigned long addr = (unsigned long) ptr;
> - unsigned long offset;
> - unsigned long pfn, flags;
> - void *kaddr;
> -
> - pgdir = current->mm->pgd;
> - if (!pgdir)
> - return -EFAULT;
> -
> - local_irq_save(flags);
> - ptep = find_current_mm_pte(pgdir, addr, NULL, );
> - if (!ptep)
> - goto err_out;
> - if (!shift)
> - shift = PAGE_SHIFT;
> -
> - /* align address to page boundary */
> - offset = addr & ((1UL << shift) - 1);
> -
> - pte = READ_ONCE(*ptep);
> - if (!pte_present(pte) || !pte_user(pte))
> - goto err_out;
> - pfn = pte_pfn(pte);
> - if (!page_is_ram(pfn))
> - goto err_out;
> -
> - /* no highmem to worry about here */
> - kaddr = pfn_to_kaddr(pfn);
> - memcpy(buf, kaddr + offset, nb);
> - ret = 0;
> -err_out:
> - local_irq_restore(flags);
> - return ret;
> -}
> -
> -static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
> -{
> - if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned long) ||
> - ((unsigned long)ptr & 7))
> - return -EFAULT;
> -
> - pagefault_disable();
> - if (!__get_user_inatomic(*ret, ptr)) {
> - pagefault_enable();
> - return 0;
> - }
> - pagefault_enable();
> -
> - return read_user_stack_slow(ptr, ret, 8);
> -}
> -
> -static inline int valid_user_sp(unsigned long sp, int is_64)
> -{
> - if (!sp || (sp & 7) || sp > (is_64 ? TASK_SIZE : 0x1UL) - 32)
> - return 0;
> - return 1;
> -}
> -
> -/*
> - * 64-bit user processes use the same stack frame for RT and non-RT signals.
> - */
> -struct signal_frame_64 {
> - chardummy[__SIGNAL_FRAMESIZE];
> - struct ucontext uc;
> - unsigned long unused[2];
> - unsigned inttramp[6];
> - struct siginfo *pinfo;
> - void*puc;
> - struct siginfo info;
> - charabigap[288];
> -};
> -
> -static int is_sigreturn_64_address(unsigned long nip, unsigned long fp)
> -{
> - if (nip == fp + offsetof(struct signal_frame_64, tramp))
> - return 1;
> - if (vdso64_rt_sigtramp && current->mm->context.vdso_base &&
> - nip == current->mm->context.vdso_base + vdso64_rt_sigtramp)
> - return 1;
> - return 0;
> -}
> -
[PATCH v7 6/6] powerpc/perf: split callchain.c by bitness
Building callchain.c with !COMPAT proved quite ugly with all the
defines. Splitting out the 32bit and 64bit parts looks better.
No code change intended.
Signed-off-by: Michal Suchanek
Reviewed-by: Christophe Leroy
---
v6:
- move current_is_64bit consolidetaion to earlier patch
- move defines to the top of callchain_32.c
- Makefile cleanup
---
arch/powerpc/perf/Makefile | 5 +-
arch/powerpc/perf/callchain.c| 365 +--
arch/powerpc/perf/callchain.h| 11 +
arch/powerpc/perf/callchain_32.c | 204 +
arch/powerpc/perf/callchain_64.c | 185
5 files changed, 405 insertions(+), 365 deletions(-)
create mode 100644 arch/powerpc/perf/callchain.h
create mode 100644 arch/powerpc/perf/callchain_32.c
create mode 100644 arch/powerpc/perf/callchain_64.c
diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
index c155dcbb8691..53d614e98537 100644
--- a/arch/powerpc/perf/Makefile
+++ b/arch/powerpc/perf/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_PERF_EVENTS) += callchain.o perf_regs.o
+obj-$(CONFIG_PERF_EVENTS) += callchain.o callchain_$(BITS).o perf_regs.o
+ifdef CONFIG_COMPAT
+obj-$(CONFIG_PERF_EVENTS) += callchain_32.o
+endif
obj-$(CONFIG_PPC_PERF_CTRS)+= core-book3s.o bhrb.o
obj64-$(CONFIG_PPC_PERF_CTRS) += ppc970-pmu.o power5-pmu.o \
diff --git a/arch/powerpc/perf/callchain.c b/arch/powerpc/perf/callchain.c
index 9b8dc822f531..8f30f1b47c78 100644
--- a/arch/powerpc/perf/callchain.c
+++ b/arch/powerpc/perf/callchain.c
@@ -15,11 +15,9 @@
#include
#include
#include
-#ifdef CONFIG_COMPAT
-#include "../kernel/ppc32.h"
-#endif
#include
+#include "callchain.h"
/*
* Is sp valid as the address of the next kernel stack frame after prev_sp?
@@ -102,367 +100,6 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx
*entry, struct pt_regs *re
}
}
-#ifdef CONFIG_PPC64
-/*
- * On 64-bit we don't want to invoke hash_page on user addresses from
- * interrupt context, so if the access faults, we read the page tables
- * to find which page (if any) is mapped and access it directly.
- */
-static int read_user_stack_slow(void __user *ptr, void *buf, int nb)
-{
- int ret = -EFAULT;
- pgd_t *pgdir;
- pte_t *ptep, pte;
- unsigned shift;
- unsigned long addr = (unsigned long) ptr;
- unsigned long offset;
- unsigned long pfn, flags;
- void *kaddr;
-
- pgdir = current->mm->pgd;
- if (!pgdir)
- return -EFAULT;
-
- local_irq_save(flags);
- ptep = find_current_mm_pte(pgdir, addr, NULL, );
- if (!ptep)
- goto err_out;
- if (!shift)
- shift = PAGE_SHIFT;
-
- /* align address to page boundary */
- offset = addr & ((1UL << shift) - 1);
-
- pte = READ_ONCE(*ptep);
- if (!pte_present(pte) || !pte_user(pte))
- goto err_out;
- pfn = pte_pfn(pte);
- if (!page_is_ram(pfn))
- goto err_out;
-
- /* no highmem to worry about here */
- kaddr = pfn_to_kaddr(pfn);
- memcpy(buf, kaddr + offset, nb);
- ret = 0;
-err_out:
- local_irq_restore(flags);
- return ret;
-}
-
-static int read_user_stack_64(unsigned long __user *ptr, unsigned long *ret)
-{
- if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned long) ||
- ((unsigned long)ptr & 7))
- return -EFAULT;
-
- pagefault_disable();
- if (!__get_user_inatomic(*ret, ptr)) {
- pagefault_enable();
- return 0;
- }
- pagefault_enable();
-
- return read_user_stack_slow(ptr, ret, 8);
-}
-
-static inline int valid_user_sp(unsigned long sp, int is_64)
-{
- if (!sp || (sp & 7) || sp > (is_64 ? TASK_SIZE : 0x1UL) - 32)
- return 0;
- return 1;
-}
-
-/*
- * 64-bit user processes use the same stack frame for RT and non-RT signals.
- */
-struct signal_frame_64 {
- chardummy[__SIGNAL_FRAMESIZE];
- struct ucontext uc;
- unsigned long unused[2];
- unsigned inttramp[6];
- struct siginfo *pinfo;
- void*puc;
- struct siginfo info;
- charabigap[288];
-};
-
-static int is_sigreturn_64_address(unsigned long nip, unsigned long fp)
-{
- if (nip == fp + offsetof(struct signal_frame_64, tramp))
- return 1;
- if (vdso64_rt_sigtramp && current->mm->context.vdso_base &&
- nip == current->mm->context.vdso_base + vdso64_rt_sigtramp)
- return 1;
- return 0;
-}
-
-/*
- * Do some sanity checking on the signal frame pointed to by sp.
- * We check the pinfo and puc pointers in the frame.
- */
-static int sane_signal_64_frame(unsigned long sp)
-{
- struct signal_frame_64 __user *sf;
- unsigned long pinfo, puc;
-
- sf = (struct signal_frame_64 __user *) sp;
- if
