Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1444536

Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20)

From Arnd Bergmann <arnd@arndb.de>
Newsgroups linux.kernel
Subject Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20)
Date 2016-07-15 21:50 +0200
Message-ID <rVhhv-2wf-1@gated-at.bofh.it> (permalink)
References <rV5q2-3U6-33@gated-at.bofh.it> <rVapI-6QL-25@gated-at.bofh.it> <rVblN-7q0-31@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Friday, July 15, 2016 3:23:05 PM CEST Sebastian Andrzej Siewior wrote:
> * Arnd Bergmann | 2016-07-15 14:29:31 [+0200]:
> 
> >Agreed, but it's hard for the compiler to figure that out. On mainline,
> 
> we have the same thing in drivers/tty/serial/8250/8250_port.c btw. Don't
> see the warning there.

Interesting, I also see no substantial difference, my best guess is
that the warning appears or disappears based on some inlining
choices that can vary.

I also notice that your "tty: serial: 8250: don't take the trylock
during oops" patch would apply on the pl011 driver as well.

> >we have this instead:
> >
> >       local_irq_save(flags);
> >       if (uap->port.sysrq)
> >               locked = 0;
> >       else if (oops_in_progress)
> >               locked = spin_trylock(&uap->port.lock);
> >       else
> >               spin_lock(&uap->port.lock);
> >…
> >       if (locked)
> >               spin_unlock(&uap->port.lock);
> >       local_irq_restore(flags);
> >
> >which looks like it's intentionally written to avoid the warning
> >and was changed by Thomas in "tty/serial/pl011: Make the locking work on RT":
> >
> >http://git.kernel.org/cgit/linux/kernel/git/rt/linux-stable-rt.git/commit/drivers/tty/serial/amba-pl011.c?h=v4.4-rt-rebase&id=7b537b66fcb12c40eb1b10eb352d570a9d34a657
> >
> >Maybe there is another way to write this upstream that avoids the
> >warning.
> 
> I don't see any other way around except for initializing flags upfront:

Sure, that always works, it's just a bit ugly since the flags word
should never be zero when it gets written back to the hardware irq
state, at least in portable code.

Maybe something like the version below?

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8a9e213387a7..676fcfa7fce4 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2158,22 +2158,9 @@ static void pl011_console_putchar(struct uart_port *port, int ch)
 }
 
 static void
-pl011_console_write(struct console *co, const char *s, unsigned int count)
+__pl011_console_write(struct uart_amba_port *uap, const char *s, unsigned int count)
 {
-	struct uart_amba_port *uap = amba_ports[co->index];
 	unsigned int old_cr = 0, new_cr;
-	unsigned long flags;
-	int locked = 1;
-
-	clk_enable(uap->clk);
-
-	local_irq_save(flags);
-	if (uap->port.sysrq)
-		locked = 0;
-	else if (oops_in_progress)
-		locked = spin_trylock(&uap->port.lock);
-	else
-		spin_lock(&uap->port.lock);
 
 	/*
 	 *	First save the CR then disable the interrupts
@@ -2195,10 +2182,24 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
 		cpu_relax();
 	if (!uap->vendor->always_enabled)
 		pl011_write(old_cr, uap, REG_CR);
+}
 
-	if (locked)
-		spin_unlock(&uap->port.lock);
-	local_irq_restore(flags);
+
+static void
+pl011_console_write(struct console *co, const char *s, unsigned int count)
+{
+	struct uart_amba_port *uap = amba_ports[co->index];
+	unsigned long flags;
+
+	clk_enable(uap->clk);
+
+	if (uap->port.sysrq || oops_in_progress) {
+		__pl011_console_write(uap, s, count);
+	} else {
+		spin_lock_irqsave(&uap->port.lock, flags);
+		__pl011_console_write(uap, s, count);
+		spin_unlock_irqrestore(&uap->port.lock, flags);
+	}
 
 	clk_disable(uap->clk);
 }

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread


Thread

Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Arnd Bergmann <arnd@arndb.de> - 2016-07-15 09:10 +0200
  Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-15 14:10 +0200
    Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Arnd Bergmann <arnd@arndb.de> - 2016-07-15 14:30 +0200
      Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-15 15:30 +0200
        Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Arnd Bergmann <arnd@arndb.de> - 2016-07-15 21:50 +0200

csiph-web