All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
@ 2019-07-10 21:30 Andreas Dannenberg
  2019-07-11 15:33 ` Andreas Dannenberg
  2019-07-17 17:22 ` Tom Rini
  0 siblings, 2 replies; 8+ messages in thread
From: Andreas Dannenberg @ 2019-07-10 21:30 UTC (permalink / raw)
  To: u-boot

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); \
 	unlikely(__ret_warn_on);					\
 })
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-10 21:30 [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL Andreas Dannenberg
@ 2019-07-11 15:33 ` Andreas Dannenberg
  2019-07-11 17:29   ` Masahiro Yamada
  2019-07-17 17:22 ` Tom Rini
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas Dannenberg @ 2019-07-11 15:33 UTC (permalink / raw)
  To: u-boot

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...

Just some quick examples about the savings...

Using buildman "bloat" reporting (-B) I see the SPL .text size for AM335x
to be reduced by 12 bytes. And for AM43xx the size goes down by 52
bytes. The benefit of the proposed change really depends on a) whether a
given platform uses SPL, and b) how many calls to BUG/WARN it has. The
USB drivers in AM335x/AM43xx are really the "heavy hitters" here. I'm
sure I could find additional examples/platforms to highlight savings if
needed.

Anyways I'm not proud of the proposed change but merely wanted to see
with this RFC if there isn't any way to do further optimizations on the
__FILE__ topic that are not overly intrusive specifically as it comes to
SPL.

Best Regards,
Andreas


> 
> 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); \
>  	unlikely(__ret_warn_on);					\
>  })
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-11 15:33 ` Andreas Dannenberg
@ 2019-07-11 17:29   ` Masahiro Yamada
  2019-07-11 18:12     ` Andreas Dannenberg
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2019-07-11 17:29 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 12, 2019 at 12:34 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
>
> 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...
>
> Just some quick examples about the savings...
>
> Using buildman "bloat" reporting (-B) I see the SPL .text size for AM335x
> to be reduced by 12 bytes. And for AM43xx the size goes down by 52
> bytes. The benefit of the proposed change really depends on a) whether a
> given platform uses SPL, and b) how many calls to BUG/WARN it has. The
> USB drivers in AM335x/AM43xx are really the "heavy hitters" here. I'm
> sure I could find additional examples/platforms to highlight savings if
> needed.
>
> Anyways I'm not proud of the proposed change but merely wanted to see
> with this RFC if there isn't any way to do further optimizations on the
> __FILE__ topic that are not overly intrusive specifically as it comes to
> SPL.


Commit 1eb2e71edd55e16562e3912881c449db69623352
was not enough to solve your problem.

Correct?



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-11 17:29   ` Masahiro Yamada
@ 2019-07-11 18:12     ` Andreas Dannenberg
  2019-07-11 18:43       ` Tom Rini
  2019-07-12  7:11       ` Masahiro Yamada
  0 siblings, 2 replies; 8+ messages in thread
From: Andreas Dannenberg @ 2019-07-11 18:12 UTC (permalink / raw)
  To: u-boot

Yamada-san,

On Fri, Jul 12, 2019 at 02:29:02AM +0900, Masahiro Yamada wrote:
> On Fri, Jul 12, 2019 at 12:34 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
> >
> > 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...
> >
> > Just some quick examples about the savings...
> >
> > Using buildman "bloat" reporting (-B) I see the SPL .text size for AM335x
> > to be reduced by 12 bytes. And for AM43xx the size goes down by 52
> > bytes. The benefit of the proposed change really depends on a) whether a
> > given platform uses SPL, and b) how many calls to BUG/WARN it has. The
> > USB drivers in AM335x/AM43xx are really the "heavy hitters" here. I'm
> > sure I could find additional examples/platforms to highlight savings if
> > needed.
> >
> > Anyways I'm not proud of the proposed change but merely wanted to see
> > with this RFC if there isn't any way to do further optimizations on the
> > __FILE__ topic that are not overly intrusive specifically as it comes to
> > SPL.
> 
> 
> Commit 1eb2e71edd55e16562e3912881c449db69623352
> was not enough to solve your problem.
> 
> Correct?

