From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751928AbcGOTtP (ORCPT ); Fri, 15 Jul 2016 15:49:15 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:52085 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751478AbcGOTtL convert rfc822-to-8bit (ORCPT ); Fri, 15 Jul 2016 15:49:11 -0400 From: Arnd Bergmann To: Sebastian Andrzej Siewior Cc: linaro-kernel@lists.linaro.org, Build bot for Mark Brown , kernel-build-reports@lists.linaro.org, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, Thomas Gleixner Subject: Re: v4.4.12-rt20 build: 0 failures 5 warnings (v4.4.12-rt20) Date: Fri, 15 Jul 2016 21:48:55 +0200 Message-ID: <77528485.4aTSS092zh@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-28-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20160715132305.GD11606@linutronix.de> References: <2319169.Jp9upmpu5F@wuerfel> <20160715132305.GD11606@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K0:l2ogn2dIztqTuzQcw+YDWYyUR834JH+87y1AZtoAcTY7au2mDu4 i9rQ7srZw2CN9wfDSfMs45+3TaKZ5VkPY1XfJxhNN2lsDCEVKzwVcfZf1Caakmgw+eyr7bg uKjF7tiKAK+xknH5QG8uekjXMwso+mzBREHol0mPnLmJGKLABO1TpjciiT6f+46hPIIbmz9 pPyhW/gF6TVg2cArmzsAg== X-UI-Out-Filterresults: notjunk:1;V01:K0:/iPr8cCFmVw=:Yq7cLNOjMmHlybTfqUJP0+ It1ymtTGDwDNXc4hvnzh9lY5Ihhxc4P5QlTgfJ4ndOPG8iumPIU8GMABXrkSAH6bNOBZH89DU 1KsYbRo94OB7QKjVSoC7b9nx8lLH7+LviaunTNRSXVfGSQIWbSpooqttwPF5d/Xkz0AiRVpMc yE3+iCydFZeTTC4mWq7L7ineX7eNUO/hKdN3iNOMzTl2xhdxJI9vsxnR56Z8+v2Rlv4xlwPH4 lzW1yfbJf8umk/ywxDR3fMSrgI0WTPzU78/4t2hffBXkPIagp3IBMlzKE8tkMz/XHuLifT6an tXzQMDn3Y8dmuWEQGk3pgXxlzHJqYOz4EQ0CAio33ZhTCiLtRXx4Sg4mGnYYM6tEvNgepvynY lw1OY6APz5KNdhJYviheAIWprFq80pUiM5MQd6p8uv3sIDIzB4cS8LVujhXL+M+YHWYz6WB3H 2tDWc+OP0F+O9ADdWUuW8mW3QqvLrBpGoDxiNQtCKZgNz7tg3KLrEMlnU582LqXrWU7ikFd3T ELZ+fe50FI3RZcF+a5dfiIbnnpkWa0XrwFKqTXd6LjkbDdA8vqfvj5miG6DkHCgJdgrurSgfo d5+/z8OF1FTCLI+xLOhniS5O1sPjkp/LZHqd6B4OIsW3E/NQ3sDXGpJuFgCFD5zp6ugNYRZPe zYHRWHaEMuxQNe5bp1ut1xtrdZJRX3C+pr9m88Qqag7aJa2TeWEi1mRI/hVEOpBZfMP1aiQzC dUjZJmIUFxuH/6gT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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); }