On Wed 2022-03-30 10:54:41, Dave Chinner wrote: > On Fri, Mar 25, 2022 at 10:19:45AM -0700, Jonathan Lassoff wrote: > > 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@xxxxxxxxxx> > > Mostly looks good, mainly just whitespace/formatting consistency > issues now. > > .... > > -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, ...) > > Use the same format as __xfs_printk() and xfs_alert_tag(): > > 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, ...); > > This still needs the __printf() attribute because we still want the > compiler to check the printf format string for issues. Also the > format for function prototypes should follow the ones that got > removed: It is actually there. But it is hidden in many removed lines. BTW: I missed it when reading the patch as well. I was surprised when I saw it after applying the patch ;-) Best Regards, Petr