On Wed, Jul 10, 2019 at 04:30:44PM -0500, Andreas Dannenberg wrote:
On several platforms space is very tight when building for SPL or TPL. To claw back a few bytes to be used for code remove the __FILE__ name from the BUG() and WARN() type macros. Since those macros still print the function name plus a line number this should not really affect the ability to backtrace an actual BUG/WARN message to a specific piece of code.
Signed-off-by: Andreas Dannenberg dannenberg@ti.com
I was looking for a way to shave off a few bytes from the SPL code size (TI AM335x) and looking at the hexdump of the SPL I found well why not further reduce some strings down in size... I was already aware of the recent compiler optimizations to drop some of the irrelevant path from the __FILE__ macro but I wanted to go one step beyond this. Dropping all the path from __FILE__ via preprocessor macro can't be easily done as others have already found so I decided to drop __FILE__ altogether (code below) and was excited about the improvements I got...
Then of course using Google I found there was prior art, specifically this discussion here:
"[U-Boot] __FILE__ usage and and SPL limits for SRAM" https://patchwork.ozlabs.org/patch/746922/
So I made this submission to "RFC" to simply re-ignite the subject to see if we can somehow find some path to proceed with such a change...
I like about the proposal referenced above that it touches more places than what I came up with, however it is missing the TPL/SPL aspect which I thought would be a good way to alleviate some of the concerns raised (Wolfgang) around not having __FILE__ in the log...
Maybe a combination of the approaches could be workable?
At the end of the day SPL/TPL are intended for very memory-constrained environments, so I feel changes like the proposed that don't really affect any of the existing functionality are good candidates to consider...
Regards, Andreas
include/linux/bug.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/bug.h b/include/linux/bug.h index 29f84168a3..36b5fddfae 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -5,9 +5,22 @@ #include <linux/build_bug.h> #include <linux/compiler.h> #include <linux/printk.h> +#include <linux/kconfig.h>
+#if defined(CONFIG_TPL_BUILD) || defined(CONFIG_SPL_BUILD) +/*
- In case of TPL/SPL use a short format not including __FILE__
- to reduce image size
- */
+#define BUG_WARN_LOC_FMT "%d@%s()" +#define BUG_WARN_LOC_ARGS __LINE__, __func__ +#else +#define BUG_WARN_LOC_FMT "%s:%d/%s()" +#define BUG_WARN_LOC_ARGS __FILE__, __LINE__, __func__ +#endif
#define BUG() do { \
- printk("BUG at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
- printk("BUG at " BUG_WARN_LOC_FMT "!\n", BUG_WARN_LOC_ARGS); \ panic("BUG!"); \
} while (0)
@@ -16,7 +29,7 @@ #define WARN_ON(condition) ({ \ int __ret_warn_on = !!(condition); \ if (unlikely(__ret_warn_on)) \
printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
printk("WARNING at " BUG_WARN_LOC_FMT "!\n", BUG_WARN_LOC_ARGS); \})
I know this is RFC but I think I'm really fine with it. With respect to SPL failures we have: - Developer is developing. It's not unreasonable to expect the developer to be able to figure out what the file is that the message came from. - Deployed, no one should see this ever, so it doesn't matter. - Deployed, vendor asks customer to pull up debug interface (or vendor tech is onsite, pulls up debug interface, etc). Still a reasonable expectation that the person seeing the log will be able to work things out with function/line numbers.
I'm not applying this right away, but I expect to soon.