Re: [PATCH v2] printk: Use two irq_works instead per-CPU

[Petr Mladek]

On Wed 2026-09-23 09:55:12, Sebastian Andrzej Siewior wrote:
> On 2026-09-22 19:35:26 [+0200], John Ogness wrote:
> > > @@ -4643,12 +4637,11 @@ static void __wake_up_klogd(int val)
> > >  	 *
> > >  	 * This pairs with devkmsg_read:A and syslog_print:A.
> > >  	 */
> > > -	if (wq_has_sleeper(&log_wait) || /* LMM(__wake_up_klogd:A) */
> > > -	    (val & PRINTK_PENDING_OUTPUT)) {
> > > -		this_cpu_or(printk_pending, val);
> > > -		irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
> > > -	}
> > > -	preempt_enable();
> > > +	if (wq_has_sleeper(&log_wait)) /* LMM(__wake_up_klogd:A) */
> > > +		irq_work_queue(&pending_wakeup_work);
> > > +
> > > +	if (val & PRINTK_PENDING_OUTPUT)
> > > +		irq_work_queue(&pending_output_work);
> > 
> > The ordering of operations has been reverse queued. Perhaps because
> > irq_work is LIFO (implementation internal detail) and you wanted to
> > preserve the current ordering?

Great catch.

> > Or maybe this ordering was chosen because
> > the code looks nicer. Either way, I think it doesn't matter if the
> > legacy flushing occurs before/after waking the klogd waiter.
> 
> Hmm. I did not give much thinking into the ordering because it shouldn't
> matter. We used to have "unlock" followed by "wakeup" in the irq-work
> callback and this is what we have now given the LIFO ordering.
> Having "wakeup" first might not take effect immediately because the
> scheduler delays it or puts it on the current CPU and then it is delayed
> until after the interrupt ("unlock") is done. So…

Honestly, I am not sure what ordering is better. And it depends on
the internal irq_work implementation now. I would keep the patch
as is.

Best Regards,
Petr