Correct. This is a great commit and I saw what all went into it and it
goes a long way in addressing the general concern and doing much needed
cleanup but I wanted to go beyond this.

Consider this example, if I use a WARN_ON() deep in the arch folder of
one of the SoCs, I would get this on the console...

	WARNING at arch/arm/mach-k3/am6_init.c:192/board_init_f()!

...and now, with the proposed change, it would boil down to this...

	WARNING at 192 at board_init_f()!


If we could just keep the base filename itself that would still be a
good size reduction, and keep the output somewhat consistent with the
original behavior, but I spent quite some time researching but without
some extreme "magic" there didn't seem to be an universal solution...


Also some additional background why I am even looking into this. One of
our platforms (AM335x) has a regression building on Travis CI with some
of our pending patches applied. Interestingly, this only happens on
Travis, which still uses GCC 7.3.0 for arm (why?). With newer, more
efficient compilers there is no such issue.

--
Andreas Dannenberg
Texas Instruments Inc

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-11 18:12     ` Andreas Dannenberg
@ 2019-07-11 18:43       ` Tom Rini
  2019-07-11 18:50         ` Andreas Dannenberg
  2019-07-12  7:11       ` Masahiro Yamada
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Rini @ 2019-07-11 18:43 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 01:12:16PM -0500, Andreas Dannenberg wrote:
> Yamada-san,
> 
> On Fri, Jul 12, 2019 at 02:29:02AM +0900, Masahiro Yamada wrote:
> > On Fri, Jul 12, 2019 at 12:34 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
> > >
> > > 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...
> > >
> > > Just some quick examples about the savings...
> > >
> > > Using buildman "bloat" reporting (-B) I see the SPL .text size for AM335x
> > > to be reduced by 12 bytes. And for AM43xx the size goes down by 52
> > > bytes. The benefit of the proposed change really depends on a) whether a
> > > given platform uses SPL, and b) how many calls to BUG/WARN it has. The
> > > USB drivers in AM335x/AM43xx are really the "heavy hitters" here. I'm
> > > sure I could find additional examples/platforms to highlight savings if
> > > needed.
> > >
> > > Anyways I'm not proud of the proposed change but merely wanted to see
> > > with this RFC if there isn't any way to do further optimizations on the
> > > __FILE__ topic that are not overly intrusive specifically as it comes to
> > > SPL.
> > 
> > 
> > Commit 1eb2e71edd55e16562e3912881c449db69623352
> > was not enough to solve your problem.
> > 
> > Correct?
> 
> Correct. This is a great commit and I saw what all went into it and it
> goes a long way in addressing the general concern and doing much needed
> cleanup but I wanted to go beyond this.
> 
> Consider this example, if I use a WARN_ON() deep in the arch folder of
> one of the SoCs, I would get this on the console...
> 
> 	WARNING at arch/arm/mach-k3/am6_init.c:192/board_init_f()!
> 
> ...and now, with the proposed change, it would boil down to this...
> 
> 	WARNING at 192 at board_init_f()!
> 
> 
> If we could just keep the base filename itself that would still be a
> good size reduction, and keep the output somewhat consistent with the
> original behavior, but I spent quite some time researching but without
> some extreme "magic" there didn't seem to be an universal solution...
> 
> 
> Also some additional background why I am even looking into this. One of
> our platforms (AM335x) has a regression building on Travis CI with some
> of our pending patches applied. Interestingly, this only happens on
> Travis, which still uses GCC 7.3.0 for arm (why?). With newer, more
> efficient compilers there is no such issue.

On this last point, gcc 7.3 is the current GCC release that doesn't have
regressions elsewhere until we reach gcc-9, for which I cannot find good
pre-built toolchains.  We can get gcc-8.3 (or 8.2?  I forget) from Arm
Ltd now, but that has problems on x86.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190711/6098aba2/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-11 18:43       ` Tom Rini
@ 2019-07-11 18:50         ` Andreas Dannenberg
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Dannenberg @ 2019-07-11 18:50 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On Thu, Jul 11, 2019 at 02:43:17PM -0400, Tom Rini wrote:
> On Thu, Jul 11, 2019 at 01:12:16PM -0500, Andreas Dannenberg wrote:

<snip>

