[10/10] powerpc: select DYNAMIC_DEBUG_RELATIVE_POINTERS for PPC64
diff mbox series

Message ID 20190409212517.7321-11-linux@rasmusvillemoes.dk
State In Next
Commit e1483eb592321fea5a99d68017761003d7f4be04
Headers show
Series
  • implement DYNAMIC_DEBUG_RELATIVE_POINTERS
Related show

Commit Message

Rasmus Villemoes April 9, 2019, 9:25 p.m. UTC
Similar to GENERIC_BUG_RELATIVE_POINTERS, one can now relativize the
four const char* members of struct _ddebug, thus saving 16 bytes per
instance (one for each pr_debug(), dev_debug() etc. in a
CONFIG_DYNAMIC_DEBUG kernel). The asm-generic implementation seems to
work out-of-the-box, though this is only compile-tested.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 arch/powerpc/Kconfig            | 1 +
 arch/powerpc/include/asm/Kbuild | 1 +
 2 files changed, 2 insertions(+)

Comments

Christophe Leroy April 23, 2019, 3:37 p.m. UTC | #1
Le 09/04/2019 à 23:25, Rasmus Villemoes a écrit :
> Similar to GENERIC_BUG_RELATIVE_POINTERS, one can now relativize the
> four const char* members of struct _ddebug, thus saving 16 bytes per
> instance (one for each pr_debug(), dev_debug() etc. in a
> CONFIG_DYNAMIC_DEBUG kernel). The asm-generic implementation seems to
> work out-of-the-box, though this is only compile-tested.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>   arch/powerpc/Kconfig            | 1 +
>   arch/powerpc/include/asm/Kbuild | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2d0be82c3061..6821c8ae1d62 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -155,6 +155,7 @@ config PPC
>   	select BUILDTIME_EXTABLE_SORT
>   	select CLONE_BACKWARDS
>   	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
> +	select DYNAMIC_DEBUG_RELATIVE_POINTERS	if PPC64

Why only PPC64 ? Won't it work with PPC32 ? Or would it be 
counter-performant on 32 bits ?

Christophe

>   	select DYNAMIC_FTRACE			if FUNCTION_TRACER
>   	select EDAC_ATOMIC_SCRUB
>   	select EDAC_SUPPORT
> diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
> index a0c132bedfae..f332e202192a 100644
> --- a/arch/powerpc/include/asm/Kbuild
> +++ b/arch/powerpc/include/asm/Kbuild
> @@ -3,6 +3,7 @@ generated-y += syscall_table_64.h
>   generated-y += syscall_table_c32.h
>   generated-y += syscall_table_spu.h
>   generic-y += div64.h
> +generic-y += dynamic_debug.h
>   generic-y += export.h
>   generic-y += irq_regs.h
>   generic-y += local64.h
>
Andrew Morton April 23, 2019, 7:36 p.m. UTC | #2
On Tue, 23 Apr 2019 17:37:33 +0200 Christophe Leroy <christophe.leroy@c-s.fr> wrote:

> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -155,6 +155,7 @@ config PPC
> >   	select BUILDTIME_EXTABLE_SORT
> >   	select CLONE_BACKWARDS
> >   	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
> > +	select DYNAMIC_DEBUG_RELATIVE_POINTERS	if PPC64
> 
> Why only PPC64 ? Won't it work with PPC32 ? Or would it be 
> counter-performant on 32 bits ?

Ditto arm and i386?
Rasmus Villemoes April 24, 2019, 6:46 a.m. UTC | #3
On 23/04/2019 21.36, Andrew Morton wrote:
> On Tue, 23 Apr 2019 17:37:33 +0200 Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> 
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -155,6 +155,7 @@ config PPC
>>>   	select BUILDTIME_EXTABLE_SORT
>>>   	select CLONE_BACKWARDS
>>>   	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
>>> +	select DYNAMIC_DEBUG_RELATIVE_POINTERS	if PPC64
>>
>> Why only PPC64 ? Won't it work with PPC32 ? Or would it be 
>> counter-performant on 32 bits ?
> 
> Ditto arm and i386?
> 

It's pointless on 32-bit platforms - I'm replacing absolute const char*
pointers with a relative s32 offset from the _ddebug descriptor, so if
sizeof(void*)==4 there's no gain.

And yes, the current implementation also wouldn't work out-of-the-box
for 32-bit platforms, since the asm needs to know how to properly
initialize a whole struct _ddebug, which (often) contains a static_key,
which in turn contains a pointer member, which both affects its size as
well as its placement inside _ddebug. The C code in dynamic_debug.c
would likely Just Work, but there's no point in complicating the asm
part for no gain, so there are static_assert()s in place to ensure
BITS_PER_LONG==64 (as well as checking the various offsetof()s etc.).

[I don't think performance matters at all, it's one extra addition to
access these fields, and that is only done in the rare cases where
someone interacts with the dynamic_debug/control sysfs file, and when
one of the activated pr_debug()s is actually hit (so a few extra
instructions drown in the printk overhead).]

I do now see that PPC64 does not select GENERIC_BUG_RELATIVE_POINTERS,
so maybe this scheme simply doesn't work on PPC64, or nobody has done
the work to reduce the sizeof(struct bug_entry) on PPC64? As I said,
I've only compile-tested arm64 and ppc64.

Rasmus

Patch
diff mbox series

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2d0be82c3061..6821c8ae1d62 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -155,6 +155,7 @@  config PPC
 	select BUILDTIME_EXTABLE_SORT
 	select CLONE_BACKWARDS
 	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
+	select DYNAMIC_DEBUG_RELATIVE_POINTERS	if PPC64
 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
 	select EDAC_ATOMIC_SCRUB
 	select EDAC_SUPPORT
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index a0c132bedfae..f332e202192a 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -3,6 +3,7 @@  generated-y += syscall_table_64.h
 generated-y += syscall_table_c32.h
 generated-y += syscall_table_spu.h
 generic-y += div64.h
+generic-y += dynamic_debug.h
 generic-y += export.h
 generic-y += irq_regs.h
 generic-y += local64.h