| Message ID | 1511871803-10385-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| Headers | show |
| Series | Remove assert() | expand |
On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: > Buildman test passed. > > > Masahiro Yamada (5): > Move CONFIG_PANIC_HANG to Kconfig > ARM: openrd: set CONFIG_LOGLEVEL to 2 > Enable CONFIG_PANIC_HANG for boards without do_reset() > treewide: convert assert() to BUG_ON() > Remove assert() Have you done a size check on this series? If not, I'll fire one up, thanks! -- Tom
2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: > >> Buildman test passed. >> >> >> Masahiro Yamada (5): >> Move CONFIG_PANIC_HANG to Kconfig >> ARM: openrd: set CONFIG_LOGLEVEL to 2 >> Enable CONFIG_PANIC_HANG for boards without do_reset() >> treewide: convert assert() to BUG_ON() >> Remove assert() > > Have you done a size check on this series? If not, I'll fire one up, > thanks! > I think I did it correctly. At least, I needed the following patch for openrd boards: http://patchwork.ozlabs.org/patch/842106/ Looks like the NAND core pull-request needs it, too.
On Wed, Nov 29, 2017 at 12:17:35AM +0900, Masahiro Yamada wrote: > 2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: > > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: > > > >> Buildman test passed. > >> > >> > >> Masahiro Yamada (5): > >> Move CONFIG_PANIC_HANG to Kconfig > >> ARM: openrd: set CONFIG_LOGLEVEL to 2 > >> Enable CONFIG_PANIC_HANG for boards without do_reset() > >> treewide: convert assert() to BUG_ON() > >> Remove assert() > > > > Have you done a size check on this series? If not, I'll fire one up, > > thanks! > > > > I think I did it correctly. > > At least, I needed the following patch for openrd boards: > http://patchwork.ozlabs.org/patch/842106/ > > Looks like the NAND core pull-request needs it, too. So, in my testing (which is gcc-6 for everything thanks to Debian/9): 06: Remove assert() arm: + clearfog peach-pi snow smdk5250 smdk5420 turris_omnia spring omap3_evm peach-pit -(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 760 bytes +(clearfog,turris_omnia) arm-linux-gnueabihf-ld.bfd: SPL image too big +(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 1080 bytes +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) arch/arm/mach-exynos/built-in.o: In function `clock_calc_best_scalar': +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) build/../arch/arm/mach-exynos/clock.c:1408: undefined reference to `panic' +(omap3_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 28 bytes And then a lot of platforms grow in size. Taking a harder peek, BUG_ON is always set. However, assert() is only set when DEBUG is set. Now, I can certainly see an argument along the lines of that being fairly unexpected, and I don't know that I would disagree. After a very quick look over what git grep -l says, perhaps we want to introduce a new (default off) option to preserve the current behavior of assert() in BUG_ON, as it looks like almost only kernel imported code is using BUG_ON. -- Tom
2017-11-29 10:42 GMT+09:00 Tom Rini <trini@konsulko.com>: > On Wed, Nov 29, 2017 at 12:17:35AM +0900, Masahiro Yamada wrote: >> 2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: >> > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: >> > >> >> Buildman test passed. >> >> >> >> >> >> Masahiro Yamada (5): >> >> Move CONFIG_PANIC_HANG to Kconfig >> >> ARM: openrd: set CONFIG_LOGLEVEL to 2 >> >> Enable CONFIG_PANIC_HANG for boards without do_reset() >> >> treewide: convert assert() to BUG_ON() >> >> Remove assert() >> > >> > Have you done a size check on this series? If not, I'll fire one up, >> > thanks! >> > >> >> I think I did it correctly. >> >> At least, I needed the following patch for openrd boards: >> http://patchwork.ozlabs.org/patch/842106/ >> >> Looks like the NAND core pull-request needs it, too. > > So, in my testing (which is gcc-6 for everything thanks to Debian/9): > 06: Remove assert() > arm: + clearfog peach-pi snow smdk5250 smdk5420 turris_omnia spring omap3_evm peach-pit > -(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 760 bytes > +(clearfog,turris_omnia) arm-linux-gnueabihf-ld.bfd: SPL image too big > +(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 1080 bytes > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) arch/arm/mach-exynos/built-in.o: In function `clock_calc_best_scalar': > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) build/../arch/arm/mach-exynos/clock.c:1408: undefined reference to `panic' > +(omap3_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 28 bytes > > And then a lot of platforms grow in size. Taking a harder peek, BUG_ON > is always set. However, assert() is only set when DEBUG is set. Now, I > can certainly see an argument along the lines of that being fairly > unexpected, and I don't know that I would disagree. > > After a very quick look over what git grep -l says, perhaps we want to > introduce a new (default off) option to preserve the current behavior of > assert() in BUG_ON, as it looks like almost only kernel imported code is > using BUG_ON. > Do you have an idea for the option name?
On Wed, Nov 29, 2017 at 11:01:02AM +0900, Masahiro Yamada wrote: > 2017-11-29 10:42 GMT+09:00 Tom Rini <trini@konsulko.com>: > > On Wed, Nov 29, 2017 at 12:17:35AM +0900, Masahiro Yamada wrote: > >> 2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: > >> > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: > >> > > >> >> Buildman test passed. > >> >> > >> >> > >> >> Masahiro Yamada (5): > >> >> Move CONFIG_PANIC_HANG to Kconfig > >> >> ARM: openrd: set CONFIG_LOGLEVEL to 2 > >> >> Enable CONFIG_PANIC_HANG for boards without do_reset() > >> >> treewide: convert assert() to BUG_ON() > >> >> Remove assert() > >> > > >> > Have you done a size check on this series? If not, I'll fire one up, > >> > thanks! > >> > > >> > >> I think I did it correctly. > >> > >> At least, I needed the following patch for openrd boards: > >> http://patchwork.ozlabs.org/patch/842106/ > >> > >> Looks like the NAND core pull-request needs it, too. > > > > So, in my testing (which is gcc-6 for everything thanks to Debian/9): > > 06: Remove assert() > > arm: + clearfog peach-pi snow smdk5250 smdk5420 turris_omnia spring omap3_evm peach-pit > > -(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 760 bytes > > +(clearfog,turris_omnia) arm-linux-gnueabihf-ld.bfd: SPL image too big > > +(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 1080 bytes > > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) arch/arm/mach-exynos/built-in.o: In function `clock_calc_best_scalar': > > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) build/../arch/arm/mach-exynos/clock.c:1408: undefined reference to `panic' > > +(omap3_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 28 bytes > > > > And then a lot of platforms grow in size. Taking a harder peek, BUG_ON > > is always set. However, assert() is only set when DEBUG is set. Now, I > > can certainly see an argument along the lines of that being fairly > > unexpected, and I don't know that I would disagree. > > > > After a very quick look over what git grep -l says, perhaps we want to > > introduce a new (default off) option to preserve the current behavior of > > assert() in BUG_ON, as it looks like almost only kernel imported code is > > using BUG_ON. > > > > Do you have an idea for the option name? ENABLE_BUG_ON_CHECKS ? -- Tom
2017-11-29 11:30 GMT+09:00 Tom Rini <trini@konsulko.com>: > On Wed, Nov 29, 2017 at 11:01:02AM +0900, Masahiro Yamada wrote: >> 2017-11-29 10:42 GMT+09:00 Tom Rini <trini@konsulko.com>: >> > On Wed, Nov 29, 2017 at 12:17:35AM +0900, Masahiro Yamada wrote: >> >> 2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: >> >> > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: >> >> > >> >> >> Buildman test passed. >> >> >> >> >> >> >> >> >> Masahiro Yamada (5): >> >> >> Move CONFIG_PANIC_HANG to Kconfig >> >> >> ARM: openrd: set CONFIG_LOGLEVEL to 2 >> >> >> Enable CONFIG_PANIC_HANG for boards without do_reset() >> >> >> treewide: convert assert() to BUG_ON() >> >> >> Remove assert() >> >> > >> >> > Have you done a size check on this series? If not, I'll fire one up, >> >> > thanks! >> >> > >> >> >> >> I think I did it correctly. >> >> >> >> At least, I needed the following patch for openrd boards: >> >> http://patchwork.ozlabs.org/patch/842106/ >> >> >> >> Looks like the NAND core pull-request needs it, too. >> > >> > So, in my testing (which is gcc-6 for everything thanks to Debian/9): >> > 06: Remove assert() >> > arm: + clearfog peach-pi snow smdk5250 smdk5420 turris_omnia spring omap3_evm peach-pit >> > -(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 760 bytes >> > +(clearfog,turris_omnia) arm-linux-gnueabihf-ld.bfd: SPL image too big >> > +(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 1080 bytes >> > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) arch/arm/mach-exynos/built-in.o: In function `clock_calc_best_scalar': >> > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) build/../arch/arm/mach-exynos/clock.c:1408: undefined reference to `panic' >> > +(omap3_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 28 bytes >> > >> > And then a lot of platforms grow in size. Taking a harder peek, BUG_ON >> > is always set. However, assert() is only set when DEBUG is set. Now, I >> > can certainly see an argument along the lines of that being fairly >> > unexpected, and I don't know that I would disagree. >> > >> > After a very quick look over what git grep -l says, perhaps we want to >> > introduce a new (default off) option to preserve the current behavior of >> > assert() in BUG_ON, as it looks like almost only kernel imported code is >> > using BUG_ON. >> > >> >> Do you have an idea for the option name? > > ENABLE_BUG_ON_CHECKS ? > > -- Do you mean CONFIG option? i.e. CONFIG_ENABLE_BUG_ON_CHECKS so we can turn it on/off globally?
On Wed, Nov 29, 2017 at 12:02:44PM +0900, Masahiro Yamada wrote: > 2017-11-29 11:30 GMT+09:00 Tom Rini <trini@konsulko.com>: > > On Wed, Nov 29, 2017 at 11:01:02AM +0900, Masahiro Yamada wrote: > >> 2017-11-29 10:42 GMT+09:00 Tom Rini <trini@konsulko.com>: > >> > On Wed, Nov 29, 2017 at 12:17:35AM +0900, Masahiro Yamada wrote: > >> >> 2017-11-28 23:52 GMT+09:00 Tom Rini <trini@konsulko.com>: > >> >> > On Tue, Nov 28, 2017 at 09:23:18PM +0900, Masahiro Yamada wrote: > >> >> > > >> >> >> Buildman test passed. > >> >> >> > >> >> >> > >> >> >> Masahiro Yamada (5): > >> >> >> Move CONFIG_PANIC_HANG to Kconfig > >> >> >> ARM: openrd: set CONFIG_LOGLEVEL to 2 > >> >> >> Enable CONFIG_PANIC_HANG for boards without do_reset() > >> >> >> treewide: convert assert() to BUG_ON() > >> >> >> Remove assert() > >> >> > > >> >> > Have you done a size check on this series? If not, I'll fire one up, > >> >> > thanks! > >> >> > > >> >> > >> >> I think I did it correctly. > >> >> > >> >> At least, I needed the following patch for openrd boards: > >> >> http://patchwork.ozlabs.org/patch/842106/ > >> >> > >> >> Looks like the NAND core pull-request needs it, too. > >> > > >> > So, in my testing (which is gcc-6 for everything thanks to Debian/9): > >> > 06: Remove assert() > >> > arm: + clearfog peach-pi snow smdk5250 smdk5420 turris_omnia spring omap3_evm peach-pit > >> > -(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 760 bytes > >> > +(clearfog,turris_omnia) arm-linux-gnueabihf-ld.bfd: SPL image too big > >> > +(am335x_hs_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 1080 bytes > >> > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) arch/arm/mach-exynos/built-in.o: In function `clock_calc_best_scalar': > >> > +(snow,smdk5250,peach-pi,smdk5420,spring,peach-pit) build/../arch/arm/mach-exynos/clock.c:1408: undefined reference to `panic' > >> > +(omap3_evm) arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 28 bytes > >> > > >> > And then a lot of platforms grow in size. Taking a harder peek, BUG_ON > >> > is always set. However, assert() is only set when DEBUG is set. Now, I > >> > can certainly see an argument along the lines of that being fairly > >> > unexpected, and I don't know that I would disagree. > >> > > >> > After a very quick look over what git grep -l says, perhaps we want to > >> > introduce a new (default off) option to preserve the current behavior of > >> > assert() in BUG_ON, as it looks like almost only kernel imported code is > >> > using BUG_ON. > >> > > >> > >> Do you have an idea for the option name? > > > > ENABLE_BUG_ON_CHECKS ? > > Do you mean CONFIG option? > i.e. CONFIG_ENABLE_BUG_ON_CHECKS > so we can turn it on/off globally? Yes, a CONFIG option to turn it on/off. -- Tom