Re: [PATCH v2 1/2] xfs: fix xfs print level wrong parsing

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Apr 21, 2023 at 07:37:15PM +0800, Guo Xuenan wrote:
> Recently, during my xfs bugfix work, notice a bug that makes
> xfs_stack_trace never take effect. This has been around here at xfs
> debug framework for a long time.
> 
> The root cause is misuse of `kstrtoint` which always return -EINVAL,
> because KERN_<LEVEL> with KERN_SOH prefix will always parse failed.
> Directly set loglevel in xfs print definition to make it work properly.
> 
> Fixes: 847f9f6875fb ("xfs: more info from kmem deadlocks and high-level error msgs")
> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
> ---
>  fs/xfs/xfs_message.c |  5 ++---
>  fs/xfs/xfs_message.h | 28 ++++++++++++++--------------
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
> index 8f495cc23903..1cfa21d62514 100644
> --- a/fs/xfs/xfs_message.c
> +++ b/fs/xfs/xfs_message.c
> @@ -30,12 +30,12 @@ __xfs_printk(
>  void
>  xfs_printk_level(
>  	const char *kern_level,
> +	const int log_level,
>  	const struct xfs_mount *mp,
>  	const char *fmt, ...)
>  {
>  	struct va_format	vaf;
>  	va_list			args;
> -	int			level;
>  
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
> @@ -45,8 +45,7 @@ xfs_printk_level(
>  
>  	va_end(args);
>  
> -	if (!kstrtoint(kern_level, 0, &level) &&
> -	    level <= LOGLEVEL_ERR &&
> +	if (log_level <= LOGLEVEL_ERR &&

Ok, this resolves my occasional squinting at this and wondering "how in
the h*** does this work?" after I set the log level, fail to get any
messages, and then decide to just do it with ftrace so I can concentrate
on finishing the problem I'm working on.

IOWs, thank you for sorting this out finally.

>  	    xfs_error_level >= XFS_ERRLEVEL_HIGH)
>  		xfs_stack_trace();
>  }
> diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
> index cc323775a12c..666a549eb989 100644
> --- a/fs/xfs/xfs_message.h
> +++ b/fs/xfs/xfs_message.h
> @@ -6,32 +6,32 @@
>  
>  struct xfs_mount;
>  
> -extern __printf(3, 4)
> -void xfs_printk_level(const char *kern_level, const struct xfs_mount *mp,
> -			const char *fmt, ...);
> +extern __printf(4, 5)
> +void xfs_printk_level(const char *kern_level, const int log_level,
> +		const struct xfs_mount *mp, const char *fmt, ...);
>  
> -#define xfs_printk_index_wrap(kern_level, mp, fmt, ...)		\
> +#define xfs_printk_index_wrap(level, mp, fmt, ...)		\
>  ({								\
> -	printk_index_subsys_emit("%sXFS%s: ", kern_level, fmt);	\
> -	xfs_printk_level(kern_level, mp, fmt, ##__VA_ARGS__);	\
> +	printk_index_subsys_emit("%sXFS%s: ", KERN_##level, fmt);	\
> +	xfs_printk_level(KERN_##level, LOGLEVEL_##level, mp, fmt, ##__VA_ARGS__); \

Not in love with the macro mess, but it seems to get the job done.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  })
>  #define xfs_emerg(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_EMERG, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(EMERG, mp, fmt, ##__VA_ARGS__)
>  #define xfs_alert(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_ALERT, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(ALERT, mp, fmt, ##__VA_ARGS__)
>  #define xfs_crit(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_CRIT, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(CRIT, mp, fmt, ##__VA_ARGS__)
>  #define xfs_err(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_ERR, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(ERR, mp, fmt, ##__VA_ARGS__)
>  #define xfs_warn(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_WARNING, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(WARNING, mp, fmt, ##__VA_ARGS__)
>  #define xfs_notice(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_NOTICE, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(NOTICE, mp, fmt, ##__VA_ARGS__)
>  #define xfs_info(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_INFO, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(INFO, mp, fmt, ##__VA_ARGS__)
>  #ifdef DEBUG
>  #define xfs_debug(mp, fmt, ...) \
> -	xfs_printk_index_wrap(KERN_DEBUG, mp, fmt, ##__VA_ARGS__)
> +	xfs_printk_index_wrap(DEBUG, mp, fmt, ##__VA_ARGS__)
>  #else
>  #define xfs_debug(mp, fmt, ...) do {} while (0)
>  #endif
> -- 
> 2.31.1
>