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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
--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
>
