Tree/Branch: v4.4.12-rt20 Git describe: v4.4.12-rt20 Commit: b4059f165a Linux 4.4.12-rt20
Build Time: 79 min 2 sec
Passed: 9 / 9 (100.00 %) Failed: 0 / 9 ( 0.00 %)
Errors: 0 Warnings: 5 Section Mismatches: 0
------------------------------------------------------------------------------- defconfigs with issues (other than build errors): 1 warnings 0 mismatches : arm64-allnoconfig 2 warnings 0 mismatches : arm64-allmodconfig 1 warnings 0 mismatches : arm-multi_v5_defconfig 2 warnings 0 mismatches : arm-multi_v7_defconfig 1 warnings 0 mismatches : x86_64-defconfig 3 warnings 0 mismatches : arm-allmodconfig 1 warnings 0 mismatches : arm-allnoconfig 1 warnings 0 mismatches : x86_64-allnoconfig 2 warnings 0 mismatches : arm64-defconfig
-------------------------------------------------------------------------------
Warnings Summary: 5 9 ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function] 2 ../include/linux/spinlock.h:246:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized] 1 ../lib/lz4/lz4hc_compress.c:514:1: warning: the frame size of 1472 bytes is larger than 1024 bytes [-Wframe-larger-than=] 1 ../drivers/xen/balloon.c:155:13: warning: 'release_memory_resource' declared 'static' but never defined [-Wunused-function] 1 ../drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1032 bytes is larger than 1024 bytes [-Wframe-larger-than=]
=============================================================================== Detailed per-defconfig build reports below:
------------------------------------------------------------------------------- arm64-allnoconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
------------------------------------------------------------------------------- arm64-allmodconfig : PASS, 0 errors, 2 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function] ../drivers/xen/balloon.c:155:13: warning: 'release_memory_resource' declared 'static' but never defined [-Wunused-function]
------------------------------------------------------------------------------- arm-multi_v5_defconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
------------------------------------------------------------------------------- arm-multi_v7_defconfig : PASS, 0 errors, 2 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function] ../include/linux/spinlock.h:246:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
------------------------------------------------------------------------------- x86_64-defconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
------------------------------------------------------------------------------- arm-allmodconfig : PASS, 0 errors, 3 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function] ../lib/lz4/lz4hc_compress.c:514:1: warning: the frame size of 1472 bytes is larger than 1024 bytes [-Wframe-larger-than=] ../drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1032 bytes is larger than 1024 bytes [-Wframe-larger-than=]
------------------------------------------------------------------------------- arm-allnoconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
------------------------------------------------------------------------------- x86_64-allnoconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
------------------------------------------------------------------------------- arm64-defconfig : PASS, 0 errors, 2 warnings, 0 section mismatches
Warnings: ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function] ../include/linux/spinlock.h:246:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized] -------------------------------------------------------------------------------
Passed with no errors, warnings or mismatches:
On Friday, July 15, 2016 5:25:17 AM CEST Build bot for Mark Brown wrote:
Tree/Branch: v4.4.12-rt20 Git describe: v4.4.12-rt20 Commit: b4059f165a Linux 4.4.12-rt20
Build Time: 79 min 2 sec
Passed: 9 / 9 (100.00 %) Failed: 0 / 9 ( 0.00 %)
Errors: 0 Warnings: 5 Section Mismatches: 0
defconfigs with issues (other than build errors): 1 warnings 0 mismatches : arm64-allnoconfig 2 warnings 0 mismatches : arm64-allmodconfig 1 warnings 0 mismatches : arm-multi_v5_defconfig 2 warnings 0 mismatches : arm-multi_v7_defconfig 1 warnings 0 mismatches : x86_64-defconfig 3 warnings 0 mismatches : arm-allmodconfig 1 warnings 0 mismatches : arm-allnoconfig 1 warnings 0 mismatches : x86_64-allnoconfig 2 warnings 0 mismatches : arm64-defconfig
Cc linux-rt-users@vger.kernel.org, let's have a look at the individual warnings:
Warnings Summary: 5 9 ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
This was introduced by the rt patchset, I'd suggest adding a fix to the next v4.4-rt release. This is almost certainly harmless.
2 ../include/linux/spinlock.h:246:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
I'm missing context here, so I don't know what caused it, but the warning is not present in v4.4.12 (without -rt), so it's worth looking into.
It happens for both arm-multi_v7_defconfig and arm64-defconfig.
1 ../lib/lz4/lz4hc_compress.c:514:1: warning: the frame size of 1472 bytes is larger than 1024 bytes [-Wframe-larger-than=] 1 ../drivers/xen/balloon.c:155:13: warning: 'release_memory_resource' declared 'static' but never defined [-Wunused-function] 1 ../drivers/mtd/chips/cfi_cmdset_0020.c:651:1: warning: the frame size of 1032 bytes is larger than 1024 bytes [-Wframe-larger-than=]
These three are present in v4.4.12 and only got fixed upstream later. I have some hope that the fixes will make it into stable kernels eventually. The "frame size" warnings are a side-effect of a gcc problem with gcov profiling, and the xen warning was introduced by a stable backport patch.
Arnd
* Arnd Bergmann | 2016-07-15 09:07:20 [+0200]:
Cc linux-rt-users@vger.kernel.org, let's have a look at the individual warnings:
thanks.
Warnings Summary: 5 9 ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
This was introduced by the rt patchset, I'd suggest adding a fix to the next v4.4-rt release. This is almost certainly harmless.
has been adressed in 4.6.4-rt8 and is harmless.
2 ../include/linux/spinlock.h:246:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]I'm missing context here, so I don't know what caused it, but the warning is not present in v4.4.12 (without -rt), so it's worth looking into.
the full log:
|In file included from include/linux/seqlock.h:35:0, | from include/linux/time.h:5, | from include/uapi/linux/timex.h:56, | from include/linux/timex.h:56, | from include/linux/sched.h:19, | from arch/arm64/include/asm/compat.h:25, | from arch/arm64/include/asm/stat.h:23, | from include/linux/stat.h:5, | from include/linux/module.h:10, | from drivers/tty/serial/amba-pl011.c:37: |drivers/tty/serial/amba-pl011.c: In function ‘pl011_console_write’: |include/linux/spinlock.h:246:3: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized] | _raw_spin_unlock_irqrestore(lock, flags); \ | ^ |drivers/tty/serial/amba-pl011.c:2065:16: note: ‘flags’ was declared here | unsigned long flags; | ^
and relevant part of the source: |pl011_console_write(struct console *co, const char *s, unsigned int count) |{ … | unsigned long flags; | int locked = 1; | … | if (uap->port.sysrq) | locked = 0; | else if (oops_in_progress) | locked = spin_trylock_irqsave(&uap->port.lock, flags); | else | spin_lock_irqsave(&uap->port.lock, flags); … | if (locked) | spin_unlock_irqrestore(&uap->port.lock, flags); … |}
looks like a false positive to me.
Arnd
Sebastian
On Friday, July 15, 2016 2:05:27 PM CEST Sebastian Andrzej Siewior wrote:
- Arnd Bergmann | 2016-07-15 09:07:20 [+0200]:
Cc linux-rt-users@vger.kernel.org, let's have a look at the individual warnings:
thanks.
Warnings Summary: 5 9 ../kernel/sched/core.c:3473:12: warning: 'preemptible_lazy' defined but not used [-Wunused-function]
This was introduced by the rt patchset, I'd suggest adding a fix to the next v4.4-rt release. This is almost certainly harmless.
has been adressed in 4.6.4-rt8 and is harmless.
Ok.
|drivers/tty/serial/amba-pl011.c: In function ‘pl011_console_write’: |include/linux/spinlock.h:246:3: warning: ‘flags’ may be used uninitialized in this function [-Wmaybe-uninitialized] | _raw_spin_unlock_irqrestore(lock, flags); \ | ^ |drivers/tty/serial/amba-pl011.c:2065:16: note: ‘flags’ was declared here | unsigned long flags; | ^
and relevant part of the source: |pl011_console_write(struct console *co, const char *s, unsigned int count) |{ … | unsigned long flags; | int locked = 1; | … | if (uap->port.sysrq) | locked = 0; | else if (oops_in_progress) | locked = spin_trylock_irqsave(&uap->port.lock, flags); | else | spin_lock_irqsave(&uap->port.lock, flags); … | if (locked) | spin_unlock_irqrestore(&uap->port.lock, flags); … |}
looks like a false positive to me.
Agreed, but it's hard for the compiler to figure that out. On mainline, 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/dr...
Maybe there is another way to write this upstream that avoids the warning.
Arnd
* 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.
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/dr...
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:
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2161,18 +2161,17 @@ pl011_console_write(struct console *co, 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; + unsigned long flags = 0; 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); + locked = spin_trylock_irqsave(&uap->port.lock, flags); else - spin_lock(&uap->port.lock); + spin_lock_irqsave(&uap->port.lock, flags);
/* * First save the CR then disable the interrupts @@ -2196,8 +2195,7 @@ pl011_console_write(struct console *co, const char *s, unsigned int count) pl011_write(old_cr, uap, REG_CR);
if (locked) - spin_unlock(&uap->port.lock); - local_irq_restore(flags); + spin_unlock_irqrestore(&uap->port.lock, flags);
clk_disable(uap->clk); }
Arnd
Sebastian
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/dr...
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); }
* Arnd Bergmann | 2016-07-15 21:48:55 [+0200]:
I also notice that your "tty: serial: 8250: don't take the trylock during oops" patch would apply on the pl011 driver as well.
That one. That is something I am not really sure about in the long run. On -RT we can't try_lock() with IRQs off and that is why I removed it. You could do the same with pl011 but you are screwed anyway because clk_enable() will take a sleeping lock and that is a no no.
So you could stay with the try_lock because it does not solve anything. In the long run I though about a console flag which denotes the console as RT-IRQ save which is the case for 8250 but not for pl011 (due to clk_enable()) so should not get on -RT into this case where it matters.
On the other hand if you oops on !RT+UP in your uart driver while holding the lock then the try_lock will fail resulting in a lockdep splat (because try_lock should not fail on UP). So might want take it as a procation in that case :)
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.
yes :)
Maybe something like the version below?
sure.
Sebastian
kernel-build-reports@lists.linaro.org
-
Arnd Bergmann -
Build bot for Mark Brown -
Sebastian Andrzej Siewior