Hello Peter, thanks for the clear explanation. I finally understand what was bothering you, it wasn’t the __bug_table size or the execution time overhead, but the code size itself.
On Fri, May 30, 2025 at 4:02 PM Peter Zijlstra peterz@infradead.org wrote:
+Mark because he loves a hack :-)
On Thu, May 29, 2025 at 12:36:55PM +0200, Alessandro Carminati wrote:
Like I said before; you need to do this on the report_bug() size of things.
I fully understand your concerns, and I truly appreciate both yours and Josh’s feedback on this matter. Please rest assured that I took your suggestions seriously and carefully evaluated the possibility of consolidating all related logic within the exception handler. After a thorough investigation, however, I encountered several limitations that led me to maintain the check in the macro. I’d like to share the rationale behind this decision:
- In the case of WARN() messages, part of the output, the
user-specified content, is emitted directly by the macro, prior to reaching the exception handler [1]. Moving the check solely to the exception handler would not prevent this early output.
Yeah, this has been really annoying me for a long while. WARN() code gen is often horrible crap because of that.
Everything I've tried so far is worse though :/ So in the end I try to never use WARN(), its just not worth it.
... /me goes down the rabbit-hole again, because well, you can't let something simple like this defeat you ;-)
Results of today's hackery below. It might actually be worth cleaning up.
- Unless we change the user-facing interface that allows suppression
based on function names, we still need to work with those names at runtime.
I'm not sure I understand this. What interface and what names? This is a new feature, so how can there be an interface that needs to be preserved?
- This leaves us with two main strategies: converting function names
to pointers (e.g., via kallsyms) or continuing to work with names. The former requires name resolution at suppression time and pointer comparison in the handler, but function names are often altered by the compiler due to inlining or other optimizations[2]. Some WARN() sites are even marked __always_inline[3], making it difficult to prevent inlining altogether.
Arguably __func__ should be the function name of the function you get inlined into. C inlining does not preserve the sequence point, so there is absolutely no point in trying to preserve the inline name.
I'm again confused though; [2] does not use __func__ at all.
Anyway, when I do something like:
void __attribute__((__always_inline__)) foo(void) { puts(__func__); }
void bar(void) { foo(); }
it uses a "foo" string, which IMO is just plain wrong. Anyway, do both compilers guarantee it will always be foo? I don't think I've seen the GCC manual be explicit about this case.
On this point: even if not explicitly stated, __func__ will always be "foo" in your example. I’m confident in this because, according to the GCC manual[1], __func__ is inserted into the source as: static const char __func__[] = "function-name"; At that point, the compiler doesn't yet know what the final code will look like or whether it will be inlined. I’m not a compiler expert, but this definition doesn’t leave much room for alternative interpretations.
- An alternative is to embed function names in the __bug_table. While potentially workable, this increases the size of the table and
requires attention to handle position-independent builds (-fPIC/-fPIE), such as using offsets relative to __start_rodata.
However, the central challenge remains: any logic that aims to suppress WARN() output must either move the entire message emission into the exception handler or accept that user-specified parts of the message will still be printed.
Well, we can set suppress_printk and then all is quiet :-) Why isn't this good enough?
As a secondary point, there are also less common architectures where it's unclear whether suppressing these warnings is a priority, which might influence how broadly the effort is applied. I hoped to have addressed the concern of having faster runtime, by exposing a counter that could skip the logic. Kess suggested using static branching that would make things even better. Could Kess' suggestion mitigate your concern on this strategy? I’m absolutely open to any further thoughts or suggestions you may have, and I appreciate your continued guidance.
I'm not really concerned with performance here, but more with the size of the code emitted by WARN_ONCE(). There are a *ton* of WARN sites, while only one report_bug() and printk().
The really offensive thing is that this is for a feature most nobody will ever need :/
The below results in:
03dc 7ac: 48 c7 c0 00 00 00 00 mov $0x0,%rax 7af: R_X86_64_32S .rodata.str1.1+0x223 03e3 7b3: ba 2a 00 00 00 mov $0x2a,%edx 03e8 7b8: 48 0f b9 d0 ud1 %rax,%rdx
And it even works :-)
Hmm... I should try and stick the format string into the __bug_table, its const after all. Then I can get 2 arguments covered.
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index f0e9acf72547..88b305d49f35 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h
The rework is impressive. I’m not in a position to judge, but for what it’s worth, you have my admiration.
That said, I see a problem, the same I faced when embedding __func__ in the bug table. __func__, and I believe also printk fmt, are constants, but their pointers might not be, especially in position-independent code. This causes issues depending on the compiler. For example, my tests worked fine with recent GCC on x86_64, but failed with Clang. Changing architecture complicates things further, GCC added support for this only in newer versions. I ran into this in v3 when the s390x build failed[2].
As mentioned in the patchset cover letter, my solution to avoid copying the strings into the bug table is to store their pointer as an offset from __start_rodata. [1]. https://gcc.gnu.org/onlinedocs/gcc/Function-Names.html [2]. https://lore.kernel.org/all/202503200847.LbkIJIXa-lkp@intel.com/