On Tue, Mar 10, 2020 at 02:48:51PM +0000, Dmitry Safonov wrote:
> On 3/10/20 1:20 PM, Andy Shevchenko wrote:
> [..]
> > @@ -3286,22 +3286,20 @@ int uart_prepare_sysrq_char(struct uart_port *port, unsigned int ch)
> > }
> > EXPORT_SYMBOL_GPL(uart_prepare_sysrq_char);
> >
> > -void uart_unlock_and_check_sysrq(struct uart_port *port, unsigned long irqflags)
> > +void uart_unlock_and_check_sysrq(struct uart_port *port, unsigned long flags)
> > +__releases(&port->lock)
> > {
> > int sysrq_ch;
>
> Probably, you could move it inside the condition it's used.
I can do this.
> Though, I wonder why you decided to rearrange the code.
I hope commit message sheds a light on this. Main reason to (easily) see
how locks are being maintained.
> Otherwise, LGTM.
Thanks, I will send v2 when we get settlement on patch 1.
> > - if (!port->has_sysrq) {
> > - spin_unlock_irqrestore(&port->lock, irqflags);
> > - return;
> > + if (port->has_sysrq) {
> > + sysrq_ch = port->sysrq_ch;
> > + port->sysrq_ch = 0;
> > + spin_unlock_irqrestore(&port->lock, flags);
> > + if (sysrq_ch)
> > + handle_sysrq(sysrq_ch);
> > + } else {
> > + spin_unlock_irqrestore(&port->lock, flags);
> > }
> > -
> > - sysrq_ch = port->sysrq_ch;
> > - port->sysrq_ch = 0;
> > -
> > - spin_unlock_irqrestore(&port->lock, irqflags);
> > -
> > - if (sysrq_ch)
> > - handle_sysrq(sysrq_ch);
> > }
> > EXPORT_SYMBOL_GPL(uart_unlock_and_check_sysrq);
>
> Thanks,
> Dmitry
--
With Best Regards,
Andy Shevchenko
