Received-SPF: pass (google.com: domain of linux-kernel+bounces-185057-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 21 May 2024 16:03:48 +0200
From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Neeraj Upadhyay <quic_neeraju@quicinc.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang.zhang1211@gmail.com>, rcu@vger.kernel.org
Subject: Re: [PATCH printk v5 29/30] rcu: Mark emergency sections in rcu
 stalls
Message-ID: <ZkypxKBSPPGIo9Se@pathway.suse.cz>
References: <20240502213839.376636-1-john.ogness@linutronix.de>
 <20240502213839.376636-30-john.ogness@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240502213839.376636-30-john.ogness@linutronix.de>

On Thu 2024-05-02 23:44:38, John Ogness wrote:
> Mark emergency sections wherever multiple lines of
> rcu stall information are generated. In an emergency
> section the CPU will not perform console output for the
> printk() calls. Instead, a flushing of the console
> output is triggered when exiting the emergency section.
> This allows the full message block to be stored as
> quickly as possible in the ringbuffer.
>
> --- a/kernel/rcu/tree_exp.h
> +++ b/kernel/rcu/tree_exp.h
> @@ -612,6 +616,7 @@ static void synchronize_rcu_expedited_wait(void)
>  			}
>  			pr_cont("\n");
>  		}
> +		nbcon_cpu_emergency_flush();

It would make more sense to do the flush inside the cycle after each
dump_cpu_task(). Something like:

		rcu_for_each_leaf_node(rnp) {
			for_each_leaf_node_possible_cpu(rnp, cpu) {
				mask = leaf_node_cpu_bit(rnp, cpu);
				if (!(READ_ONCE(rnp->expmask) & mask))
					continue;
				preempt_disable(); // For smp_processor_id() in dump_cpu_task().
				dump_cpu_task(cpu);
				preempt_enable();
+				nbcon_cpu_emergency_flush();
			}
			rcu_exp_print_detail_task_stall_rnp(rnp);
		}


Or maybe, it is limited onto to few CPUs by rcu_for_each_leaf_node(rnp)?


>  		rcu_for_each_leaf_node(rnp) {
>  			for_each_leaf_node_possible_cpu(rnp, cpu) {
>  				mask = leaf_node_cpu_bit(rnp, cpu);

Otherwise, it looks good to me.

Best Regards,
Petr