| Message ID | 20200209215935.77472-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] log: output for CONFIG_LOG=n | expand |
On 2/9/20 4:59 PM, Heinrich Schuchardt wrote: > If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, > and for DEBUG=1 also debug messages. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> Why not just change the default for CONFIG_LOG to y? This is effectively the same, except it still allows users to completely disable logging altogether. --Sean
On 2/9/20 11:21 PM, Sean Anderson wrote: > On 2/9/20 4:59 PM, Heinrich Schuchardt wrote: >> If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, >> and for DEBUG=1 also debug messages. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > Why not just change the default for CONFIG_LOG to y? This is effectively > the same, except it still allows users to completely disable logging > altogether. > > --Sean > I have tested your suggestion for qemu_arm64_defconfig: Without my patch and CONFIG_LOG=n: u-boot.bin 664200 bytes With my patch and CONFIG_LOG=n: u-boot.bin 664432 bytes Without my patch but with CONFIG_LOG=y and CONFIG_CONSOLE=y: u-boot.bin 666648 bytes So your suggestion consumes 2216 additional bytes to produce the essentially the same console output. IMHO CONFIG_LOG=y is currently only helpful in the following situation: * You are debugging your board and want to interactively change logging levels. * You want to log to a remote syslog server. Best regards Heinrich
Hi Heinrich, On Sun, 9 Feb 2020 at 15:33, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > On 2/9/20 11:21 PM, Sean Anderson wrote: > > On 2/9/20 4:59 PM, Heinrich Schuchardt wrote: > >> If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, > >> and for DEBUG=1 also debug messages. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > > > Why not just change the default for CONFIG_LOG to y? This is effectively > > the same, except it still allows users to completely disable logging > > altogether. > > > > --Sean > > > Reviewed-by: Simon Glass <sjg at chromium.org> > I have tested your suggestion for qemu_arm64_defconfig: > > Without my patch and CONFIG_LOG=n: > > u-boot.bin 664200 bytes > > With my patch and CONFIG_LOG=n: > > u-boot.bin 664432 bytes > > Without my patch but with CONFIG_LOG=y and CONFIG_CONSOLE=y: What is CONFIG_CONSOLE? > > u-boot.bin 666648 bytes > > So your suggestion consumes 2216 additional bytes to produce the > essentially the same console output. OK. That is a lot more than I thought. I'm not sure if it is possible to update the log test to cover your new case? > > IMHO CONFIG_LOG=y is currently only helpful in the following situation: > > * You are debugging your board and want to interactively change > logging levels. > * You want to log to a remote syslog server. Actually a major reason is that you want the full firmware log to be reported to Linux so you can check for warnings, etc. However we don't currently support this. Regards, Simon
On 2/11/20 12:13 AM, Simon Glass wrote: > Hi Heinrich, > > On Sun, 9 Feb 2020 at 15:33, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> On 2/9/20 11:21 PM, Sean Anderson wrote: >>> On 2/9/20 4:59 PM, Heinrich Schuchardt wrote: >>>> If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, >>>> and for DEBUG=1 also debug messages. >>>> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>> >>> Why not just change the default for CONFIG_LOG to y? This is effectively >>> the same, except it still allows users to completely disable logging >>> altogether. >>> >>> --Sean >>> >> > > Reviewed-by: Simon Glass <sjg at chromium.org> > >> I have tested your suggestion for qemu_arm64_defconfig: >> >> Without my patch and CONFIG_LOG=n: >> >> u-boot.bin 664200 bytes >> >> With my patch and CONFIG_LOG=n: >> >> u-boot.bin 664432 bytes >> >> Without my patch but with CONFIG_LOG=y and CONFIG_CONSOLE=y: > > What is CONFIG_CONSOLE? This is a typo. It should be CONFIG_LOG_CONSOLE. Thanks for reviewing. > >> >> u-boot.bin 666648 bytes >> >> So your suggestion consumes 2216 additional bytes to produce the >> essentially the same console output. > > OK. That is a lot more than I thought. > > I'm not sure if it is possible to update the log test to cover your new case? The current log test case in not a close fit, as filtering will be irrelevant. It should be possible to create a test using console recording (CONFIG_CONSOLE_RECORD=y). Looking at test/dm/test-main.c it seems that you once wanted to use console recording in a test but I could not identify any test actually using it up to now. Best regards Heinrich > >> >> IMHO CONFIG_LOG=y is currently only helpful in the following situation: >> >> * You are debugging your board and want to interactively change >> logging levels. >> * You want to log to a remote syslog server. > > Actually a major reason is that you want the full firmware log to be > reported to Linux so you can check for warnings, etc. However we don't > currently support this. > > Regards, > Simon >
Hi Heinrich, On Mon, 10 Feb 2020 at 21:17, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > > > On 2/11/20 12:13 AM, Simon Glass wrote: > > Hi Heinrich, > > > > On Sun, 9 Feb 2020 at 15:33, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > >> > >> On 2/9/20 11:21 PM, Sean Anderson wrote: > >>> On 2/9/20 4:59 PM, Heinrich Schuchardt wrote: > >>>> If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, > >>>> and for DEBUG=1 also debug messages. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > >>> > >>> Why not just change the default for CONFIG_LOG to y? This is effectively > >>> the same, except it still allows users to completely disable logging > >>> altogether. > >>> > >>> --Sean > >>> > >> > > > > Reviewed-by: Simon Glass <sjg at chromium.org> > > > >> I have tested your suggestion for qemu_arm64_defconfig: > >> > >> Without my patch and CONFIG_LOG=n: > >> > >> u-boot.bin 664200 bytes > >> > >> With my patch and CONFIG_LOG=n: > >> > >> u-boot.bin 664432 bytes > >> > >> Without my patch but with CONFIG_LOG=y and CONFIG_CONSOLE=y: > > > > What is CONFIG_CONSOLE? > > This is a typo. It should be CONFIG_LOG_CONSOLE. > > Thanks for reviewing. > > > > >> > >> u-boot.bin 666648 bytes > >> > >> So your suggestion consumes 2216 additional bytes to produce the > >> essentially the same console output. > > > > OK. That is a lot more than I thought. > > > > I'm not sure if it is possible to update the log test to cover your new case? > > The current log test case in not a close fit, as filtering will be > irrelevant. > > It should be possible to create a test using console recording > (CONFIG_CONSOLE_RECORD=y). > > Looking at test/dm/test-main.c it seems that you once wanted to use > console recording in a test but I could not identify any test actually > using it up to now. There is a pending pull request in dm/master which has this. One challenge might be that you need a test that only runs if CONFIG_LOG is not enabled. Perhaps you could use sandbox_spl for that? Regards, Simon
diff --git a/include/log.h b/include/log.h index 62fb8afbd0..0453876001 100644 --- a/include/log.h +++ b/include/log.h @@ -115,11 +115,11 @@ static inline int _log_nop(enum log_category_t cat, enum log_level_t level, #define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt) #else #define _LOG_MAX_LEVEL LOGL_INFO -#define log_err(_fmt...) log_nop(LOG_CATEGORY, LOGL_ERR, ##_fmt) -#define log_warning(_fmt...) log_nop(LOG_CATEGORY, LOGL_WARNING, ##_fmt) -#define log_notice(_fmt...) log_nop(LOG_CATEGORY, LOGL_NOTICE, ##_fmt) -#define log_info(_fmt...) log_nop(LOG_CATEGORY, LOGL_INFO, ##_fmt) -#define log_debug(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG, ##_fmt) +#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_info(_fmt, ...) printf(_fmt, ##__VA_ARGS__) +#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__) #define log_content(_fmt...) log_nop(LOG_CATEGORY, \ LOGL_DEBUG_CONTENT, ##_fmt) #define log_io(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt)
If CONFIG_LOG=n, we should still output errors, warnings, notices, infos, and for DEBUG=1 also debug messages. Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- include/log.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.24.1