Received-SPF: pass (google.com: domain of linux-kernel+bounces-71174-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: John Ogness <john.ogness@linutronix.de>
To: Waiman Long <longman@redhat.com>, Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>, Steven Rostedt
 <rostedt@goodmis.org>, Thomas Gleixner <tglx@linutronix.de>,
 linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>, Ingo
 Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>, Boqun Feng
 <boqun.feng@gmail.com>
Subject: Re: [PATCH printk v2 26/26] lockdep: Mark emergency section in
 lockdep splats
In-Reply-To: <6d8f5126-3595-4abd-9b05-b19a659d1cea@redhat.com>
References: <20240218185726.1994771-1-john.ogness@linutronix.de>
 <20240218185726.1994771-27-john.ogness@linutronix.de>
 <6d8f5126-3595-4abd-9b05-b19a659d1cea@redhat.com>
Date: Mon, 19 Feb 2024 12:17:20 +0106
Message-ID: <878r3gbtev.fsf@jogness.linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On 2024-02-18, Waiman Long <longman@redhat.com> wrote:
> On 2/18/24 13:57, John Ogness wrote:
> lockdep.c has multiple functions that print stuff to the console, like
>
>  =C2=A0- print_circular_bug_header()
>  =C2=A0- print_bad_irq_dependency()
>  =C2=A0- print_deadlock_bug()
>  =C2=A0- print_collision()
>  =C2=A0- print_usage_bug()
>  =C2=A0- print_irq_inversion_bug()
>  =C2=A0- print_lock_invalid_wait_context()
>  =C2=A0- print_lock_nested_lock_not_held()
>  =C2=A0- print_unlock_imbalance_bug()
>  =C2=A0- print_lock_contention_bug()
>  =C2=A0- print_freed_lock_bug()
>  =C2=A0- print_held_locks_bug()
>  =C2=A0- lockdep_rcu_suspicious()
>
> So what is special about print_usage_bug() that it needs this
> emergency treatment but not the other ones?

I do not expect to be able to identify all "emergency printing" paths in
the kernel from the beginning. This series initially marks some sections
that are IMHO interesting for the feature.

As you are implying, for lockdep probably all printing should be
considered emergency. Is it preferred to place the markers outside the
high-level print functions, for example:

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 00465373d358..7a4e4f4a9156 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2182,10 +2182,12 @@ check_noncircular(struct held_lock *src, struct hel=
d_lock *target,
 			*trace =3D save_trace();
 		}
=20
+		nbcon_cpu_emergency_enter();
 		if (src->class_idx =3D=3D target->class_idx)
 			print_deadlock_bug(current, src, target);
 		else
 			print_circular_bug(&src_entry, target_entry, src, target);
+		nbcon_cpu_emergency_exit();
 	}
=20
 	return ret;

Or is it preferred to put them directly around the various pr_warn()
blocks (as the patch in this series is doing)?

John Ogness