[PATCH 10/15] bugs/s390: Pass in 'cond_str' to __EMIT_BUG()

[PATCH 10/15] bugs/s390: Pass in 'cond_str' to __EMIT_BUG()

[Ingo Molnar]

Pass in the condition string from __WARN_FLAGS(), but do not
concatenate it with __FILE__, because the __bug_table is
apparently indexed by 16 bits and increasing string size
overflows it on defconfig builds.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: <linux-arch@vger.kernel.org>
---
 arch/s390/include/asm/bug.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index ef3e495ec1e3..30f8785a01f5 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,7 +8,7 @@
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 
-#define __EMIT_BUG(x) do {					\
+#define __EMIT_BUG(cond_str, x) do {				\
 	asm_inline volatile(					\
 		"0:	mc	0,0\n"				\
 		".section .rodata.str,\"aMS\",@progbits,1\n"	\
@@ -27,7 +27,7 @@
 
 #else /* CONFIG_DEBUG_BUGVERBOSE */
 
-#define __EMIT_BUG(x) do {					\
+#define __EMIT_BUG(cond_str, x) do {				\
 	asm_inline volatile(					\
 		"0:	mc	0,0\n"				\
 		".section __bug_table,\"aw\"\n"			\
@@ -42,12 +42,12 @@
 #endif /* CONFIG_DEBUG_BUGVERBOSE */
 
 #define BUG() do {					\
-	__EMIT_BUG(0);					\
+	__EMIT_BUG("", 0);				\
 	unreachable();					\
 } while (0)
 
 #define __WARN_FLAGS(cond_str, flags) do {		\
-	__EMIT_BUG(BUGFLAG_WARNING|(flags));		\
+	__EMIT_BUG(cond_str, BUGFLAG_WARNING|(flags));	\
 } while (0)
 
 #define WARN_ON(x) ({					\
-- 
2.45.2

Re: [PATCH 10/15] bugs/s390: Pass in 'cond_str' to __EMIT_BUG()

[Heiko Carstens]

On Thu, May 15, 2025 at 02:46:39PM +0200, Ingo Molnar wrote:
> Pass in the condition string from __WARN_FLAGS(), but do not
> concatenate it with __FILE__, because the __bug_table is
> apparently indexed by 16 bits and increasing string size
> overflows it on defconfig builds.

Could you provide your change which didn't work?

I cannot see how anything would overflow. Trying the below on top of
your series seems to work like expected.

In order to keep things easy this drops the mergeable section trick
and results in a small increase of the rodata section, but I doubt
that would explain what you have seen.

Also allyesconfig builds without errors.

diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 30f8785a01f5..837bfbde0c51 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -11,16 +11,14 @@
 #define __EMIT_BUG(cond_str, x) do {				\
 	asm_inline volatile(					\
 		"0:	mc	0,0\n"				\
-		".section .rodata.str,\"aMS\",@progbits,1\n"	\
-		"1:	.asciz	\""__FILE__"\"\n"		\
-		".previous\n"					\
 		".section __bug_table,\"aw\"\n"			\
-		"2:	.long	0b-.\n"				\
-		"	.long	1b-.\n"				\
-		"	.short	%0,%1\n"			\
-		"	.org	2b+%2\n"			\
+		"1:	.long	0b-.\n"				\
+		"	.long	%0-.\n"				\
+		"	.short	%1,%2\n"			\
+		"	.org	1b+%3\n"			\
 		".previous\n"					\
-		: : "i" (__LINE__),				\
+		: : "i" (WARN_CONDITION_STR(cond_str) __FILE__),\
+		    "i" (__LINE__),				\
 		    "i" (x),					\
 		    "i" (sizeof(struct bug_entry)));		\
 } while (0)

[PATCH 16/15] bugs/s390: Use in 'cond_str' to __EMIT_BUG()

[Ingo Molnar]

* Heiko Carstens <hca@linux.ibm.com> wrote:

> On Thu, May 15, 2025 at 02:46:39PM +0200, Ingo Molnar wrote:
> > Pass in the condition string from __WARN_FLAGS(), but do not
> > concatenate it with __FILE__, because the __bug_table is
> > apparently indexed by 16 bits and increasing string size
> > overflows it on defconfig builds.
> 
> Could you provide your change which didn't work?
> 
> I cannot see how anything would overflow. Trying the below on top of
> your series seems to work like expected.
> 
> In order to keep things easy this drops the mergeable section trick
> and results in a small increase of the rodata section, but I doubt
> that would explain what you have seen.
> 
> Also allyesconfig builds without errors.
> 
> diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
> index 30f8785a01f5..837bfbde0c51 100644
> --- a/arch/s390/include/asm/bug.h
> +++ b/arch/s390/include/asm/bug.h
> @@ -11,16 +11,14 @@
>  #define __EMIT_BUG(cond_str, x) do {				\
>  	asm_inline volatile(					\
>  		"0:	mc	0,0\n"				\
> -		".section .rodata.str,\"aMS\",@progbits,1\n"	\
> -		"1:	.asciz	\""__FILE__"\"\n"		\
> -		".previous\n"					\
>  		".section __bug_table,\"aw\"\n"			\
> -		"2:	.long	0b-.\n"				\
> -		"	.long	1b-.\n"				\
> -		"	.short	%0,%1\n"			\
> -		"	.org	2b+%2\n"			\
> +		"1:	.long	0b-.\n"				\
> +		"	.long	%0-.\n"				\
> +		"	.short	%1,%2\n"			\
> +		"	.org	1b+%3\n"			\
>  		".previous\n"					\
> -		: : "i" (__LINE__),				\
> +		: : "i" (WARN_CONDITION_STR(cond_str) __FILE__),\
> +		    "i" (__LINE__),				\
>  		    "i" (x),					\
>  		    "i" (sizeof(struct bug_entry)));		\
>  } while (0)