> > Also some additional background why I am even looking into this. One of
> > our platforms (AM335x) has a regression building on Travis CI with some
> > of our pending patches applied. Interestingly, this only happens on
> > Travis, which still uses GCC 7.3.0 for arm (why?). With newer, more
> > efficient compilers there is no such issue.
> 
> On this last point, gcc 7.3 is the current GCC release that doesn't have
> regressions elsewhere until we reach gcc-9, for which I cannot find good
> pre-built toolchains.  We can get gcc-8.3 (or 8.2?  I forget) from Arm
> Ltd now, but that has problems on x86.

Thanks for the background Tom, I wondered about the reasons behind.

Regards, Andreas


--
Andreas Dannenberg
Texas Instruments Inc

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-11 18:12     ` Andreas Dannenberg
  2019-07-11 18:43       ` Tom Rini
@ 2019-07-12  7:11       ` Masahiro Yamada
  1 sibling, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2019-07-12  7:11 UTC (permalink / raw)
  To: u-boot

Hi Andreas,

On Fri, Jul 12, 2019 at 3:12 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
>
> Yamada-san,
>
> On Fri, Jul 12, 2019 at 02:29:02AM +0900, Masahiro Yamada wrote:
> > On Fri, Jul 12, 2019 at 12:34 AM Andreas Dannenberg <dannenberg@ti.com> wrote:
> > >
> > > 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...
> > >
> > > Just some quick examples about the savings...
> > >
> > > Using buildman "bloat" reporting (-B) I see the SPL .text size for AM335x
> > > to be reduced by 12 bytes. And for AM43xx the size goes down by 52
> > > bytes. The benefit of the proposed change really depends on a) whether a
> > > given platform uses SPL, and b) how many calls to BUG/WARN it has. The
> > > USB drivers in AM335x/AM43xx are really the "heavy hitters" here. I'm
> > > sure I could find additional examples/platforms to highlight savings if
> > > needed.
> > >
> > > Anyways I'm not proud of the proposed change but merely wanted to see
> > > with this RFC if there isn't any way to do further optimizations on the
> > > __FILE__ topic that are not overly intrusive specifically as it comes to
> > > SPL.
> >
> >
> > Commit 1eb2e71edd55e16562e3912881c449db69623352
> > was not enough to solve your problem.
> >
> > Correct?
>
> Correct. This is a great commit and I saw what all went into it and it
> goes a long way in addressing the general concern and doing much needed
> cleanup but I wanted to go beyond this.
>
> Consider this example, if I use a WARN_ON() deep in the arch folder of
> one of the SoCs, I would get this on the console...
>
>         WARNING at arch/arm/mach-k3/am6_init.c:192/board_init_f()!
>
> ...and now, with the proposed change, it would boil down to this...
>
>         WARNING at 192 at board_init_f()!
>
>
> If we could just keep the base filename itself that would still be a
> good size reduction, and keep the output somewhat consistent with the
> original behavior, but I spent quite some time researching but without
> some extreme "magic" there didn't seem to be an universal solution...
>
>
> Also some additional background why I am even looking into this. One of
> our platforms (AM335x) has a regression building on Travis CI with some
> of our pending patches applied. Interestingly, this only happens on
> Travis, which still uses GCC 7.3.0 for arm (why?). With newer, more
> efficient compilers there is no such issue.


OK, then.
Thanks for explanation.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
  2019-07-10 21:30 [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL Andreas Dannenberg
  2019-07-11 15:33 ` Andreas Dannenberg
@ 2019-07-17 17:22 ` Tom Rini
  1 sibling, 0 replies; 8+ messages in thread
From: Tom Rini @ 2019-07-17 17:22 UTC (permalink / raw)
  To: u-boot

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); \
>  	unlikely(__ret_warn_on);					\
>  })

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190717/d6fedddd/attachment.sig>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-17 17:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-10 21:30 [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL Andreas Dannenberg
2019-07-11 15:33 ` Andreas Dannenberg
2019-07-11 17:29   ` Masahiro Yamada
2019-07-11 18:12     ` Andreas Dannenberg
2019-07-11 18:43       ` Tom Rini
2019-07-11 18:50         ` Andreas Dannenberg
2019-07-12  7:11       ` Masahiro Yamada
2019-07-17 17:22 ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.