Re: [RFC 6/8] x86_64/bug: Implement __WARN_printf()
From: Peter Zijlstra
Date: Mon Jun 02 2025 - 11:57:27 EST
On Mon, Jun 02, 2025 at 08:02:24AM -0700, Linus Torvalds wrote:
> On Mon, 2 Jun 2025 at 07:52, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > Use the normal COUNT_ARGS() trick to split the variadic WARN() macro
> > into per nr_args sub-marcos, except use a custom mapping such that 4
> > and above map to another variadic that does the current thing as
> > fallback.
>
> Does this horror work with clang? Because I suspect not. The games you
> play with inline asm are too disgusting for words.
Yes, it absolutely builds with clang. The inline asm isn't something we
don't already do elsewhere :-) *cough* extable *cough*
> But honestly, even if it does,I really hate this kind of insane
> complexity for dubious reasons.
>
> If you have a warning that is *so* critical for performance that you
> can't deal with the register movement that comes from the compiler
> doing this for you, just remove the warning.
>
> Don't make our build system do something this disgusting.
It isn't just the register movement. What we currently have for WARN()
is:
WARN(cond, fmt, arg...)
if (unlikely(cond)) {
__warn_printf(fmt, arg);
ud2
}
Where the UD2 bug_entry will have NO_CUT_HERE, because __warn_printf()
will have started the printing.
Part of the problem is with unlikely() not causing the text to break out
into .text.unlikely, but at best it moves it to the end of whatever
function we're in.
This also means that if you do WARN_ONCE() the whole ONCE machinery is
also dumped into that function.
And now someone wants to go add some KUnit specific testing to this as
well, which is also dumped into the function.
This is all cruft that shouldn't be in the normal .text.
The horrors I did will change things into:
if (unlikely(cond))
ud1 regs
where the bug_entry will then have the fmt, and the ud1 instruction some
regs (provided 3 or less args). Then all the ONCE and KUnit crap can
live in the exception handler. Not littered around the real code.
Now, I can still relate to: "this is too horrible for words". But then I
would strongly suggest people go poke at the compilers so we can get:
if (really_unlikely(cond)) {
whatever;
}
such that the compiler is forced to split whatever into a cold
sub-function placed in .text.unlikely. Then it doesn't really matter how
much crap we stick in there, it will not affect the normal code paths /
I$.
Anyway, I had fun hacking this up :-)