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 <pet...@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-...@intel.com/
