Re: [PATCH v3 2/2] Add XFS messages to printk index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the useful feedback from all. I've integrated the
formatting suggestions and will follow up with a [PATCH v4] set.

My initial commit message describing a "semi-stable interface" was a
poor choice of words on my part. My idea of "interface" was users
comparing the index entries from release to release and developing on
top of that; I didn't take into consideration the contextual meaning
of "interface" in Kernel development. This change doesn't create or
alter any interfaces, this just makes the backing store of information
for an existing interface more correct and complete.
It is my hope that once these changes are made, XFS maintainers will
hardly need to engage with or keep track of this printk indexing
effort. These are only needed because printk() isn't called directly.

-- jof

On Wed, 30 Mar 2022 at 12:07, Chris Down <chris@xxxxxxxxxxxxxx> wrote:
>
> Jonathan Lassoff writes:
> >In order for end users to quickly react to new issues that come up in
> >production, it is proving useful to leverage the printk indexing system.
> >This printk index enables kernel developers to use calls to printk()
> >with changeable ad-hoc format strings, while still enabling end users
> >to detect changes from release to release.
> >
> >So that detailed XFS messages are captures by this printk index, this
> >patch wraps the xfs_<level> and xfs_alert_tag functions.
> >
> >Signed-off-by: Jonathan Lassoff <jof@xxxxxxxxxx>
>
> After Dave's suggested whitespace/ordering fixup, feel free to add:
>
> Reviewed-by: Chris Down <chris@xxxxxxxxxxxxxx>
>
> Thanks!
>
> >---
> >
> >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 |  2 +-
> > fs/xfs/xfs_message.h | 29 ++++++++++++++++++++---------
> > 2 files changed, 21 insertions(+), 10 deletions(-)
> >
> >diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
> >index ede8a4f2f676..f01efad20420 100644
> >--- a/fs/xfs/xfs_message.c
> >+++ b/fs/xfs/xfs_message.c
> >@@ -51,7 +51,7 @@ void xfs_printk_level(
> > }
> >
> > void
> >-xfs_alert_tag(
> >+_xfs_alert_tag(
> >       const struct xfs_mount  *mp,
> >       int                     panic_tag,
> >       const char              *fmt, ...)
> >diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
> >index 2f609800e806..6f9a1b67c4d7 100644
> >--- a/fs/xfs/xfs_message.h
> >+++ b/fs/xfs/xfs_message.h
> >@@ -6,34 +6,45 @@
> >
> > struct xfs_mount;
> >
> >+#define xfs_printk_index_wrap(kern_level, mp, fmt, ...)               \
> >+({                                                            \
> >+      printk_index_subsys_emit("%sXFS%s: ", kern_level, fmt); \
> >+      xfs_printk_level(kern_level, mp, fmt, ##__VA_ARGS__);   \
> >+})
> > extern __printf(3, 4)
> > 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__)
> >+      xfs_printk_index_wrap(KERN_EMERG, mp, fmt, ##__VA_ARGS__)
> > #define xfs_alert(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_ALERT, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_ALERT, mp, fmt, ##__VA_ARGS__)
> > #define xfs_crit(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_CRIT, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_CRIT, mp, fmt, ##__VA_ARGS__)
> > #define xfs_err(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_ERR, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_ERR, mp, fmt, ##__VA_ARGS__)
> > #define xfs_warn(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_WARNING, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_WARNING, mp, fmt, ##__VA_ARGS__)
> > #define xfs_notice(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_NOTICE, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_NOTICE, mp, fmt, ##__VA_ARGS__)
> > #define xfs_info(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_INFO, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_INFO, mp, fmt, ##__VA_ARGS__)
> > #ifdef DEBUG
> > #define xfs_debug(mp, fmt, ...) \
> >-      xfs_printk_level(KERN_DEBUG, mp, fmt, ##__VA_ARGS__)
> >+      xfs_printk_index_wrap(KERN_DEBUG, mp, fmt, ##__VA_ARGS__)
> > #else
> > #define xfs_debug(mp, fmt, ...) do {} while (0)
> > #endif
> >
> >+#define xfs_alert_tag(mp, tag, fmt, ...)                      \
> >+({                                                            \
> >+      printk_index_subsys_emit("%sXFS%s: ", KERN_ALERT, fmt); \
> >+      _xfs_alert_tag(mp, tag, fmt, ##__VA_ARGS__);            \
> >+})
> >+
> > extern __printf(3, 4)
> >-void xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
> >+void _xfs_alert_tag(const struct xfs_mount *mp, int tag, const char *fmt, ...);
> >
> >
> > #define xfs_printk_ratelimited(func, dev, fmt, ...)                   \
> >--
> >2.35.1
> >



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux