Re: [PATCH v3 1/2] Simplify XFS logging methods.

[Chris Down <chris@chrisdown.name>]

Jonathan Lassoff writes:
>Rather than have a constructor to define many nearly-identical
>functions, use preprocessor macros to pass down a kernel logging level
>to a common function.
>
>Signed-off-by: Jonathan Lassoff <jof@thejof.com>

This is a good simplification, thanks.

Reviewed-by: Chris Down <chris@chrisdown.name>

>---
>
>Notes:
>    [PATCH v1]
>      * De-duplicate kernel logging levels and tidy whitespace.
>    [PATCH v2]
>      * Split changes into two patches:
>         - function and prototype de-duplication.
>         - Adding printk indexing
>    [PATCH v3]
>      * Fix some whitespace and semicolon. *facepalm*
>
> fs/xfs/xfs_message.c | 53 ++++++++++++++++++--------------------------
> fs/xfs/xfs_message.h | 47 +++++++++++++++++++++------------------
> 2 files changed, 47 insertions(+), 53 deletions(-)
>
>diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
>index bc66d95c8d4c..ede8a4f2f676 100644
>--- a/fs/xfs/xfs_message.c
>+++ b/fs/xfs/xfs_message.c
>@@ -27,37 +27,28 @@ __xfs_printk(
> 	printk("%sXFS: %pV\n", level, vaf);
> }
>
>-#define define_xfs_printk_level(func, kern_level)		\
>-void func(const struct xfs_mount *mp, const char *fmt, ...)	\
>-{								\
>-	struct va_format	vaf;				\
>-	va_list			args;				\
>-	int			level;				\
>-								\
>-	va_start(args, fmt);					\
>-								\
>-	vaf.fmt = fmt;						\
>-	vaf.va = &args;						\
>-								\
>-	__xfs_printk(kern_level, mp, &vaf);			\
>-	va_end(args);						\
>-								\
>-	if (!kstrtoint(kern_level, 0, &level) &&		\
>-	    level <= LOGLEVEL_ERR &&				\
>-	    xfs_error_level >= XFS_ERRLEVEL_HIGH)		\
>-		xfs_stack_trace();				\
>-}								\
>-
>-define_xfs_printk_level(xfs_emerg, KERN_EMERG);
>-define_xfs_printk_level(xfs_alert, KERN_ALERT);
>-define_xfs_printk_level(xfs_crit, KERN_CRIT);
>-define_xfs_printk_level(xfs_err, KERN_ERR);
>-define_xfs_printk_level(xfs_warn, KERN_WARNING);
>-define_xfs_printk_level(xfs_notice, KERN_NOTICE);
>-define_xfs_printk_level(xfs_info, KERN_INFO);
>-#ifdef DEBUG
>-define_xfs_printk_level(xfs_debug, KERN_DEBUG);
>-#endif
>+void xfs_printk_level(
>+	const char *kern_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;
>+	vaf.va = &args;
>+
>+	__xfs_printk(kern_level, mp, &vaf);
>+
>+	va_end(args);
>+
>+	if (!kstrtoint(kern_level, 0, &level) &&
>+	    level <= LOGLEVEL_ERR &&
>+	    xfs_error_level >= XFS_ERRLEVEL_HIGH)
>+		xfs_stack_trace();
>+}
>
> void
> xfs_alert_tag(
>diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
>index bb9860ec9a93..2f609800e806 100644
>--- a/fs/xfs/xfs_message.h
>+++ b/fs/xfs/xfs_message.h
>@@ -6,33 +6,36 @@
>
> struct xfs_mount;
>
>-extern __printf(2, 3)
>-void xfs_emerg(const struct xfs_mount *mp, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_alert(const struct xfs_mount *mp, const char *fmt, ...);
> extern __printf(3, 4)
>-void xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_crit(const struct xfs_mount *mp, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_err(const struct xfs_mount *mp, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_warn(const struct xfs_mount *mp, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_notice(const struct xfs_mount *mp, const char *fmt, ...);
>-extern __printf(2, 3)
>-void xfs_info(const struct xfs_mount *mp, const char *fmt, ...);
>-
>+void xfs_printk_level(
>+	const char *kern_level,
>+	const struct xfs_mount *mp,
>+	const char *fmt, ...);
>+#define xfs_emerg(mp, fmt, ...) \
>+	xfs_printk_level(KERN_EMERG, mp, fmt, ##__VA_ARGS__)
>+#define xfs_alert(mp, fmt, ...) \
>+	xfs_printk_level(KERN_ALERT, mp, fmt, ##__VA_ARGS__)
>+#define xfs_crit(mp, fmt, ...) \
>+	xfs_printk_level(KERN_CRIT, mp, fmt, ##__VA_ARGS__)
>+#define xfs_err(mp, fmt, ...) \
>+	xfs_printk_level(KERN_ERR, mp, fmt, ##__VA_ARGS__)
>+#define xfs_warn(mp, fmt, ...) \
>+	xfs_printk_level(KERN_WARNING, mp, fmt, ##__VA_ARGS__)
>+#define xfs_notice(mp, fmt, ...) \
>+	xfs_printk_level(KERN_NOTICE, mp, fmt, ##__VA_ARGS__)
>+#define xfs_info(mp, fmt, ...) \
>+	xfs_printk_level(KERN_INFO, mp, fmt, ##__VA_ARGS__)
> #ifdef DEBUG
>-extern __printf(2, 3)
>-void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...);
>+#define xfs_debug(mp, fmt, ...) \
>+	xfs_printk_level(KERN_DEBUG, mp, fmt, ##__VA_ARGS__)
> #else
>-static inline __printf(2, 3)
>-void xfs_debug(const struct xfs_mount *mp, const char *fmt, ...)
>-{
>-}
>+#define xfs_debug(mp, fmt, ...) do {} while (0)
> #endif
>
>+extern __printf(3, 4)
>+void xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
>+
>+
> #define xfs_printk_ratelimited(func, dev, fmt, ...)			\
> do {									\
> 	static DEFINE_RATELIMIT_STATE(_rs,				\
>
>base-commit: 34af78c4e616c359ed428d79fe4758a35d2c5473
>-- 
>2.35.1
>