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 changable
ad-hoc format strings, while still enabling end users to detect changes and
develop a semi-stable interface for detecting and parsing these messages.

So that detailed FAT messages are captured by this printk index, this patch
wraps fat_msg with a macro.

PATCH v1 -- Fix indentation with tabs in fat_msg macro
PATCH v2 -- Define FAT_PRINTK_PREFIX
PATCH v3 -- Fix kernel-doc comment for _fat_msg()

Reported-by: kernel test robot <lkp@intel.com>
---
 fs/fat/fat.h  |  9 ++++++++-
 fs/fat/misc.c | 14 ++++++++++----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 02d4d4234956..2a20a21f2fb9 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -433,8 +433,15 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...);
 	__fat_fs_error(sb, 1, fmt , ## args)
 #define fat_fs_error_ratelimit(sb, fmt, args...) \
 	__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
+
+#define FAT_PRINTK_PREFIX "%sFAT-fs (%s): "
+#define fat_msg(sb, level, fmt, args...)				\
+do {									\
+	printk_index_subsys_emit(FAT_PRINTK_PREFIX, level, fmt, ##args);\
+	_fat_msg(sb, level, fmt, ##args);				\
+} while(0)
 __printf(3, 4) __cold
-void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
+void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
 #define fat_msg_ratelimit(sb, level, fmt, args...)	\
 	do {	\
 			if (__ratelimit(&MSDOS_SB(sb)->ratelimit))	\
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 91ca3c304211..855477d89f41 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -42,10 +42,16 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
 EXPORT_SYMBOL_GPL(__fat_fs_error);
 
 /**
- * fat_msg() - print preformated FAT specific messages. Every thing what is
- * not fat_fs_error() should be fat_msg().
+ * _fat_msg() - Print a preformatted FAT message based on a superblock.
+ * @sb: A pointer to a &struct super_block
+ * @level: A Kernel printk level constant
+ * @fmt: The printf-style format string to print.
+ *
+ * Everything that is not fat_fs_error() should be fat_msg().
+ *
+ * fat_msg() wraps _fat_msg() for printk indexing.
  */
-void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
+void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -53,7 +59,7 @@ void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
+	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id, &vaf);
 	va_end(args);
 }
 
-- 
2.30.2

On Thu, 24 Mar 2022 02:19:58 +0000 Jonathan Lassoff <jof@thejof.com> 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 changable
> ad-hoc format strings, while still enabling end users to detect changes and
> develop a semi-stable interface for detecting and parsing these messages.
> 
> So that detailed FAT messages are captured by this printk index, this patch
> wraps fat_msg with a macro.

It would be nice to see some before-and-after sample output to help
reviewers understand this proposal.

> Reported-by: kernel test robot <lkp@intel.com>

We'll need a Signed-off-by: for this, please. 
Documentation/process/submitting-patches.rst describes this.

On Fri, 25 Mar 2022 at 20:18, Andrew Morton <akpm@linux-foundation.org> wrote:
> It would be nice to see some before-and-after sample output to help
> reviewers understand this proposal.

By diffing the output of the debugfs interface
(/sys/kernel/debug/printk/index/vmlinux), it looks like this:

--- before
+++ after
-<-1> fs/fat/misc.c:56 fat_msg "%sFAT-fs (%s): %pV\n"
+<3> fs/fat/dir.c:1323 fat_add_entries "%sFAT-fs (%s): Corrupted
directory (i_pos %lld)"
+<4> fs/fat/dir.c:1066 fat_remove_entries "%sFAT-fs (%s): Couldn't
remove the long name slots"
+<4> fs/fat/dir.c:173 uni16_to_x8 "%sFAT-fs (%s): filename was
truncated while converting."
+<3> fs/fat/dir.c:102 fat__get_entry "%sFAT-fs (%s): Directory
bread(block %llu) failed"
+<3> fs/fat/fatent.c:110 fat_ent_bread "%sFAT-fs (%s): FAT read failed
(blocknr %llu)"
+<3> fs/fat/fatent.c:97 fat12_ent_bread "%sFAT-fs (%s): FAT read
failed (blocknr %llu)"
+<6> fs/fat/inode.c:1889 fat_fill_super "%sFAT-fs (%s): Can't find a
valid FAT filesystem"
+<4> fs/fat/inode.c:1878 fat_fill_super "%sFAT-fs (%s): mounting with
\"discard\" option, but the device does not support discard"
+<3> fs/fat/inode.c:1871 fat_fill_super "%sFAT-fs (%s): get root inode failed"
+<3> fs/fat/inode.c:1837 fat_fill_super "%sFAT-fs (%s): IO charset %s not found"
+<3> fs/fat/inode.c:1829 fat_fill_super "%sFAT-fs (%s): codepage %s not found"
+<3> fs/fat/inode.c:1794 fat_fill_super "%sFAT-fs (%s): count of
clusters too big (%u)"
+<3> fs/fat/inode.c:1766 fat_fill_super "%sFAT-fs (%s): bogus number
of directory entries (%u)"
+<4> fs/fat/inode.c:1738 fat_fill_super "%sFAT-fs (%s): Invalid FSINFO
signature: 0x%08x, 0x%08x (sector = %lu)"
+<3> fs/fat/inode.c:1731 fat_fill_super "%sFAT-fs (%s): bread failed,
FSINFO block (sector = %lu)"
+<3> fs/fat/inode.c:1688 fat_fill_super "%sFAT-fs (%s): unable to read
boot sector (logical sector size = %lu)"
+<3> fs/fat/inode.c:1680 fat_fill_super "%sFAT-fs (%s): unable to set
blocksize %u"
+<3> fs/fat/inode.c:1671 fat_fill_super "%sFAT-fs (%s): logical sector
size too small for device (logical sector size = %u)"
+<3> fs/fat/inode.c:1650 fat_fill_super "%sFAT-fs (%s): unable to read
boot sector"
+<6> fs/fat/inode.c:1580 fat_read_static_bpb "%sFAT-fs (%s): This
looks like a DOS 1.x volume; assuming default BPB values"
+<4> fs/fat/inode.c:1573 fat_read_static_bpb "%sFAT-fs (%s): This
looks like a DOS 1.x volume, but isn't a recognized floppy size (%llu
sectors)"
+<3> fs/fat/inode.c:1559 fat_read_static_bpb "%sFAT-fs (%s): %s; DOS
2.x BPB is non-zero"
+<3> fs/fat/inode.c:1548 fat_read_static_bpb "%sFAT-fs (%s): %s; no
bootstrapping code"
+<3> fs/fat/inode.c:1525 fat_read_bpb "%sFAT-fs (%s): bogus number of
FAT sectors"
+<3> fs/fat/inode.c:1511 fat_read_bpb "%sFAT-fs (%s): bogus logical
sector size %u"
+<3> fs/fat/inode.c:1502 fat_read_bpb "%sFAT-fs (%s): invalid media
value (0x%02x)"
+<3> fs/fat/inode.c:1491 fat_read_bpb "%sFAT-fs (%s): bogus number of
FAT structure"
+<3> fs/fat/inode.c:1485 fat_read_bpb "%sFAT-fs (%s): bogus number of
reserved sectors"
+<4> fs/fat/inode.c:1366 parse_options "%sFAT-fs (%s): utf8 is not a
recommended IO charset for FAT filesystems, filesystem will be case
sensitive!"
+<3> fs/fat/inode.c:1355 parse_options "%sFAT-fs (%s): Unrecognized
mount option \"%s\" or missing value"
+<6> fs/fat/inode.c:1349 parse_options "%sFAT-fs (%s): \"%s\" option
is obsolete, not supported now"
+<3> fs/fat/inode.c:869 __fat_write_inode "%sFAT-fs (%s): unable to
read inode block for updating (i_pos %lld)"
+<3> fs/fat/inode.c:690 fat_set_state "%sFAT-fs (%s): unable to read
boot sector to mark fs as dirty"
+<4> fs/fat/inode.c:682 fat_set_state "%sFAT-fs (%s): Volume was not
properly unmounted. Some data may be corrupt. Please run fsck."
+<4> fs/fat/inode.c:643 fat_free_eofblocks "%sFAT-fs (%s): Failed to
update on disk inode for unused fallocated blocks, inode could be
corrupted. Please run fsck"
+<3> fs/fat/misc.c:86 fat_clusters_flush "%sFAT-fs (%s): Invalid
FSINFO signature: 0x%08x, 0x%08x (sector = %lu)"
+<3> fs/fat/misc.c:79 fat_clusters_flush "%sFAT-fs (%s): bread failed
in fat_clusters_flush"
+<3> fs/fat/misc.c:39 __fat_fs_error "%sFAT-fs (%s): Filesystem has
been set read-only"
+<3> fs/fat/misc.c:31 __fat_fs_error "%sFAT-fs (%s): error, %pV"
+<3> fs/fat/nfs.c:225 fat_rebuild_parent "%sFAT-fs (%s): unable to
read cluster of parent directory"
+<3> fs/fat/nfs.c:73 __fat_nfs_get_inode "%sFAT-fs (%s): unable to
read block(%llu) for building NFS inode"

> > Reported-by: kernel test robot <lkp@intel.com>
>
> We'll need a Signed-off-by: for this, please.
> Documentation/process/submitting-patches.rst describes this.

*facepalm*
Sorry about that and thanks for some patience.... I'm still a total newbie.

In some of the other subsystems, I've gotten some resistance to
mentioning a "semi-stable interface" (with regard to the existing
debugfs interface, since it's not actually making any stability
guarantees), so I'd like to follow up again with a [PATCH v4] that
rewords the message and includes a Signed-off-by:

-- jof

Jonathan Lassoff <jof@thejof.com> 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 changable
> ad-hoc format strings, while still enabling end users to detect changes and
> develop a semi-stable interface for detecting and parsing these messages.
>
> So that detailed FAT messages are captured by this printk index, this patch
> wraps fat_msg with a macro.
>
> PATCH v1 -- Fix indentation with tabs in fat_msg macro
> PATCH v2 -- Define FAT_PRINTK_PREFIX
> PATCH v3 -- Fix kernel-doc comment for _fat_msg()
>
> Reported-by: kernel test robot <lkp@intel.com>

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Thanks.

> ---
>  fs/fat/fat.h  |  9 ++++++++-
>  fs/fat/misc.c | 14 ++++++++++----
>  2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index 02d4d4234956..2a20a21f2fb9 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -433,8 +433,15 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...);
>  	__fat_fs_error(sb, 1, fmt , ## args)
>  #define fat_fs_error_ratelimit(sb, fmt, args...) \
>  	__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
> +
> +#define FAT_PRINTK_PREFIX "%sFAT-fs (%s): "
> +#define fat_msg(sb, level, fmt, args...)				\
> +do {									\
> +	printk_index_subsys_emit(FAT_PRINTK_PREFIX, level, fmt, ##args);\
> +	_fat_msg(sb, level, fmt, ##args);				\
> +} while(0)
>  __printf(3, 4) __cold
> -void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
> +void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
>  #define fat_msg_ratelimit(sb, level, fmt, args...)	\
>  	do {	\
>  			if (__ratelimit(&MSDOS_SB(sb)->ratelimit))	\
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 91ca3c304211..855477d89f41 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -42,10 +42,16 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
>  EXPORT_SYMBOL_GPL(__fat_fs_error);
>  
>  /**
> - * fat_msg() - print preformated FAT specific messages. Every thing what is
> - * not fat_fs_error() should be fat_msg().
> + * _fat_msg() - Print a preformatted FAT message based on a superblock.
> + * @sb: A pointer to a &struct super_block
> + * @level: A Kernel printk level constant
> + * @fmt: The printf-style format string to print.
> + *
> + * Everything that is not fat_fs_error() should be fat_msg().
> + *
> + * fat_msg() wraps _fat_msg() for printk indexing.
>   */
> -void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
> +void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -53,7 +59,7 @@ void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
> -	printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
> +	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id, &vaf);
>  	va_end(args);
>  }

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>