mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Linus Torvalds <torvalds@linuxfoundation.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org, kees@kernel.org,
	acarmina@redhat.com, jpoimboe@kernel.org, mark.rutland@arm.com,
	maciej.wieczor-retman@intel.com
Subject: Re: [PATCH v2 00/12] x86: WARN() hackery
Date: Mon, 10 Nov 2025 23:20:44 +0100	[thread overview]
Message-ID: <20251110222044.GC278048@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <CAHk-=wj4gDw2iMi-OSbCov3SVDJCQr=YCRr3JZr9Mz-eNwfmGw@mail.gmail.com>

On Mon, Nov 10, 2025 at 08:18:02AM -0800, Linus Torvalds wrote:
> On Mon, 10 Nov 2025 at 03:58, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > We should probably just merge these, as they improve the code-gen for WARN()
> > significantly on x86
> 
> Yes..  And I'd actually like to see some example code generation in
> the commit messages just to make that "improve the code-gen" more
> explicit.

Damn, I actually thought to include some and then I totally forgot.

So lets see:

        if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
                      "corrupted preempt_count: %s/%d/0x%x\n",
                      current->comm, current->pid, preempt_count()))

#define WARN_ONCE(condition, format...)                         \
        DO_ONCE_LITE_IF(condition, WARN, 1, format)

#define WARN(condition, format...) ({                                   \
        int __ret_warn_on = !!(condition);                              \
        if (unlikely(__ret_warn_on))                                    \
                __WARN_printf(TAINT_WARN, format);                      \
        unlikely(__ret_warn_on);                                        \
})

#define __WARN_printf(taint, arg...) do {                               \
                instrumentation_begin();                                \
                __warn_printk(arg);                                     \
                __WARN_FLAGS("", BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
                instrumentation_end();                                  \
        } while (0)

Turns into this majestic pile:


# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        cmpl    $2, %ecx        #, _7
        jne     .L1472  #,

...

.L1472:
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        cmpb    $0, __already_done.11(%rip)     #, __already_done
        je      .L1513  #,

...

.L1513:
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        movb    $1, __already_done.11(%rip)     #, __already_done
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,


# 0 "" 2
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
#NO_APP
        movl    1424(%r14), %edx        # _15->pid, _15->pid
        leaq    1912(%r14), %rsi        #, _17
        movq    $.LC43, %rdi    #,
        call    __warn_printk   #
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,

# 0 "" 2
# 5093 "../kernel/sched/core.c" 1
        1:       .byte 0x0f, 0x0b ;
.pushsection __bug_table,"aw"
912:
        .pushsection .discard.annotate_data,"M", @progbits, 8
        .long 912b - .
        .long 1
        .popsection
        2:      .long 1b - .    # bug_entry::bug_addr
        .long .LC1 - .  # bug_entry::file       #
        .word 5093      # bug_entry::line       #
        .word 2313      # bug_entry::flags      #
        .org 2b + 12    #
.popsection
.pushsection .discard.annotate_insn,"M", @progbits, 8
        .long 1b - .
        .long 8		# ANNOTYPE_REACHABLE
        .popsection


While afterwards it looks like so:


# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        cmpl    $2, %ecx        #, _7
        jne     .L1442  #,

...

.L1442:
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
#APP
# 5093 "../kernel/sched/core.c" 1
        lea (2f)(%rip), %rdi    # bug
1:
.pushsection __bug_table,"aw"
        912:
        .pushsection .discard.annotate_data,"M", @progbits, 8
        .long 912b - .
        .long 1
        .popsection
        2:
        .long 1b - .    # bug_entry::bug_addr
        .long .LC43 - . # bug_entry::format     #
        .long .LC1 - .  # bug_entry::file       #
        .word 5093      # bug_entry::line       #
        .word 2323      # bug_entry::flags      #
        .org 2b + 16    #
.popsection
        
# 0 "" 2
#NO_APP  
        movl    1424(%r14), %edx        # pretmp_19->pid, pretmp_19->pid
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        leaq    1912(%r14), %rsi        #, _13
# ../kernel/sched/core.c:5093:  if (WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
        call    __SCT__WARN_trap        #

Where at runtime we patch that "call __SCT__WARN_trap" to read like:

	ud1 (%ecx), %rdi


The whole DO_ONCE_LITE_IF/__already_done thing is gone (replaced with
BUGFLAG_ONCE) and the external __warn_prink() call is also gone, now
inside the exception handler through the pt_regs->va_list magic.

I'll see if I can clean that up somewhat and stick in the changelog.

      reply	other threads:[~2025-11-10 22:20 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 11:46 Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 01/12] x86: Rework __bug_table helpers Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 02/12] bug: Add BUG_FORMAT infrastructure Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 03/12] bug: Clean up CONFIG_GENERIC_BUG_RELATIVE_POINTERS Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 04/12] bug: Add BUG_FORMAT_ARGS infrastructure Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 05/12] bug: Add report_bug_entry() Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 06/12] bug: Implement WARN_ON() using __WARN_FLAGS() Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 07/12] bug: Allow architectures to provide __WARN_printf() Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 08/12] x86/bug: Add BUG_FORMAT basics Peter Zijlstra
2025-11-25 11:17   ` Peter Zijlstra
2025-11-25 12:33     ` Peter Zijlstra
2025-11-25 15:17       ` Peter Zijlstra
2025-11-25 16:27         ` Linus Torvalds
2025-11-26  9:54           ` Peter Zijlstra
2025-11-26 10:56           ` Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 09/12] x86/bug: Use BUG_FORMAT for DEBUG_BUGVERBOSE_DETAILED Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 10/12] x86_64/bug: Implement __WARN_printf() Peter Zijlstra
2025-11-11  9:54   ` Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 11/12] x86/bug: Implement WARN_ONCE() Peter Zijlstra
2025-11-10 11:46 ` [PATCH v2 12/12] x86_64/bug: Inline the UD1 Peter Zijlstra
2025-11-10 16:18 ` [PATCH v2 00/12] x86: WARN() hackery Linus Torvalds
2025-11-10 22:20   ` Peter Zijlstra [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251110222044.GC278048@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acarmina@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=torvalds@linuxfoundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®