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
