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

[Sebastian Andrzej Siewior]

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? 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…

> Reviewed-by: John Ogness <john.ogness@xxxxxxxxxxxxx>

Sebastian