[09/10] arm64: select DYNAMIC_DEBUG_RELATIVE_POINTERS
diff mbox series

Message ID 20190409212517.7321-10-linux@rasmusvillemoes.dk
State In Next
Commit 9d9247d08d7269d9772bd5c3af9e642a3c1bd8bc
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/arm64/Kconfig            | 1 +
 arch/arm64/include/asm/Kbuild | 1 +
 2 files changed, 2 insertions(+)

Comments

Arnd Bergmann April 26, 2019, 9:39 a.m. UTC | #1
On Tue, Apr 9, 2019 at 11:26 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> 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>

This causes a build regression when compiling with clang,
see http://staging-storage.kernelci.org/next-clang/master/next-20190424/arm64/allmodconfig/clang-8/build.log

../init/initramfs.c:477:3: error: invalid operand in inline asm:
'.ifndef __UNIQUE_ID_ddebug18.pushsection __verbose,"aw".type
__UNIQUE_ID_ddebug18, STT_OBJECT.size __UNIQUE_ID_ddebug18,
${5:c}1:__UNIQUE_ID_ddebug18: .int ${0:c} - 1b /*
_ddebug::modname_disp */ .int ${1:c} - 1b /* _ddebug::function_disp */
.int ${2:c} - 1b /* _ddebug::filename_disp */ .int ${3:c} - 1b /*
_ddebug::format_disp */ .int ${4:c}      /* _ddebug::flags_lineno */
.org 1b+${5:c}.popsection.set __UNIQUE_ID_ddebug18.ddebug.once,
${6:c}.elseif __UNIQUE_ID_ddebug18.ddebug.once - ${6:c}.line 477 -
1.error "'__UNIQUE_ID_ddebug18' used as _ddebug identifier more than
once".endif'
                pr_debug("Detected %s compressed data\n", compress_name);
                ^
../include/linux/printk.h:336:2: note: expanded from macro 'pr_debug'
        dynamic_pr_debug(fmt, ##__VA_ARGS__)
        ^
../include/linux/dynamic_debug.h:158:2: note: expanded from macro
'dynamic_pr_debug'
        _dynamic_func_call(fmt, __dynamic_pr_debug,             \
        ^
../include/linux/dynamic_debug.h:148:2: note: expanded from macro
'_dynamic_func_call'
        __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
        ^
../include/linux/dynamic_debug.h:128:2: note: expanded from macro
'__dynamic_func_call'
        DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt);         \
        ^
../include/asm-generic/dynamic_debug.h:92:15: note: expanded from
macro 'DEFINE_DYNAMIC_DEBUG_METADATA'
        asm volatile(".ifndef " __stringify(name) "\n"                  \
                     ^

I assume the same thing happens on the other architectures as well.

        Arnd

> ---
>  arch/arm64/Kconfig            | 1 +
>  arch/arm64/include/asm/Kbuild | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 7e34b9eba5de..d0871d523d5d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -83,6 +83,7 @@ config ARM64
>         select CRC32
>         select DCACHE_WORD_ACCESS
>         select DMA_DIRECT_REMAP
> +       select DYNAMIC_DEBUG_RELATIVE_POINTERS
>         select EDAC_SUPPORT
>         select FRAME_POINTER
>         select GENERIC_ALLOCATOR
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index 1e17ea5c372b..1ead0d645686 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -3,6 +3,7 @@ generic-y += delay.h
>  generic-y += div64.h
>  generic-y += dma.h
>  generic-y += dma-contiguous.h
> +generic-y += dynamic_debug.h
>  generic-y += early_ioremap.h
>  generic-y += emergency-restart.h
>  generic-y += hw_irq.h
> --
> 2.20.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rasmus Villemoes April 26, 2019, 10:05 a.m. UTC | #2
On 26/04/2019 11.39, Arnd Bergmann wrote:
> On Tue, Apr 9, 2019 at 11:26 PM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>>
>> 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>
> 
> This causes a build regression when compiling with clang,
> see http://staging-storage.kernelci.org/next-clang/master/next-20190424/arm64/allmodconfig/clang-8/build.log

Yes, see also https://github.com/ClangBuiltLinux/linux/issues/456 .

The quickest short-term fix is to append "if CC_IS_GCC" to the select
statements. Then when a fix lands in clang one can change that to "if
CC_IS_GCC || CLANG_VERSION >= something". It's probably best if we fix
-next builds ASAP instead of waiting for knowing the proper value of
"something". Nathan, Nick, WDYT?

I had 0day verify my patches before sending them out officially, and
thought it also did clang builds. But apparently not, or not with enough
arch/.config combinations?

Rasmus
Nathan Chancellor April 26, 2019, 1 p.m. UTC | #3
On Fri, Apr 26, 2019 at 12:05:57PM +0200, Rasmus Villemoes wrote:
> On 26/04/2019 11.39, Arnd Bergmann wrote:
> > On Tue, Apr 9, 2019 at 11:26 PM Rasmus Villemoes
> > <linux@rasmusvillemoes.dk> wrote:
> >>
> >> 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>
> > 
> > This causes a build regression when compiling with clang,
> > see http://staging-storage.kernelci.org/next-clang/master/next-20190424/arm64/allmodconfig/clang-8/build.log
> 
> Yes, see also https://github.com/ClangBuiltLinux/linux/issues/456 .
> 
> The quickest short-term fix is to append "if CC_IS_GCC" to the select
> statements. Then when a fix lands in clang one can change that to "if
> CC_IS_GCC || CLANG_VERSION >= something". It's probably best if we fix
> -next builds ASAP instead of waiting for knowing the proper value of
> "something". Nathan, Nick, WDYT?

Those select statements have to be added regardless, we might as well do
it now. It should unbreak the auto builders because they use the latest
clang stable version, which is 8.0.0.

For the record, the fix in Clang is https://reviews.llvm.org/D60887 and
should land shortly unless there are any further objections, meaning
this will be fixed in the 9.0.0 release.

'if CC_IS_GCC || CLANG_VERSION >= 90000' should do it on the arm64 and
powerpc select statements (x86 works fine because %c support has always
been present).

> 
> I had 0day verify my patches before sending them out officially, and
> thought it also did clang builds. But apparently not, or not with enough
> arch/.config combinations?

They do not do clang builds unfortunately. Nick was in contact with them
before forced asm-goto on x86 happened and derailed that.

We try to do our own tests and let people know when stuff breaks but
that is usually after it hits -next, rather than the mailing list.

> 
> Rasmus

Cheers,
Nathan

Patch
diff mbox series

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7e34b9eba5de..d0871d523d5d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -83,6 +83,7 @@  config ARM64
 	select CRC32
 	select DCACHE_WORD_ACCESS
 	select DMA_DIRECT_REMAP
+	select DYNAMIC_DEBUG_RELATIVE_POINTERS
 	select EDAC_SUPPORT
 	select FRAME_POINTER
 	select GENERIC_ALLOCATOR
diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index 1e17ea5c372b..1ead0d645686 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -3,6 +3,7 @@  generic-y += delay.h
 generic-y += div64.h
 generic-y += dma.h
 generic-y += dma-contiguous.h
+generic-y += dynamic_debug.h
 generic-y += early_ioremap.h
 generic-y += emergency-restart.h
 generic-y += hw_irq.h