lkml.org 
[lkml]   [2019]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Regression in dbdda842fe96 ("printk: Add console owner and waiter logic to load balance console writes") [Was: Regression in fd5f7cde1b85 ("...")]
On (09/26/19 10:58), Petr Mladek wrote:
[..]
> > - spin_lock(&sport->port.lock);
> > -
> > + uart_port_lock_irqsave(&sport->port, flags);
>
> uart_port_lock_irqsave() does not exist.

... Oh. Good catch! Apparently I still carry around my patch set
which added printk_safe to TTY/UART locking API.

> Instead the current users do:
>
> spin_lock_irqsave(&port->lock, flags);

Right.

[..]

> I like this approach. It allows to remove hacks with locks.

[..]

> Or I would keep the locking as is and add some API
> just for the sysrq handling:
>
>
> int uart_store_sysrq_char(struct uart_port *port, unsigned int ch);
> unsigned int uart_get_sysrq_char(struct uart_port *port);

Looks good. We also probably can remove struct uart_port's
->sysrq member and clean up locking in drivers' ->write()
callbacks:

if (sport->sysrq)
locked = 0;
else if (oops_in_progress)
locked = spin_trylock_irqsave(&sport->lock, flags);
else
spin_lock_irqsave(&sport->lock, flags);

Because this ->sysrq branch makes driver completely lockless globally,
for all CPUs, not only for sysrq-CPU.

> And use it the following way:
>
> int handle_irq()
> {
> unsined int sysrq, sysrq_ch;
>
> spin_lock(&port->lock);
> [...]
> sysrq = uart_store_sysrq_char(port, ch);
> if (!sysrq)
> [...]
> [...]
>
> out:
> sysrq_ch = uart_get_sysrq_char(port);
> spin_unlock(&port->lock);
>
> if (sysrq_ch)
> handle_sysrq(sysrq_ch);
> }

Looks good.

-ss

\
 
 \ /
  Last update: 2019-09-27 06:26    [W:0.036 / U:7.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site