public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC] bug.h: Drop filename from BUG/WARNING logs if building for TPL/SPL
Date: Wed, 17 Jul 2019 13:22:39 -0400	[thread overview]
Message-ID: <20190717172239.GD20116@bill-the-cat> (raw)
In-Reply-To: <20190710213044.19985-1-dannenberg@ti.com>

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>

      parent reply	other threads:[~2019-07-17 17:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190717172239.GD20116@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox