Re: [RFC PATCH v2 2/4] jbd2: introduce some new log interfaces

[Jan Kara]

On Sat 23-01-21 20:00:44, Chunguang Xu wrote:
> From: Chunguang Xu <brookxu@xxxxxxxxxxx>
> 
> Compared to directly using numbers to indicate levels, using abstract
> error, warn, notice, info, debug to indicate levels may be more
> convenient for code reading and writing. Similar to other kernel
> modules, some basic log interfaces are introduced.
> 
> Signed-off-by: Chunguang Xu <brookxu@xxxxxxxxxxx>

One more thing I've noticed when reading this patch:

> +
> +#ifdef CONFIG_JBD2_DEBUG
> +/*
> + * Define JBD2_EXPENSIVE_CHECKING to enable more expensive internal
> + * consistency checks.  By default we don't do this unless
> + * CONFIG_JBD2_DEBUG is on.
> + */
> +#define JBD2_EXPENSIVE_CHECKING
> +extern ushort jbd2_journal_enable_debug;
> +void jbd2_log(int level, journal_t *j, const char *file, const char *func,
> +		      unsigned int line, const char *fmt, ...);
> +
> +#define JBD2_ERR	1	/* error conditions */
> +#define JBD2_WARN	2	/* warning conditions */
> +#define JBD2_NOTICE	3	/* normal but significant condition */
> +#define JBD2_INFO	4	/* informational */
> +#define JBD2_DEBUG	5	/* debug-level messages */

This is actually not true. All the jbd_debug() messages are really debug
messages, not errors, not warnings. It is just a different level of detail.
Honestly, these days, I'd rather discard all the levels, use pr_debug()
function to print these messages inside jdb2_debug() and defer to
CONFIG_DYNAMIC_DEBUG framework for configuration of which messages are
interesting for a particular debug session.

								Honza

> +
> +#define jbd2_err(j, fmt, a...)						\
> +	jbd2_log(JBD2_ERR, j, __FILE__, __func__, __LINE__, (fmt), ##a)
> +
> +#define jbd2_warn(j, fmt, a...)						\
> +	jbd2_log(JBD2_WARN, j, __FILE__, __func__, __LINE__, (fmt), ##a)
> +
> +#define jbd2_notice(j, fmt, a...)					\
> +	jbd2_log(JBD2_NOTICE, j, __FILE__, __func__, __LINE__, (fmt), ##a)
> +
> +#define jbd2_info(j, fmt, a...)						\
> +	jbd2_log(JBD2_INFO, j, __FILE__, __func__, __LINE__, (fmt), ##a)
> +
> +#define jbd2_debug(j, fmt, a...)					\
> +	jbd2_log(JBD2_DEBUG, j, __FILE__, __func__, __LINE__, (fmt), ##a)
> +
> +#else
> +
> +#define jbd2_err(j, fmt, a...)
> +#define jbd2_warn(j, fmt, a...)
> +#define jbd2_notice(j, fmt, a...)
> +#define jbd2_info(j, fmt, a...)
> +#define jbd2_debug(j, fmt, a...)
> +
> +#endif
>  #endif
>  
>  /*
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR