All of lore.kernel.org
 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 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.