* RE: SATA hang on 8315E triggered by heavy flash write?
From: Xie Shaohui-B21989 @ 2013-05-23 6:04 UTC (permalink / raw)
To: Anthony Foiani; +Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <g8v362qg8.fsf@dworkin.scrye.com>
Hi, Anthony Foiani,
Thanks for the confirmation.=20
So it seems the NOR write break the signal Integrity of SATA.
I don't have schematic and board right now, could you please measure signal=
s related to NOR write to see if anything abnormal? Is the board use FPGA o=
r CPLD to control signal?
If stop NOR write, could the SATA recover and work?
Best Regards,=20
Shaohui Xie
> -----Original Message-----
> From: Anthony Foiani [mailto:tkil@scrye.com]
> Sent: Thursday, May 23, 2013 1:52 PM
> To: Xie Shaohui-B21989
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org
> Subject: Re: SATA hang on 8315E triggered by heavy flash write?
>=20
>=20
> Shaohui --
>=20
> Thanks for the quick reply! Please find my investigation and results
> below.
>=20
> Xie Shaohui-B21989 <B21989@freescale.com> writes:
>=20
> > 1. only update NOR for a long enough time, for ex. tens of seconds,
> > see if error happens;
>=20
> It seems that I can do this without any errors:
>=20
> / # flash_erase /dev/mtd1 0 0
> Erasing 64 Kibyte @ 7f0000 -- 100 % complete
> / # dd if=3D/dev/zero of=3D/dev/mtd1
> dd: writing '/dev/mtd1': No space left on device
> 16385+0 records in
> 16384+0 records out
> 8388608 bytes (8.0MB) copied, 62.399439 seconds, 131.3KB/s
>=20
> > 2. only r/w SSD without NOR operation, see if error happens;
>=20
> Again, no problem:
>=20
> /ssd # ls -al biggie.bin
> -rw-r--r-- 1 root root 2330607084 May 22 19:34 biggie.bin
> /ssd # ls -alh biggie.bin
> -rw-r--r-- 1 root root 2.2G May 22 19:34 biggie.bin
> /ssd # time cp biggie.bin biggie2.bin
> real 3m 27.55s
> user 0m 2.60s
> sys 2m 16.13s
>=20
> > 3. r/w SSD first and keep it run, then start to read NOR, if no
> > error for a long time, then start to write NOR, see how long the
> > error will happen.
>=20
> Doing a NOR read during heavy SATA r/w seems to succeed, with no errors
> on the console:
>=20
> [window 1]
> /ssd # time cp biggie.bin biggie2.bin
>=20
> [window 2]
> / # dd if=3D/dev/mtd1 of=3D/dev/null
> 16384+0 records in
> 16384+0 records out
> 8388608 bytes (8.0MB) copied, 6.380613 seconds, 1.3MB/s
>=20
> Doing a NOR write fails almost instantly (within a second):
>=20
> [window 1]
> /ssd # time cp biggie.bin biggie2.bin
>=20
> [window 2]
> / # dd if=3D/dev/zero of=3D/dev/mtd1
>=20
> [console]
> [ 5160.269106] ata2.00: exception Emask 0x10 SAct 0x0 SErr 0x0 action
> 0x6 frozen
> [ 5160.276387] ata2.00: failed command: READ DMA
> [ 5160.280905] ata2.00: cmd c8/00:00:60:f3:01/00:00:00:00:00/e0 tag 0
> dma 131072 in
> [ 5160.280928] res 50/00:00:f0:c0:48/00:00:00:00:00/e0 Emask
> 0x10 (ATA bus error)
> [ 5160.296386] ata2.00: status: { DRDY }
> [ 5160.300195] ata2: hard resetting link
> [ 5160.347858] ata2: setting speed (in hard reset)
> [ 5170.439981] ata2: No Signature Update
> [ 5170.611901] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [ 5170.618204] ata2.00: link online but device misclassified
> [ 5175.623918] ata2.00: qc timeout (cmd 0xec)
> [ 5175.628147] ata2.00: failed to IDENTIFY (I/O error, err_mask=3D0x4)
> [ 5175.634347] ata2.00: revalidation failed (errno=3D-5)
> [ 5175.639373] ata2: hard resetting link
> [ 5176.143847] ata2: Hardreset failed, not off-lined 0
> [ 5176.155867] ata2: setting speed (in hard reset)
> [ 5185.743871] ata2: No Signature Update
> [ 5185.915900] ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 310)
> [ 5185.922203] ata2.00: link online but device misclassified
> [ 5195.927910] ata2.00: qc timeout (cmd 0xec)
> [ 5195.932140] ata2.00: failed to IDENTIFY (I/O error, err_mask=3D0x4)
> [ 5195.938342] ata2.00: revalidation failed (errno=3D-5)
> [ 5195.943430] ata2: hard resetting link
> [ 5196.443885] ata2: Hardreset failed, not off-lined 0
> ...
>=20
> At this point, a hard reset / full power cycle is needed to recover.
>=20
> The board is an MPC8315ERDB derivative, and I'm running a patched
> 3.4.36 kernel.
>=20
> I've uploaded some (possibly) relevant files to:
>=20
> http://foiani.home.dyndns.org/~tony/linux/ppc-sata-issues-201305/
>=20
> There is a diff from 3.4.36, a devtree, and a kernel config.
>=20
> Please let me know if there is any more information that I can contribute=
.
>=20
> Best regards,
> Anthony Foiani
^ permalink raw reply
* [PATCH] ASoC: fsl: expand the size of the name in fsl_ssi_private struct
From: Nicolin Chen @ 2013-05-23 6:26 UTC (permalink / raw)
To: linuxppc-dev, alsa-devel; +Cc: broonie, timur
strcpy(ssi_private->name, p) in probe() sets "name" by "p", gotten from dts,
while the length of "p", if the devicetree node name of SSI is commonly set,
would always be larger than 1 char size, so need a larger size for "name".
Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
sound/soc/fsl/fsl_ssi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2f2d837..b6a5f94 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -152,5 +152,5 @@ struct fsl_ssi_private {
} stats;
- char name[1];
+ char name[32];
};
--
1.7.1
^ permalink raw reply related
* [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Chen Gang @ 2013-05-23 7:57 UTC (permalink / raw)
To: Russell King - ARM Linux, hskinnemoen, egtvedt, Mike Frysinger,
ysato, rkuo, jejb, deller, Benjamin Herrenschmidt,
paulus@samba.org, schwidefsky, heiko.carstens, linux390, lethal,
jdike, richard, Thomas Gleixner, mingo@redhat.com, hpa, x86,
Arnd Bergmann, Eric W. Biederman, Serge Hallyn, paulmck,
Frederic Weisbecker, David Miller, Andrew Morton, akinobu.mita,
Catalin Marinas, walken, Will Deacon, Geert Uytterhoeven
Cc: linux-arch, linux-s390, user-mode-linux-devel, linux-parisc,
linux-sh, linux-hexagon, linux-kernel@vger.kernel.org,
user-mode-linux-user, uclinux-dist-devel,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
The crazy user can unset 'CONFIG_BUG' in menuconfig: "> General setup >
Configure standard kernel features (expert users) > BUG() Support".
But in fact, we always need it, and quite a few of architectures have
already implemented it (e.g. alpha, arc, arm, avr32, blackfin, cris,
frv, ia64, m68k, mips, mn10300, parisc, powerpc, s390, sh, sparc, x86).
And kernel also already has prepared a default effective implementation
for the architectures which is unwilling to implement it by themselves
(e.g. arm64, c6x, h8300, hexagon, m32r, metag, microblaze, openrisc,
score, tile, um, unicore32, xtensa).
So need get rid of 'CONFIG_BUG', and let it always enabled everywhere.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
arch/arm/Kconfig | 1 -
arch/avr32/Kconfig | 1 -
arch/blackfin/Kconfig | 1 -
arch/h8300/Kconfig | 1 -
arch/hexagon/Kconfig | 1 -
arch/parisc/Kconfig | 2 --
arch/powerpc/Kconfig | 1 -
arch/s390/Kconfig | 2 +-
arch/sh/Kconfig | 2 +-
arch/um/Kconfig.common | 1 -
arch/x86/Kconfig | 1 -
include/asm-generic/bug.h | 29 -----------------------------
init/Kconfig | 10 ----------
lib/Kconfig.debug | 2 +-
14 files changed, 3 insertions(+), 52 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a7fc5ea..ea4a146 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -265,7 +265,6 @@ config PHYS_OFFSET
config GENERIC_BUG
def_bool y
- depends on BUG
source "init/Kconfig"
diff --git a/arch/avr32/Kconfig b/arch/avr32/Kconfig
index bdc3558..7c9005a 100644
--- a/arch/avr32/Kconfig
+++ b/arch/avr32/Kconfig
@@ -55,7 +55,6 @@ config GENERIC_CALIBRATE_DELAY
config GENERIC_BUG
def_bool y
- depends on BUG
source "init/Kconfig"
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index a117652..637dc42 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -47,7 +47,6 @@ config GENERIC_CSUM
config GENERIC_BUG
def_bool y
- depends on BUG
config ZONE_DMA
def_bool y
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 303e4f9..88848da 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -56,7 +56,6 @@ config GENERIC_CALIBRATE_DELAY
config GENERIC_BUG
bool
- depends on BUG
config TIME_LOW_RES
bool
diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 33a9792..f50cc8f 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -84,7 +84,6 @@ config STACKTRACE_SUPPORT
config GENERIC_BUG
def_bool y
- depends on BUG
menu "Machine selection"
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 6507dab..5de1f8c 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -10,7 +10,6 @@ config PARISC
select RTC_CLASS
select RTC_DRV_GENERIC
select INIT_ALL_POSSIBLE
- select BUG
select HAVE_PERF_EVENTS
select GENERIC_ATOMIC64 if !64BIT
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
@@ -62,7 +61,6 @@ config ARCH_HAS_ILOG2_U64
config GENERIC_BUG
bool
default y
- depends on BUG
config GENERIC_HWEIGHT
bool
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c33e3ad..34f4ca9 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -187,7 +187,6 @@ config AUDIT_ARCH
config GENERIC_BUG
bool
default y
- depends on BUG
config SYS_SUPPORTS_APM_EMULATION
default y if PMAC_APM_EMU
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index da183c5..5d7b3db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -29,7 +29,7 @@ config GENERIC_HWEIGHT
def_bool y
config GENERIC_BUG
- def_bool y if BUG
+ def_bool y
config GENERIC_BUG_RELATIVE_POINTERS
def_bool y
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 8c868cf..d555e7f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -84,7 +84,7 @@ config RWSEM_XCHGADD_ALGORITHM
config GENERIC_BUG
def_bool y
- depends on BUG && SUPERH32
+ depends on SUPERH32
config GENERIC_CSUM
def_bool y
diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
index bceee66..7aae42a 100644
--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -53,7 +53,6 @@ config GENERIC_CALIBRATE_DELAY
config GENERIC_BUG
bool
default y
- depends on BUG
config HZ
int
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 723e42e..a36e1b4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -166,7 +166,6 @@ config GENERIC_ISA_DMA
config GENERIC_BUG
def_bool y
- depends on BUG
select GENERIC_BUG_RELATIVE_POINTERS if X86_64
config GENERIC_BUG_RELATIVE_POINTERS
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..5d50903 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -12,8 +12,6 @@
#ifndef __ASSEMBLY__
#include <linux/kernel.h>
-#ifdef CONFIG_BUG
-
#ifdef CONFIG_GENERIC_BUG
struct bug_entry {
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
@@ -106,33 +104,6 @@ extern void warn_slowpath_null(const char *file, const int line);
unlikely(__ret_warn_on); \
})
-#else /* !CONFIG_BUG */
-#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
-#endif
-
-#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
-#endif
-
-#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#ifndef WARN
-#define WARN(condition, format...) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#define WARN_TAINT(condition, taint, format...) WARN_ON(condition)
-
-#endif
-
#define WARN_ON_ONCE(condition) ({ \
static bool __section(.data.unlikely) __warned; \
int __ret_warn_once = !!(condition); \
diff --git a/init/Kconfig b/init/Kconfig
index 7fb26a6..bc1dd49 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1360,16 +1360,6 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
-config BUG
- bool "BUG() support" if EXPERT
- default y
- help
- Disabling this option eliminates support for BUG and WARN, reducing
- the size of your kernel image and potentially quietly ignoring
- numerous fatal conditions. You should only consider disabling this
- option for embedded systems with no facilities for reporting errors.
- Just say Y.
-
config ELF_CORE
depends on COREDUMP
default y
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 566cf2b..54b3251 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -700,7 +700,7 @@ config HAVE_DEBUG_BUGVERBOSE
config DEBUG_BUGVERBOSE
bool "Verbose BUG() reporting (adds 70K)" if DEBUG_KERNEL && EXPERT
- depends on BUG && (GENERIC_BUG || HAVE_DEBUG_BUGVERBOSE)
+ depends on GENERIC_BUG || HAVE_DEBUG_BUGVERBOSE
default y
help
Say Y here to make BUG() panics output the file name and line number
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Geert Uytterhoeven @ 2013-05-23 8:40 UTC (permalink / raw)
To: Chen Gang
Cc: Catalin Marinas, Linux-sh list, Heiko Carstens, paulus@samba.org,
H. Peter Anvin, Michel Lespinasse, Hans-Christian Egtvedt,
Linux-Arch, linux-s390, Russell King - ARM Linux, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <519DCBEF.3090208@asianux.com>
On Thu, May 23, 2013 at 9:57 AM, Chen Gang <gang.chen@asianux.com> wrote:
> The crazy user can unset 'CONFIG_BUG' in menuconfig: "> General setup >
> Configure standard kernel features (expert users) > BUG() Support".
>
> But in fact, we always need it, and quite a few of architectures have
Sorry, but we don't. I think you don't get the meaning of the BUG config symbol
(see below).
> already implemented it (e.g. alpha, arc, arm, avr32, blackfin, cris,
> frv, ia64, m68k, mips, mn10300, parisc, powerpc, s390, sh, sparc, x86).
What do you mean by "already implemented it"? E.g. on m68k, I can disable
or enable CONFIG_BUG. Both work.
> And kernel also already has prepared a default effective implementation
> for the architectures which is unwilling to implement it by themselves
> (e.g. arm64, c6x, h8300, hexagon, m32r, metag, microblaze, openrisc,
> score, tile, um, unicore32, xtensa).
This is not about providing an implementation or not...
> -config BUG
> - bool "BUG() support" if EXPERT
> - default y
> - help
> - Disabling this option eliminates support for BUG and WARN, reducing
> - the size of your kernel image and potentially quietly ignoring
> - numerous fatal conditions. You should only consider disabling this
> - option for embedded systems with no facilities for reporting errors.
> - Just say Y.
... It's about reducing memory size on devices where you can't show bug or
warning messages.
> So need get rid of 'CONFIG_BUG', and let it always enabled everywhere.
So please keep it.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] ASoC: fsl: expand the size of the name in fsl_ssi_private struct
From: Nicolin Chen @ 2013-05-23 8:51 UTC (permalink / raw)
To: linuxppc-dev, alsa-devel; +Cc: broonie, timur
strcpy(ssi_private->name, p) in probe() sets "name" by "p", gotten from dts,
while the length of "p", if the devicetree node name of SSI is commonly set,
would always be larger than 1 char size, so need a larger size for "name".
Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
sound/soc/fsl/fsl_ssi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2f2d837..b6a5f94 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -152,5 +152,5 @@ struct fsl_ssi_private {
} stats;
- char name[1];
+ char name[32];
};
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Arnd Bergmann @ 2013-05-23 8:54 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390,
Russell King - ARM Linux, Yoshinori Sato, Richard Weinberger,
Helge Deller, the arch/x86 maintainers, James E.J. Bottomley,
mingo@redhat.com, Frederic Weisbecker, Paul McKenney,
Håvard Skinnemoen, Serge Hallyn, Mike Frysinger, uml-devel,
Will Deacon, Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <CAMuHMdU7QuzgmWCH145p8PVebBzPo8DBAvbY+0AZa2cmGXmRHw@mail.gmail.com>
On Thursday 23 May 2013, Geert Uytterhoeven wrote:
> > -config BUG
> > - bool "BUG() support" if EXPERT
> > - default y
> > - help
> > - Disabling this option eliminates support for BUG and WARN, reducing
> > - the size of your kernel image and potentially quietly ignoring
> > - numerous fatal conditions. You should only consider disabling this
> > - option for embedded systems with no facilities for reporting errors.
> > - Just say Y.
>
> ... It's about reducing memory size on devices where you can't show bug or
> warning messages.
>
> > So need get rid of 'CONFIG_BUG', and let it always enabled everywhere.
>
> So please keep it.
Agreed. The one annoying property of disabling BUG() support is that it causes
a large number of build warnings since the compiler now has to assume that a lot
of code is reachable when it is normally annotate as unreachable.
When I do "randconfig" tests, I always turn on CONFIG_BUG because of this.
Arnd
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Russell King - ARM Linux @ 2013-05-23 9:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <CAMuHMdU7QuzgmWCH145p8PVebBzPo8DBAvbY+0AZa2cmGXmRHw@mail.gmail.com>
On Thu, May 23, 2013 at 10:40:29AM +0200, Geert Uytterhoeven wrote:
> On Thu, May 23, 2013 at 9:57 AM, Chen Gang <gang.chen@asianux.com> wrote:
> > -config BUG
> > - bool "BUG() support" if EXPERT
> > - default y
> > - help
> > - Disabling this option eliminates support for BUG and WARN, reducing
> > - the size of your kernel image and potentially quietly ignoring
> > - numerous fatal conditions. You should only consider disabling this
> > - option for embedded systems with no facilities for reporting errors.
> > - Just say Y.
>
> ... It's about reducing memory size on devices where you can't show bug or
> warning messages.
And turning off CONFIG_BUG causes lots of warning messages at compile time
about functions which are returning nothing which shouldn't.
The problem is: trying to fix that _will_ mean the result is a larger
kernel than if you just do the usual arch-implemented thing of placing
an defined faulting instruction at the BUG() site - which defeats the
purpose of turning off CONFIG_BUG.
Therefore, it's better that CONFIG_BUG always be y and we stop kidding
ourselves that it's possible to turn this off and safely save space.
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Geert Uytterhoeven @ 2013-05-23 9:12 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <20130523090534.GJ18614@n2100.arm.linux.org.uk>
On Thu, May 23, 2013 at 11:05 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, May 23, 2013 at 10:40:29AM +0200, Geert Uytterhoeven wrote:
>> On Thu, May 23, 2013 at 9:57 AM, Chen Gang <gang.chen@asianux.com> wrote:
>> > -config BUG
>> > - bool "BUG() support" if EXPERT
>> > - default y
>> > - help
>> > - Disabling this option eliminates support for BUG and WARN, reducing
>> > - the size of your kernel image and potentially quietly ignoring
>> > - numerous fatal conditions. You should only consider disabling this
>> > - option for embedded systems with no facilities for reporting errors.
>> > - Just say Y.
>>
>> ... It's about reducing memory size on devices where you can't show bug or
>> warning messages.
>
> And turning off CONFIG_BUG causes lots of warning messages at compile time
> about functions which are returning nothing which shouldn't.
>
> The problem is: trying to fix that _will_ mean the result is a larger
> kernel than if you just do the usual arch-implemented thing of placing
> an defined faulting instruction at the BUG() site - which defeats the
> purpose of turning off CONFIG_BUG.
Is __builtin_unreachable() working well these days?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Arnd Bergmann @ 2013-05-23 9:39 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390,
Russell King - ARM Linux, Yoshinori Sato, Richard Weinberger,
Helge Deller, the arch/x86 maintainers, James E.J. Bottomley,
mingo@redhat.com, Frederic Weisbecker, Paul McKenney,
Håvard Skinnemoen, Serge Hallyn, Mike Frysinger, uml-devel,
Will Deacon, Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <CAMuHMdWez-j1Maa3BD7ucmzv0_zFJnChERQiHFmkCaZUzG0_AA@mail.gmail.com>
On Thursday 23 May 2013, Geert Uytterhoeven wrote:
> > The problem is: trying to fix that will mean the result is a larger
> > kernel than if you just do the usual arch-implemented thing of placing
> > an defined faulting instruction at the BUG() site - which defeats the
> > purpose of turning off CONFIG_BUG.
>
> Is __builtin_unreachable() working well these days?
>
Hmm, I just tried the trivial patch below, which seemed to do the right thing.
Needs a little more investigation, but that might actually be the correct
solution. I thought that at some point __builtin_unreachable() was the same
as "do {} while (1)", but this is not the case with the gcc I was using --
it just tells gcc that we don't expect to ever get here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..9afff7d 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -108,11 +108,11 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
+#define BUG() __builtin_unreachable ()
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (condition) __builtin_unreachable(); } while(0)
#endif
#ifndef HAVE_ARCH_WARN_ON
^ permalink raw reply related
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Chen Gang @ 2013-05-23 10:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Catalin Marinas, Linux-sh list, Heiko Carstens, paulus@samba.org,
H. Peter Anvin, Michel Lespinasse, Hans-Christian Egtvedt,
Linux-Arch, linux-s390, Russell King - ARM Linux, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <CAMuHMdWez-j1Maa3BD7ucmzv0_zFJnChERQiHFmkCaZUzG0_AA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]
On 05/23/2013 05:12 PM, Geert Uytterhoeven wrote:
> On Thu, May 23, 2013 at 11:05 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> > On Thu, May 23, 2013 at 10:40:29AM +0200, Geert Uytterhoeven wrote:
>>> >> On Thu, May 23, 2013 at 9:57 AM, Chen Gang <gang.chen@asianux.com> wrote:
>>>> >> > -config BUG
>>>> >> > - bool "BUG() support" if EXPERT
>>>> >> > - default y
>>>> >> > - help
>>>> >> > - Disabling this option eliminates support for BUG and WARN, reducing
>>>> >> > - the size of your kernel image and potentially quietly ignoring
>>>> >> > - numerous fatal conditions. You should only consider disabling this
>>>> >> > - option for embedded systems with no facilities for reporting errors.
>>>> >> > - Just say Y.
>>> >>
>>> >> ... It's about reducing memory size on devices where you can't show bug or
>>> >> warning messages.
>> >
>> > And turning off CONFIG_BUG causes lots of warning messages at compile time
>> > about functions which are returning nothing which shouldn't.
>> >
>> > The problem is: trying to fix that _will_ mean the result is a larger
>> > kernel than if you just do the usual arch-implemented thing of placing
>> > an defined faulting instruction at the BUG() site - which defeats the
>> > purpose of turning off CONFIG_BUG.
> Is __builtin_unreachable() working well these days?
In fact, using __builtin_unreachable() is a standard way for
architectures to implemented their own BUG() (e.g. x86, s390, powerpc,
arm ...)
Before __builtin_unreachable(), must need an inline asm instruction
which architecture specific.
I have test using __builtin_unreachable() without an related asm
instruction before, it prints many unexpected things (please see the
attachment).
So I think, it is not suitable to use it in "asm-generic/bug.h"
Thanks.
--
Chen Gang
Asianux Corporation
[-- Attachment #2: test0.c --]
[-- Type: text/plain, Size: 1234 bytes --]
#include <stdio.h>
#include <sys/types.h>
#include <error.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
int main()
{
int file;
int ret;
char buf[0x100];
file = open("/tmp/work.c", O_RDONLY);
if (file == -1) {
printf("\nopen file failed. errno = %d\n", errno);
goto err;
} else
printf("\nopen file succeed.\n");
printf("before unreachable\n");
__builtin_unreachable();
printf("after unreachable\n");
if (lseek(file, 10, SEEK_END) < 0) {
printf("\nlseek file failed. errno = %d\n", errno);
goto err;
}
ret = read(file, buf, 0x100);
if (ret < 0) {
printf("\n1st read file failed. errno = %d, ret = %d\n", errno, ret);
goto err;
} else
printf("\n1st read file succeed. errno = %d, ret = %d\n", errno, ret);
ret = read(file, buf, 0x100);
if (ret < 0) {
printf("\n2nd read file failed. errno = %d, ret = %d\n", errno, ret);
goto err;
} else
printf("\n2nd read file succeed. errno = %d, ret = %d\n", errno, ret);
ret = read(file, buf, 0x100);
if (ret < 0) {
printf("\n2rd read file failed. errno = %d, ret = %d\n", errno, ret);
goto err;
} else
printf("\n3rd read file succeed. errno = %d, ret = %d\n", errno, ret);
return 0;
err:
return -1;
}
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Russell King - ARM Linux @ 2013-05-23 10:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <201305231139.38233.arnd@arndb.de>
On Thu, May 23, 2013 at 11:39:37AM +0200, Arnd Bergmann wrote:
> On Thursday 23 May 2013, Geert Uytterhoeven wrote:
> > > The problem is: trying to fix that will mean the result is a larger
> > > kernel than if you just do the usual arch-implemented thing of placing
> > > an defined faulting instruction at the BUG() site - which defeats the
> > > purpose of turning off CONFIG_BUG.
> >
> > Is __builtin_unreachable() working well these days?
> >
>
> Hmm, I just tried the trivial patch below, which seemed to do the right thing.
> Needs a little more investigation, but that might actually be the correct
> solution. I thought that at some point __builtin_unreachable() was the same
> as "do {} while (1)", but this is not the case with the gcc I was using --
> it just tells gcc that we don't expect to ever get here.
All this is doing is hiding the warning, nothing more.
What the compiler does is this:
beq 1f
... some asm code ...
__builtin_reachable() point
maybe a literal table
1: ... some asm code doing some other part of the function ...
and what will happen is that the first block of asm will fall through the
(possibly present) literal table into the following asm code. So, as
specified in the gcc manual, if you ever hit a __builtin_unreachable()
point, your program is undefined (as in, the behaviour of it can no longer
be known.)
We can't make that guarantee with BUG() - because sometimes they do fire
and sometimes in the most unlikely scenarios, particularly if you're not
looking, or at the most inconvenient time.
So, if you want to use this, then you should update the CONFIG_BUG text
to include a warning to this effect:
Warning: if CONFIG_BUG is turned off, and control flow reaches
a BUG(), the system behaviour will be undefined.
so that people can make an informed choice about this, because at the
moment:
Disabling this option eliminates support for BUG and WARN, reducing
the size of your kernel image and potentially quietly ignoring
numerous fatal conditions. You should only consider disabling this
option for embedded systems with no facilities for reporting errors.
Just say Y.
will become completely misleading. Turning this option off will _not_
result in "quietly ignoring numerous fatal conditions".
And I come back to one of my previous arguments - is it not better to
panic() if we hit one of these conditions so that the system can try to
do a panic-reboot rather than continue blindly into the unknown?
^ permalink raw reply
* RE: [PATCH] ASoC: fsl: expand the size of the name in fsl_ssi_private struct
From: David Laight @ 2013-05-23 10:21 UTC (permalink / raw)
To: Nicolin Chen, linuxppc-dev, alsa-devel; +Cc: broonie, timur
In-Reply-To: <1369299072-25147-1-git-send-email-b42378@freescale.com>
> strcpy(ssi_private->name, p) in probe() sets "name" by "p", gotten =
from dts,
> while the length of "p", if the devicetree node name of SSI is =
commonly set,
> would always be larger than 1 char size, so need a larger size for =
"name".
Are you sure this isn't allowed for when the structure is allocated?
Otherwise you also need to use strlcpy() as well.
> @@ -152,5 +152,5 @@ struct fsl_ssi_private {
> } stats;
>=20
> - char name[1];
> + char name[32];
> };
This looks like what c99 allows 'char name[0]' be used for.
But ensure the \0 is allowed for before changing to 1 to 0.
David
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Russell King - ARM Linux @ 2013-05-23 10:29 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Geert Uytterhoeven, Frederic Weisbecker, Paul McKenney,
Håvard Skinnemoen, Serge Hallyn, Mike Frysinger,
Arnd Bergmann, Will Deacon, Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
linux-hexagon, Martin Schwidefsky, linux390, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <878v369fdd.fsf@xmission.com>
On Thu, May 23, 2013 at 03:09:50AM -0700, Eric W. Biederman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
> > On Thursday 23 May 2013, Geert Uytterhoeven wrote:
> >> > The problem is: trying to fix that will mean the result is a larger
> >> > kernel than if you just do the usual arch-implemented thing of placing
> >> > an defined faulting instruction at the BUG() site - which defeats the
> >> > purpose of turning off CONFIG_BUG.
> >>
> >> Is __builtin_unreachable() working well these days?
> >>
> >
> > Hmm, I just tried the trivial patch below, which seemed to do the right thing.
> > Needs a little more investigation, but that might actually be the correct
> > solution. I thought that at some point __builtin_unreachable() was the same
> > as "do {} while (1)", but this is not the case with the gcc I was using --
> > it just tells gcc that we don't expect to ever get here.
>
> Yes.
>
> We already have this abstracted in compiler.h as the macro unreachable,
> so the slight modification of your patch below should handle this case.
>
> For compilers without __builtin_unreachable() unreachable() expands to
> do {} while(1) but an infinite loop seems reasonable and preserves the
> semantics of the code, unlike the current noop that is do {} while(0).
Semantics of the code really don't come in to it if you use unreachable().
unreachable() is an effective do { } while (0) to the compiler. It just
doesn't warn about it anymore. It's actually worse than that - it's
permission to the compiler to just stop considering flow control at that
point and do anything it likes with the following instruction slot.
What __builtin_unreachable() means to the compiler is "we will *never*
get here". That isn't the case for BUG() - BUG() means "we hope that
we will never get here, but we might, and if we do your data is in
grave danger."
We should either have something at that point (like a call to a function
which panics) or remove the ability to turn off CONFIG_BUG and anyone who
cares about kernel size needs to come up with a single trapping
instruction BUG() implementation.
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Chen Gang @ 2013-05-23 10:41 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Catalin Marinas, Linux-sh list, Heiko Carstens, paulus@samba.org,
H. Peter Anvin, Michel Lespinasse, Hans-Christian Egtvedt,
Linux-Arch, linux-s390, uml-devel, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <20130523100409.GK18614@n2100.arm.linux.org.uk>
On 05/23/2013 06:04 PM, Russell King - ARM Linux wrote:
> So, if you want to use this, then you should update the CONFIG_BUG text
> to include a warning to this effect:
>
> Warning: if CONFIG_BUG is turned off, and control flow reaches
> a BUG(), the system behaviour will be undefined.
>
> so that people can make an informed choice about this, because at the
> moment:
>
> Disabling this option eliminates support for BUG and WARN, reducing
> the size of your kernel image and potentially quietly ignoring
> numerous fatal conditions. You should only consider disabling this
> option for embedded systems with no facilities for reporting errors.
> Just say Y.
>
> will become completely misleading. Turning this option off will _not_
> result in "quietly ignoring numerous fatal conditions".
>
> And I come back to one of my previous arguments - is it not better to
> panic() if we hit one of these conditions so that the system can try to
> do a panic-reboot rather than continue blindly into the unknown?
But I still suggest to delete CONFIG_BUG in common kernel.
Since currently, disable 'CONFIG_BUG' is not a common features (most of
architectures are always enable it), it is only belongs to some
architectures specific features (may some embedded systems).
It is not suitable to still let 'CONFIG_BUG' exist in
"asm-generic/bug.h" which is only for common features.
And each architecture can customize their own BUG(), if one architecture
wants to Disabling this option, let it specify its own BUG().
So, most of architectures need not consider this issue again.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Arnd Bergmann @ 2013-05-23 10:59 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <20130523100409.GK18614@n2100.arm.linux.org.uk>
On Thursday 23 May 2013, Russell King - ARM Linux wrote:
> So, if you want to use this, then you should update the CONFIG_BUG text
> to include a warning to this effect:
>
> Warning: if CONFIG_BUG is turned off, and control flow reaches
> a BUG(), the system behaviour will be undefined.
>
> so that people can make an informed choice about this, because at the
> moment:
>
> Disabling this option eliminates support for BUG and WARN, reducing
> the size of your kernel image and potentially quietly ignoring
> numerous fatal conditions. You should only consider disabling this
> option for embedded systems with no facilities for reporting errors.
> Just say Y.
>
> will become completely misleading. Turning this option off will not
> result in "quietly ignoring numerous fatal conditions".
I must be missing something, to me the two descriptions mean the same thing.
> And I come back to one of my previous arguments - is it not better to
> panic() if we hit one of these conditions so that the system can try to
> do a panic-reboot rather than continue blindly into the unknown?
I think this all comes from the 'linux-tiny' project that tried to squeeze
out the last bits of kernel object code size at some point. The idea was
that if you have code like
BUG_ON(something_unexpected_happened());
or
switch (my_enum) {
case FOO:
return f1();
case BAR:
return f2();
default:
BUG();
}
You don't just want to avoid the code for printing the bug message and
the invalid instruction, we also want the compiler to not emit the
function call or check the enum for unexpected values. The meaning of
BUG() is really that person writing that statement was sure it cannot
happen unless there is a bug in the kernel, which has likely already
corrupted data. Printing a diagnostic at this point is nice if someone
is there to look at it, but letting the kernel do further actions that
may be undefined is not going to make things worse.
Arnd
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Eric W. Biederman @ 2013-05-23 10:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390,
Russell King - ARM Linux, Yoshinori Sato, Richard Weinberger,
Helge Deller, the arch/x86 maintainers, James E.J. Bottomley,
mingo@redhat.com, Geert Uytterhoeven, Frederic Weisbecker,
Paul McKenney, Håvard Skinnemoen, Serge Hallyn,
Mike Frysinger, uml-devel, Will Deacon, Jeff Dike, Akinobu Mita,
uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <201305231139.38233.arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> On Thursday 23 May 2013, Geert Uytterhoeven wrote:
>> > The problem is: trying to fix that will mean the result is a larger
>> > kernel than if you just do the usual arch-implemented thing of placing
>> > an defined faulting instruction at the BUG() site - which defeats the
>> > purpose of turning off CONFIG_BUG.
>>
>> Is __builtin_unreachable() working well these days?
>>
>
> Hmm, I just tried the trivial patch below, which seemed to do the right thing.
> Needs a little more investigation, but that might actually be the correct
> solution. I thought that at some point __builtin_unreachable() was the same
> as "do {} while (1)", but this is not the case with the gcc I was using --
> it just tells gcc that we don't expect to ever get here.
Yes.
We already have this abstracted in compiler.h as the macro unreachable,
so the slight modification of your patch below should handle this case.
For compilers without __builtin_unreachable() unreachable() expands to
do {} while(1) but an infinite loop seems reasonable and preserves the
semantics of the code, unlike the current noop that is do {} while(0).
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..9afff7d 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -108,11 +108,11 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
+#define BUG() unreachable ()
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (condition) unreachable(); } while(0)
#endif
#ifndef HAVE_ARCH_WARN_ON
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl: expand the size of the name in fsl_ssi_private struct
From: Timur Tabi @ 2013-05-23 11:19 UTC (permalink / raw)
To: David Laight, Nicolin Chen, linuxppc-dev, alsa-devel; +Cc: broonie
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7248@saturn3.aculab.com>
David Laight wrote:
>> strcpy(ssi_private->name, p) in probe() sets "name" by "p", gotten from dts,
>> >while the length of "p", if the devicetree node name of SSI is commonly set,
>> >would always be larger than 1 char size, so need a larger size for "name".
> Are you sure this isn't allowed for when the structure is allocated?
> Otherwise you also need to use strlcpy() as well.
Yes, this is already handled properly:
p = strrchr(np->full_name, '/') + 1;
ssi_private = kzalloc(sizeof(struct fsl_ssi_private) + strlen(p),
GFP_KERNEL);
Nicolin's patch is wrong. Do not apply it.
--
Timur Tabi
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Chen Gang @ 2013-05-23 11:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Catalin Marinas, Linux-sh list, Heiko Carstens, paulus@samba.org,
H. Peter Anvin, Michel Lespinasse, Hans-Christian Egtvedt,
Linux-Arch, linux-s390, Russell King - ARM Linux, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <201305231259.43750.arnd@arndb.de>
On 05/23/2013 06:59 PM, Arnd Bergmann wrote:
> You don't just want to avoid the code for printing the bug message and
> the invalid instruction, we also want the compiler to not emit the
> function call or check the enum for unexpected values. The meaning of
> BUG() is really that person writing that statement was sure it cannot
> happen unless there is a bug in the kernel, which has likely already
> corrupted data. Printing a diagnostic at this point is nice if someone
> is there to look at it, but letting the kernel do further actions that
> may be undefined is not going to make things worse.
So I think neither unreachable() nor panic() are suitable for this
condition.
I guess 'CONFIG_BUG' is not belong to common features, now (and in the
future), so it is not suitable still exist in "asm-generic/bug.h", need
remove it firstly.
And then let the specific architectures to implement their own BUG(), if
they want some special features.
SO most of arches can skip this issue.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Russell King - ARM Linux @ 2013-05-23 11:24 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <201305231259.43750.arnd@arndb.de>
On Thu, May 23, 2013 at 12:59:43PM +0200, Arnd Bergmann wrote:
> On Thursday 23 May 2013, Russell King - ARM Linux wrote:
> > So, if you want to use this, then you should update the CONFIG_BUG text
> > to include a warning to this effect:
> >
> > Warning: if CONFIG_BUG is turned off, and control flow reaches
> > a BUG(), the system behaviour will be undefined.
> >
> > so that people can make an informed choice about this, because at the
> > moment:
> >
> > Disabling this option eliminates support for BUG and WARN, reducing
> > the size of your kernel image and potentially quietly ignoring
> > numerous fatal conditions. You should only consider disabling this
> > option for embedded systems with no facilities for reporting errors.
> > Just say Y.
> >
> > will become completely misleading. Turning this option off will not
> > result in "quietly ignoring numerous fatal conditions".
>
> I must be missing something, to me the two descriptions mean the same thing.
To me, the current text suggests that we still detect the fatal condition
but the code continues to execute in a manner controlled by the program.
The latter is uncontrolled code (or data) execution in ways unspecified
by the program.
> You don't just want to avoid the code for printing the bug message and
> the invalid instruction, we also want the compiler to not emit the
> function call or check the enum for unexpected values. The meaning of
> BUG() is really that person writing that statement was sure it cannot
> happen unless there is a bug in the kernel, which has likely already
> corrupted data. Printing a diagnostic at this point is nice if someone
> is there to look at it, but letting the kernel do further actions that
> may be undefined is not going to make things worse.
I'm not talking about printing a diagnostic. I'm talking about the CPU
remaining under the control of the program it is running - that being
the Linux kernel.
With CONFIG_BUG unset, turning on things like reboot-on-panic and such
like is worthless. Arguably even is having a hardware watchdog - because
even if you hit one of these BUG() conditions where the CPU goes off and
does its own thing, it might be sufficient that the system is still able
to take care of the watchdog.
This is the problem you guys are missing - unreachable() means "we lose
control of the CPU at this point".
If you have an embedded system and you've taken out all the printk()
stuff, you most certainly want the system to do _something_ if you hit
an unexpected condition.
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Arnd Bergmann @ 2013-05-23 12:09 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <20130523112401.GO18614@n2100.arm.linux.org.uk>
On Thursday 23 May 2013, Russell King - ARM Linux wrote:
> This is the problem you guys are missing - unreachable() means "we lose
> control of the CPU at this point".
I'm absolutely aware of this. Again, the current behaviour of doing nothing
at all isn't very different from undefined behavior when you get when you
get to the end of a function returning a pointer without a "return" statement,
or when you return from a function that has determined that it is not safe
to continue.
> If you have an embedded system and you've taken out all the printk()
> stuff, you most certainly want the system to do something if you hit
> an unexpected condition.
I did not claim that it was a good idea to disable BUG(), all I said is
that "random stuff may happen" is probably what Matt Mackall had in mind when
he introduced the option.
Arnd
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Russell King - ARM Linux @ 2013-05-23 12:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, Yoshinori Sato,
Richard Weinberger, Helge Deller, the arch/x86 maintainers,
James E.J. Bottomley, mingo@redhat.com, Geert Uytterhoeven,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, uml-devel, Will Deacon, Jeff Dike,
Akinobu Mita, uml-user, uclinux-dist-devel@blackfin.uclinux.org,
Thomas Gleixner, linux-arm-kernel@lists.infradead.org,
Parisc List, linux-kernel@vger.kernel.org, Richard Kuo,
Paul Mundt, Eric W. Biederman, linux-hexagon, Martin Schwidefsky,
linux390, Andrew Morton, linuxppc-dev@lists.ozlabs.org,
David Miller
In-Reply-To: <201305231409.02359.arnd@arndb.de>
On Thu, May 23, 2013 at 02:09:02PM +0200, Arnd Bergmann wrote:
> On Thursday 23 May 2013, Russell King - ARM Linux wrote:
> > This is the problem you guys are missing - unreachable() means "we lose
> > control of the CPU at this point".
>
> I'm absolutely aware of this. Again, the current behaviour of doing nothing
> at all isn't very different from undefined behavior when you get when you
> get to the end of a function returning a pointer without a "return" statement,
> or when you return from a function that has determined that it is not safe
> to continue.
Running off the end of a function like that is a different kettle of fish.
The execution path is still as the compiler intends - what isn't is that
the data returned is likely to be random trash.
That's _quite_ different from the CPU starting to execute the contents
of a literal data pool.
^ permalink raw reply
* Re: [PATCH 1/5] perf: New conditional branch filter criteria in branch stack sampling
From: Stephane Eranian @ 2013-05-23 13:36 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Michael Neuling, ak@linux.intel.com, Peter Zijlstra, LKML,
Linux PPC dev, Ingo Molnar
In-Reply-To: <1369203761-12649-2-git-send-email-khandual@linux.vnet.ibm.com>
On Wed, May 22, 2013 at 8:22 AM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> POWER8 PMU based BHRB supports filtering for conditional branches.
> This patch introduces new branch filter PERF_SAMPLE_BRANCH_COND which
> will extend the existing perf ABI. Other architectures can provide
> this functionality with either HW filtering support (if present) or
> with SW filtering of instructions.
>
Reviewed-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> include/uapi/linux/perf_event.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index fb104e5..cb0de86 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
> + PERF_SAMPLE_BRANCH_COND = 1U << 7, /* conditional branches */
>
> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
> };
>
> #define PERF_SAMPLE_BRANCH_PLM_ALL \
> --
> 1.7.11.7
>
^ permalink raw reply
* Re: [PATCH 4/5] x86, perf: Add conditional branch filtering support
From: Stephane Eranian @ 2013-05-23 13:38 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Michael Neuling, ak@linux.intel.com, Peter Zijlstra, LKML,
Linux PPC dev, Ingo Molnar
In-Reply-To: <1369203761-12649-5-git-send-email-khandual@linux.vnet.ibm.com>
On Wed, May 22, 2013 at 8:22 AM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
>
> This patch adds conditional branch filtering support,
> enabling it for PERF_SAMPLE_BRANCH_COND in perf branch
> stack sampling framework by utilizing an available
> software filter X86_BR_JCC.
>
Reviewed-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> arch/x86/kernel/cpu/perf_event_intel_lbr.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> index d978353..a0d6387 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter(struct perf_event *event)
>
> if (br_type & PERF_SAMPLE_BRANCH_IND_CALL)
> mask |= X86_BR_IND_CALL;
> +
> + if (br_type & PERF_SAMPLE_BRANCH_COND)
> + mask |= X86_BR_JCC;
> +
> /*
> * stash actual user request into reg, it may
> * be used by fixup code for some CPU
> @@ -626,6 +630,7 @@ static const int nhm_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
> * NHM/WSM erratum: must include IND_JMP to capture IND_CALL
> */
> [PERF_SAMPLE_BRANCH_IND_CALL] = LBR_IND_CALL | LBR_IND_JMP,
> + [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
> };
>
> static const int snb_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
> @@ -637,6 +642,7 @@ static const int snb_lbr_sel_map[PERF_SAMPLE_BRANCH_MAX] = {
> [PERF_SAMPLE_BRANCH_ANY_CALL] = LBR_REL_CALL | LBR_IND_CALL
> | LBR_FAR,
> [PERF_SAMPLE_BRANCH_IND_CALL] = LBR_IND_CALL,
> + [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
> };
>
> /* core */
> --
> 1.7.11.7
>
^ permalink raw reply
* Re: [PATCH 5/5] perf, documentation: Description for conditional branch filter
From: Stephane Eranian @ 2013-05-23 13:39 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Michael Neuling, ak@linux.intel.com, Peter Zijlstra, LKML,
Linux PPC dev, Ingo Molnar
In-Reply-To: <1369203761-12649-6-git-send-email-khandual@linux.vnet.ibm.com>
On Wed, May 22, 2013 at 8:22 AM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> Adding documentation support for conditional branch filter.
>
Reviewed-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> tools/perf/Documentation/perf-record.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index d4da111..8b5e1ed 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -169,12 +169,13 @@ following filters are defined:
> - any_call: any function call or system call
> - any_ret: any function return or system call return
> - ind_call: any indirect branch
> + - cond: conditional branches
> - u: only when the branch target is at the user level
> - k: only when the branch target is in the kernel
> - hv: only when the target is at the hypervisor level
>
> +
> -The option requires at least one branch type among any, any_call, any_ret, ind_call.
> +The option requires at least one branch type among any, any_call, any_ret, ind_call, cond.
> The privilege levels may be omitted, in which case, the privilege levels of the associated
> event are applied to the branch filter. Both kernel (k) and hypervisor (hv) privilege
> levels are subject to permissions. When sampling on multiple events, branch stack sampling
> --
> 1.7.11.7
>
^ permalink raw reply
* Re: [PATCH] arch: configuration, deleting 'CONFIG_BUG' since always need it.
From: Geert Uytterhoeven @ 2013-05-23 14:10 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Catalin Marinas, Linux-sh list, Chen Gang, Heiko Carstens,
paulus@samba.org, H. Peter Anvin, Michel Lespinasse,
Hans-Christian Egtvedt, Linux-Arch, linux-s390, uml-devel,
Yoshinori Sato, Richard Weinberger, Helge Deller,
the arch/x86 maintainers, James E.J. Bottomley, mingo@redhat.com,
Frederic Weisbecker, Paul McKenney, Håvard Skinnemoen,
Serge Hallyn, Mike Frysinger, Arnd Bergmann, Will Deacon,
Jeff Dike, Akinobu Mita, uml-user,
uclinux-dist-devel@blackfin.uclinux.org, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Parisc List,
linux-kernel@vger.kernel.org, Richard Kuo, Paul Mundt,
Eric W. Biederman, linux-hexagon, Martin Schwidefsky, linux390,
Andrew Morton, linuxppc-dev@lists.ozlabs.org, David Miller
In-Reply-To: <20130523125033.GP18614@n2100.arm.linux.org.uk>
On Thu, May 23, 2013 at 2:50 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, May 23, 2013 at 02:09:02PM +0200, Arnd Bergmann wrote:
>> On Thursday 23 May 2013, Russell King - ARM Linux wrote:
>> > This is the problem you guys are missing - unreachable() means "we lose
>> > control of the CPU at this point".
>>
>> I'm absolutely aware of this. Again, the current behaviour of doing nothing
>> at all isn't very different from undefined behavior when you get when you
>> get to the end of a function returning a pointer without a "return" statement,
>> or when you return from a function that has determined that it is not safe
>> to continue.
>
> Running off the end of a function like that is a different kettle of fish.
> The execution path is still as the compiler intends - what isn't is that
> the data returned is likely to be random trash.
>
> That's _quite_ different from the CPU starting to execute the contents
> of a literal data pool.
I agree it's best to e.g. trap and reboot.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox