Re: [PATCH v2] kcsan: Don't enter an nbcon emergency section for verbose reports

[John Ogness <john.ogness@linutronix.de>]

On 2026-11-05, Petr Mladek <pmladek@suse.com> wrote:
> On Wed 2026-11-04 15:48:06, John Ogness wrote:
>> On 2026-11-04, Breno Leitao <leitao@debian.org> wrote:
>> >> Wouldn't the below diff be the proper fix?
>> >> 
>> >> John
>> >> 
>> >> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
>> >> index c3dc84a7cef26..71f1b4b8f06d5 100644
>> >> --- a/kernel/locking/lockdep.c
>> >> +++ b/kernel/locking/lockdep.c
>> >> @@ -4245,8 +4245,6 @@ void print_irqtrace_events(struct task_struct *curr)
>> >>  {
>> >>  	const struct irqtrace_events *trace = &curr->irqtrace;
>> >>  
>> >> -	nbcon_cpu_emergency_enter();
>> >> -
>> >>  	printk("irq event stamp: %u\n", trace->irq_events);
>> >>  	printk("hardirqs last  enabled at (%u): [<%px>] %pS\n",
>> >>  		trace->hardirq_enable_event, (void *)trace->hardirq_enable_ip,
>> >> @@ -4260,8 +4258,6 @@ void print_irqtrace_events(struct task_struct *curr)
>> >>  	printk("softirqs last disabled at (%u): [<%px>] %pS\n",
>> >>  		trace->softirq_disable_event, (void *)trace->softirq_disable_ip,
>> >>  		(void *)trace->softirq_disable_ip);
>> >> -
>> >> -	nbcon_cpu_emergency_exit();
>> >>  }
>> >>  
>> >>  static int HARDIRQ_verbose(struct lock_class *class)
>> >> @@ -5797,8 +5793,11 @@ static noinstr void check_flags(unsigned long flags)
>> >>  	}
>> >>  #endif
>> >>  
>> >> -	if (!debug_locks)
>> >> +	if (!debug_locks) {
>> >> +		nbcon_cpu_emergency_enter();
>> >>  		print_irqtrace_events(current);
>> >> +		nbcon_cpu_emergency_exit();
>> >> +	}
>> >>  
>> >>  	instrumentation_end();
>> >>  #endif
>> >
>> > Yes, but it was unclear to me what were the invocations of
>> > print_irqtrace_events() that needed the emergency turned on.
>> >
>> > how did you decided that only check_flags() needed it?
>> 
>> The emergency enter/exit's are intended for lockdep splats. AFAICT
>> check_flags() was the only call to print_irqtrace_events() that was not
>> already in emergency state for lockdep splats.
>>
>> KCSAN is not the only non-lockdep caller of print_irqtrace_events(). So
>> my suggested fix applies to the other non-lockdep users as well. Commit
>> 59cd94ef8009 ("lockdep: Mark emergency sections in lockdep splats") was
>> only intended for lockdep splats.
>
> I agree with this reasoning. Well, more messages might be printed
> in check_flags(). IMHO, we should print them all inside
> nbcon_cpu_emergency_enter()/exit(). I mean:
>
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -5769,6 +5769,7 @@ static noinstr void check_flags(unsigned long flags)
>  
>  	/* Get the warning out..  */
>  	instrumentation_begin();
> +	nbcon_cpu_emergency_enter();
>  
>  	if (irqs_disabled_flags(flags)) {
>  		if (DEBUG_LOCKS_WARN_ON(lockdep_hardirqs_enabled())) {
> @@ -5800,6 +5801,7 @@ static noinstr void check_flags(unsigned long flags)
>  	if (!debug_locks)
>  		print_irqtrace_events(current);
>  
> +	nbcon_cpu_emergency_exit();
>  	instrumentation_end();
>  #endif
>  }

I had thought about this. But there really is no need for this because
DEBUG_LOCKS_WARN_ON() performs a WARN(). Also, I was concerned about
nbcon_cpu_emergency_enter() causing some side-effects to the
flag-checking.

> And there seems to be one more lockdep-caller of
> print_irqtrace_events():
>
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -4649,6 +4649,8 @@ void lockdep_cleanup_dead_cpu(unsigned int cpu, struct task_struct *idle)
>  	if (unlikely(!debug_locks))
>  		return;
>  
> +	nbcon_cpu_emergency_enter();
> +
>  	if (unlikely(per_cpu(hardirqs_enabled, cpu))) {
>  		pr_warn("CPU %u left hardirqs enabled!", cpu);
>  		if (idle)
> @@ -4656,6 +4658,8 @@ void lockdep_cleanup_dead_cpu(unsigned int cpu, struct task_struct *idle)
>  		/* Clean it up for when the CPU comes online again. */
>  		per_cpu(hardirqs_enabled, cpu) = 0;
>  	}
> +
> +	nbcon_cpu_emergency_enter();
>  }
>  
>  static int

I also had evaluated this. It is not a lockdep splat, but rather a
warning (not WARN) about inproper cleanup.

John