On 1/3/20 8:56 PM, Dave Chinner wrote:

On Mon, Dec 23, 2019 at 04:55:58PM -0600, Tony Asleson wrote:

quoted

Add persistent durable name to xfs messages so we can
correlate them with other messages for the same block
device.

Signed-off-by: Tony Asleson <redacted>
---
 fs/xfs/xfs_message.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
index 9804efe525a9..8447cdd985b4 100644
--- a/fs/xfs/xfs_message.c
+++ b/fs/xfs/xfs_message.c

@@ -20,6 +20,23 @@ __xfs_printk(
 	const struct xfs_mount	*mp,
 	struct va_format	*vaf)
 {
+	char dict[128];
+	int dict_len = 0;
+
+	if (mp && mp->m_super && mp->m_super->s_bdev &&
+		mp->m_super->s_bdev->bd_disk) {
+		dict_len = dev_durable_name(
+			disk_to_dev(mp->m_super->s_bdev->bd_disk)->parent,
+			dict,
+			sizeof(dict));
+		if (dict_len) {
+			printk_emit(
+				0, level[1] - '0', dict, dict_len,
+				"XFS (%s): %pV\n",  mp->m_fsname, vaf);
+			return;
+		}
+	}

NACK on the ground this is a gross hack.

James suggested I utilize dev_printk, which does make things simpler.
Would something like this be acceptable?

diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
index 9804efe525a9..0738c74a8d3a 100644
--- a/fs/xfs/xfs_message.c
+++ b/fs/xfs/xfs_message.c

@@ -20,11 +20,18 @@ __xfs_printk(
        const struct xfs_mount  *mp,
        struct va_format        *vaf)
 {
+       struct device *dev = NULL;
+
+       if (mp && mp->m_super && mp->m_super->s_bdev &&
+               mp->m_super->s_bdev->bd_disk) {
+               dev = disk_to_dev(mp->m_super->s_bdev->bd_disk)->parent;
+       }
+
        if (mp && mp->m_fsname) {
-               printk("%sXFS (%s): %pV\n", level, mp->m_fsname, vaf);
+               dev_printk(level, dev, "XFS (%s): %pV\n", mp->m_fsname,

vaf);
                return;
        }
-       printk("%sXFS: %pV\n", level, vaf);
+       dev_printk(level, dev, "XFS: %pV\n", vaf);
 }

quoted

+
 	if (mp && mp->m_fsname) {

mp->m_fsname is the name of the device we use everywhere for log
messages, it's set up at mount time so we don't have to do runtime
evaulation of the device name every time we need to emit the device
name in a log message.

So, if you have some sooper speshial new device naming scheme, it
needs to be stored into the struct xfs_mount to replace mp->m_fsname.

I don't think we want to replace mp->m_fsname with the vpd 0x83 device
identifier.  This proposed change is adding a key/value structured data
to the log message for non-ambiguous device identification over time,
not to place the ID in the human readable portion of the message.  The
existing name is useful too, especially when it involves a partition.

And if you have some sooper spehsial new printk API that uses this
new device name, everything XFS emits needs to use it
unconditionally as we do with mp->m_fsname now.

IOWs, this isn't conditional code - it either works for the entire
life of the mount for every message we have to emit with a single
setup call, or the API is broken and needs to be rethought.

I've been wondering why the struct scsi device uses rcu data for the vpd
as I would not think that it would be changing for a specific device.
Perhaps James can shed some light on this?

-Tony