So I'm not sure what happened: I tried to reproduce what I did 
originally, but my naive patch ran into assembler build errors when a 
WARN_ON() macro tried to use the '%' C operator, such as 
fs/crypto/crypto.c:123:

 include/linux/compiler_types.h:497:20: error: invalid 'asm': invalid %-code
 arch/s390/include/asm/bug.h:12:2: note: in expansion of macro 'asm_inline'
 arch/s390/include/asm/bug.h:50:2: note: in expansion of macro '__EMIT_BUG'
 include/asm-generic/bug.h:119:3: note: in expansion of macro '__WARN_FLAGS'
 fs/crypto/crypto.c:123:6: note: in expansion of macro 'WARN_ON_ONCE'

Which corresponds to:

        if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
                return -EINVAL;

I'm quite sure I never saw these build errors - I saw linker errors 
related to the u16 overflow I documented in the changelog. (Note to 
self: copy & paste more of the build error context next time around.)

Your version doesn't have that build problem, so I picked it up with 
the changelog below and your Signed-off-by. Does that look good to you?

Thanks,

	Ingo

===================================>
From 7128294ca8b997efb1d85c7405c8c6e9af1a170d Mon Sep 17 00:00:00 2001
From: Heiko Carstens <hca@linux.ibm.com>
Date: Tue, 20 May 2025 15:39:27 +0200
Subject: [PATCH] bugs/s390: Use 'cond_str' in __EMIT_BUG()

In order to keep things easy this drops the mergeable section trick
and results in a small increase of the rodata section.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-arch@vger.kernel.org
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Link: https://lore.kernel.org/r/20250520133927.7932C19-hca@linux.ibm.com
---
 arch/s390/include/asm/bug.h | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 30f8785a01f5..837bfbde0c51 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -11,16 +11,14 @@
 #define __EMIT_BUG(cond_str, x) do {				\
 	asm_inline volatile(					\
 		"0:	mc	0,0\n"				\
-		".section .rodata.str,\"aMS\",@progbits,1\n"	\
-		"1:	.asciz	\""__FILE__"\"\n"		\
-		".previous\n"					\
 		".section __bug_table,\"aw\"\n"			\
-		"2:	.long	0b-.\n"				\
-		"	.long	1b-.\n"				\
-		"	.short	%0,%1\n"			\
-		"	.org	2b+%2\n"			\
+		"1:	.long	0b-.\n"				\
+		"	.long	%0-.\n"				\
+		"	.short	%1,%2\n"			\
+		"	.org	1b+%3\n"			\
 		".previous\n"					\
-		: : "i" (__LINE__),				\
+		: : "i" (WARN_CONDITION_STR(cond_str) __FILE__),\
+		    "i" (__LINE__),				\
 		    "i" (x),					\
 		    "i" (sizeof(struct bug_entry)));		\
 } while (0)

Re: [PATCH 16/15] bugs/s390: Use in 'cond_str' to __EMIT_BUG()

[Heiko Carstens]

On Mon, Jun 09, 2025 at 10:27:44AM +0200, Ingo Molnar wrote:
> So I'm not sure what happened: I tried to reproduce what I did 
> originally, but my naive patch ran into assembler build errors when a 
> WARN_ON() macro tried to use the '%' C operator, such as 
> fs/crypto/crypto.c:123:
> 
>  include/linux/compiler_types.h:497:20: error: invalid 'asm': invalid %-code
>  arch/s390/include/asm/bug.h:12:2: note: in expansion of macro 'asm_inline'
>  arch/s390/include/asm/bug.h:50:2: note: in expansion of macro '__EMIT_BUG'
>  include/asm-generic/bug.h:119:3: note: in expansion of macro '__WARN_FLAGS'
>  fs/crypto/crypto.c:123:6: note: in expansion of macro 'WARN_ON_ONCE'
> 
> Which corresponds to:
> 
>         if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
>                 return -EINVAL;
> 
> I'm quite sure I never saw these build errors - I saw linker errors 
> related to the u16 overflow I documented in the changelog. (Note to 
> self: copy & paste more of the build error context next time around.)
> 
> Your version doesn't have that build problem, so I picked it up with 
> the changelog below and your Signed-off-by. Does that look good to you?

Yes, fine with me.