RE: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
OK, resend without. BR Lionel -Original Message- From: Steven Rostedt [mailto:rost...@goodmis.org] Sent: mercredi 22 mars 2017 17:11 To: Lionel DEBIEVE Cc: Thomas Gleixner ; linux-rt-us...@vger.kernel.org; linux-kernel@vger.kernel.org; bige...@linutronix.de; Patrice CHOTARD ; Greg Kroah-Hartman ; Jiri Slaby Subject: Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware On Wed, 22 Mar 2017 08:43:50 + Lionel DEBIEVE wrote: > Just to agree with Thomas. > > Do you want me to resend the patch without RT tag? > Yes please. And hopefully it will be picked up in mainline. -- Steve
Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
On Wed, 22 Mar 2017 08:43:50 + Lionel DEBIEVE wrote: > Just to agree with Thomas. > > Do you want me to resend the patch without RT tag? > Yes please. And hopefully it will be picked up in mainline. -- Steve
Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
Just to agree with Thomas. Do you want me to resend the patch without RT tag? BR, Lionel On 03/21/2017 09:15 PM, Steven Rostedt wrote: > On Tue, 21 Mar 2017 19:51:47 +0100 (CET) > Thomas Gleixner wrote: > >> On Tue, 21 Mar 2017, Steven Rostedt wrote: /* * Disable interrupts so we don't get the IRQ line bouncing >>> I'm nervous about the above comment, which in full is: >>> >>> /* >>> * Disable interrupts so we don't get the IRQ line bouncing >>> * up and down while interrupts are disabled. >>> */ >>> >>> I'm not sure if disabling interrupts helps on an SMP system. This >>> patch does change what happens when port->sysrq is set. But I'm not >>> sure we care. >> It disables interrupts at the device level which obviously helps >> whether on SMP or not. >> > OK, so this has nothing to do with the local_irq_save() that is being > removed, which would be fine then. > > > -- Steve >
Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
On Tue, 21 Mar 2017 19:51:47 +0100 (CET) Thomas Gleixner wrote: > On Tue, 21 Mar 2017, Steven Rostedt wrote: > > > /* > > >* Disable interrupts so we don't get the IRQ line > > > bouncing > > > > I'm nervous about the above comment, which in full is: > > > > /* > > * Disable interrupts so we don't get the IRQ line bouncing > > * up and down while interrupts are disabled. > > */ > > > > I'm not sure if disabling interrupts helps on an SMP system. This > > patch does change what happens when port->sysrq is set. But I'm not > > sure we care. > > It disables interrupts at the device level which obviously helps > whether on SMP or not. > OK, so this has nothing to do with the local_irq_save() that is being removed, which would be fine then. -- Steve
Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
On Tue, 21 Mar 2017, Steven Rostedt wrote: > > /* > > * Disable interrupts so we don't get the IRQ line bouncing > > I'm nervous about the above comment, which in full is: > > /* >* Disable interrupts so we don't get the IRQ line bouncing >* up and down while interrupts are disabled. >*/ > > I'm not sure if disabling interrupts helps on an SMP system. This patch > does change what happens when port->sysrq is set. But I'm not sure we > care. It disables interrupts at the device level which obviously helps whether on SMP or not. Thanks, tglx
Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware
On Tue, 21 Mar 2017 17:05:40 +0100 wrote: > From: Lionel Debieve > > The lock is a sleeping lock and local_irq_save() is not the > standard implementation now. Working for both -RT and non > RT. If this is for both RT and non RT then the patch subject should just be [PATCH] and not [PATCH RT] as the latter tells upstream folks not to bother. > > Signed-off-by: Lionel Debieve > --- > drivers/tty/serial/st-asc.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c > index 379e5bd..1815423 100644 > --- a/drivers/tty/serial/st-asc.c > +++ b/drivers/tty/serial/st-asc.c > @@ -803,13 +803,12 @@ static void asc_console_write(struct console > *co, const char *s, unsigned count) int locked = 1; > u32 intenable; > > - local_irq_save(flags); > if (port->sysrq) > locked = 0; /* asc_interrupt has already claimed the > lock */ else if (oops_in_progress) > - locked = spin_trylock(&port->lock); > + locked = spin_trylock_irqsave(&port->lock, flags); > else > - spin_lock(&port->lock); > + spin_lock_irqsave(&port->lock, flags); > > /* >* Disable interrupts so we don't get the IRQ line bouncing I'm nervous about the above comment, which in full is: /* * Disable interrupts so we don't get the IRQ line bouncing * up and down while interrupts are disabled. */ I'm not sure if disabling interrupts helps on an SMP system. This patch does change what happens when port->sysrq is set. But I'm not sure we care. -- Steve > @@ -827,8 +826,7 @@ static void asc_console_write(struct console *co, > const char *s, unsigned count) asc_out(port, ASC_INTEN, intenable); > > if (locked) > - spin_unlock(&port->lock); > - local_irq_restore(flags); > + spin_unlock_irqrestore(&port->lock, flags); > } > > static int asc_console_setup(struct console *co, char *options)