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

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

 



Hi Petr,

Petr Mladek writes:
On Fri 2022-03-25 10:19:46, Jonathan Lassoff wrote:
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>

--- 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);	\

I would probably use "%sXFS: " for the first parameter as
a compromise here.

It affects how the printk formats are shown in debugfs. With the
current patch I see in /sys/kernel/debug/printk/index/vmlinux:

<4> fs/xfs/libxfs/xfs_ag.c:877 xfs_ag_shrink_space "%sXFS%s: Error %d reserving per-AG metadata reserve pool."
<1> fs/xfs/libxfs/xfs_ag.c:151 xfs_initialize_perag_data "%sXFS%s: AGF corruption. Please run xfs_repair."
<4> fs/xfs/libxfs/xfs_alloc.c:2429 xfs_agfl_reset "%sXFS%s: WARNING: Reset corrupted AGFL on AG %u. %d blocks leaked. Please unmount and run xfs_repair."
<4> fs/xfs/libxfs/xfs_alloc.c:262 xfs_alloc_get_rec "%sXFS%s: start block 0x%x block count 0x%x"
<4> fs/xfs/libxfs/xfs_alloc.c:260 xfs_alloc_get_rec "%sXFS%s: %s Freespace BTree record corruption in AG %d detected!"
<1> fs/xfs/libxfs/xfs_attr_remote.c:304 xfs_attr_rmtval_copyout "%sXFS%s: remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)"
<4> fs/xfs/libxfs/xfs_bmap.c:1129 xfs_iread_bmbt_block "%sXFS%s: corrupt dinode %llu, (btree extents)."

In reality, the prefix is chosen in __xfs_printk() at runtime:

	+ "%sXFS (%s): "	when mp->m_super is defined
	+ "%sXFS: "		otherwise

It means that "%sXFS: " is not perfect but it looks closer to reality
than "%sXFS%s: ".

I think we do actually want "%sXFS%s: " here. Without that, it's not possible to be confident marrying up a userspace detector to its original printk counterpart if the detector actually looks at what's in mp->m_super->s_id (eg. to exclude or include some device).

Some messages in practice also typically only ever come out with mp->m_super present, so the userspace detector is likely to accomodate for that whether it uses the data in mp->m_super->s_id or not. Since we can't detect which printks those are at compile time, we pretty much have to use "%sXFS%s: ".

Thanks,

Chris



[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