Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
* [v4 00/11] Add persistent durable identifier to storage log messages
@ 2020-07-24 17:16 Tony Asleson
  2020-07-24 17:16 ` [v4 01/11] struct device: Add function callback durable_name Tony Asleson
                   ` (11 more replies)
  0 siblings, 12 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:16 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Today users have no easy way to correlate kernel log messages for storage
devices across reboots, device dynamic add/remove, or when the device is
physically or logically moved from from system to system.  This is due
to the existing log IDs which identify how the device is attached and not
a unique ID of what is attached.  Additionally, even when the attachment
hasn't changed, it's not always obvious which messages belong to the
device as the different areas in the storage stack use different
identifiers, eg. (sda, sata1.00, sd 0:0:0:0).

This change addresses this by adding a unique ID to each log
message.  It couples the existing structured key/value logging capability
and VPD 0x83 device identification.


Some examples of logs filtered for a specific device utilizing this patch
series.

$ journalctl -b  _KERNEL_DURABLE_NAME="`cat /sys/block/sdb/device/wwid`" 
| cut -c 25- | fmt -t
l: scsi 1:0:0:0: Attached scsi generic sg1 type 0
l: sd 1:0:0:0: [sdb] 209715200 512-byte logical blocks: (107 GB/100 GiB)
l: sd 1:0:0:0: [sdb] Write Protect is off
l: sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00
l: sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
   support DPO or FUA
l: sd 1:0:0:0: [sdb] Attached SCSI disk
l: ata2.00: exception Emask 0x0 SAct 0x8 SErr 0x8 action 0x6 frozen
l: ata2.00: failed command: READ FPDMA QUEUED
l: ata2.00: cmd 60/01:18:10:27:00/00:00:00:00:00/40 tag 3 ncq dma 512
            in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
l: ata2.00: status: { DRDY }
l: ata2.00: configured for UDMA/100
l: ata2.00: device reported invalid CHS sector 0
l: ata2.00: exception Emask 0x0 SAct 0x4000 SErr 0x4000 action 0x6 frozen
l: ata2.00: failed command: READ FPDMA QUEUED
l: ata2.00: cmd 60/01:70:10:27:00/00:00:00:00:00/40 tag 14 ncq dma 512
            in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
l: ata2.00: status: { DRDY }
l: ata2.00: configured for UDMA/100
l: ata2.00: device reported invalid CHS sector 0
l: ata2.00: exception Emask 0x0 SAct 0x80000000 SErr 0x80000000 action
            0x6 frozen
l: ata2.00: failed command: READ FPDMA QUEUED
l: ata2.00: cmd 60/01:f8:10:27:00/00:00:00:00:00/40 tag 31 ncq dma 512
            in res 40/00:ff:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
l: ata2.00: status: { DRDY }
l: ata2.00: configured for UDMA/100
l: ata2.00: NCQ disabled due to excessive errors
l: ata2.00: exception Emask 0x0 SAct 0x40000 SErr 0x40000 action 0x6
            frozen
l: ata2.00: failed command: READ FPDMA QUEUED
l: ata2.00: cmd 60/01:90:10:27:00/00:00:00:00:00/40 tag 18 ncq dma 512
            in res 40/00:01:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
l: ata2.00: status: { DRDY }
l: ata2.00: configured for UDMA/100

$ journalctl -b  _KERNEL_DURABLE_NAME="`cat /sys/block/nvme0n1/wwid`" 
| cut -c 25- | fmt -t
l: blk_update_request: critical medium error, dev nvme0n1, sector 10000
   op 0x0:(READ) flags 0x80700 phys_seg 4 prio class 0
l: blk_update_request: critical medium error, dev nvme0n1, sector 10000
   op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
l: Buffer I/O error on dev nvme0n1, logical block 1250, async page read

$ journalctl -b  _KERNEL_DURABLE_NAME="`cat /sys/block/sdc/device/wwid`"
| cut -c 25- | fmt -t
l: sd 8:0:0:0: Power-on or device reset occurred
l: sd 8:0:0:0: [sdc] 16777216 512-byte logical blocks: (8.59 GB/8.00 GiB)
l: sd 8:0:0:0: Attached scsi generic sg2 type 0
l: sd 8:0:0:0: [sdc] Write Protect is off
l: sd 8:0:0:0: [sdc] Mode Sense: 63 00 00 08
l: sd 8:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't
   support DPO or FUA
l: sd 8:0:0:0: [sdc] Attached SCSI disk
l: sd 8:0:0:0: [sdc] tag#255 FAILED Result: hostbyte=DID_OK
   driverbyte=DRIVER_SENSE cmd_age=0s
l: sd 8:0:0:0: [sdc] tag#255 Sense Key : Medium Error [current]
l: sd 8:0:0:0: [sdc] tag#255 Add. Sense: Unrecovered read error
l: sd 8:0:0:0: [sdc] tag#255 CDB: Read(10) 28 00 00 00 27 10 00 00 01 00
l: blk_update_request: critical medium error, dev sdc, sector 10000 op
   0x0:(READ) flags 0x0 phys_seg 1 prio class 0

There should be no changes to the log message content with this patch series.
I ran release kernel and this patch series and did a compare while forcing the
kernel through the same errors paths to verify.

The first 5 commits in the patch series utilize changes needed for dev_printk
code path.  The last 6 commits in the patch add the needed changes to utilize
durable_name_printk.  The function durable_name_printk is nothing more than
a printk that adds structured key/value durable name to unmodified printk
output.  I structured it this way so only a subset of the patch series could
be theoretically applied if we cannot get agreement on complete patch series.

v2:
- Incorporated changes suggested by James Bottomley
- Removed string function which removed leading/trailing/duplicate adjacent
  spaces from generated id, value matches /sys/block/<device>/device/wwid
- Remove xfs patch, limiting changes to lower block layers
- Moved callback from struct device_type to struct device.  Struct device_type
  is typically static const and with a number of different areas using shared
  implementation of genhd unable to modify for each of the different areas.

v3:
- Increase the size of the buffers for NVMe id generation and
  dev_vprintk_emit
  
v4:
- Back out dev_printk for those locations that weren't using it before, so that
  we don't change the content of the user visible log message by using a
  function durable_name_printk.
- Remove RFC from patch series.

Tony Asleson (11):
  struct device: Add function callback durable_name
  create_syslog_header: Add durable name
  dev_vprintk_emit: Increase hdr size
  scsi: Add durable_name for dev_printk
  nvme: Add durable name for dev_printk
  libata: Add ata_scsi_durable_name
  Add durable_name_printk
  libata: use durable_name_printk
  Add durable_name_printk_ratelimited
  print_req_error: Use durable_name_printk_ratelimited
  buffer_io_error: Use durable_name_printk_ratelimited

 block/blk-core.c           |  5 ++++-
 drivers/ata/libata-core.c  | 17 +++++++-------
 drivers/ata/libata-scsi.c  | 13 ++++++++++-
 drivers/base/core.c        | 46 +++++++++++++++++++++++++++++++++++++-
 drivers/nvme/host/core.c   | 18 +++++++++++++++
 drivers/scsi/scsi_lib.c    | 14 ++++++++++++
 drivers/scsi/scsi_sysfs.c  | 23 +++++++++++++++++++
 drivers/scsi/sd.c          |  2 ++
 fs/buffer.c                | 10 +++++++--
 include/linux/dev_printk.h | 14 ++++++++++++
 include/linux/device.h     |  4 ++++
 include/scsi/scsi_device.h |  3 +++
 12 files changed, 156 insertions(+), 13 deletions(-)


base-commit: 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 01/11] struct device: Add function callback durable_name
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
@ 2020-07-24 17:16 ` Tony Asleson
  2020-07-24 17:16 ` [v4 02/11] create_syslog_header: Add durable name Tony Asleson
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:16 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Function callback and function to be used to write a persistent
durable name to the supplied character buffer.  This will be used to add
structured key-value data to log messages for hardware related errors
which allows end users to correlate message and specific hardware.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/base/core.c    | 24 ++++++++++++++++++++++++
 include/linux/device.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 0cad34f1eede..511b7d2fc916 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2304,6 +2304,30 @@ int dev_set_name(struct device *dev, const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(dev_set_name);
 
+/**
+ * dev_durable_name - Write "DURABLE_NAME"=<durable name> in buffer
+ * @dev: device
+ * @buffer: character buffer to write results
+ * @len: length of buffer
+ * @return: Number of bytes written to buffer
+ */
+int dev_durable_name(const struct device *dev, char *buffer, size_t len)
+{
+	int tmp, dlen;
+
+	if (dev && dev->durable_name) {
+		tmp = snprintf(buffer, len, "DURABLE_NAME=");
+		if (tmp < len) {
+			dlen = dev->durable_name(dev, buffer + tmp,
+							len - tmp);
+			if (dlen > 0 && ((dlen + tmp) < len))
+				return dlen + tmp;
+		}
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dev_durable_name);
+
 /**
  * device_to_dev_kobj - select a /sys/dev/ directory for the device
  * @dev: device
diff --git a/include/linux/device.h b/include/linux/device.h
index ac8e37cd716a..281755404c21 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -613,6 +613,8 @@ struct device {
 	struct iommu_group	*iommu_group;
 	struct dev_iommu	*iommu;
 
+	int (*durable_name)(const struct device *dev, char *buff, size_t len);
+
 	bool			offline_disabled:1;
 	bool			offline:1;
 	bool			of_node_reused:1;
@@ -654,6 +656,8 @@ static inline const char *dev_name(const struct device *dev)
 extern __printf(2, 3)
 int dev_set_name(struct device *dev, const char *name, ...);
 
+int dev_durable_name(const struct device *d, char *buffer, size_t len);
+
 #ifdef CONFIG_NUMA
 static inline int dev_to_node(struct device *dev)
 {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 02/11] create_syslog_header: Add durable name
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
  2020-07-24 17:16 ` [v4 01/11] struct device: Add function callback durable_name Tony Asleson
@ 2020-07-24 17:16 ` Tony Asleson
  2020-07-24 17:16 ` [v4 03/11] dev_vprintk_emit: Increase hdr size Tony Asleson
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:16 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

This gets us a persistent durable name for code that logs messages in the
block layer that have the appropriate callbacks setup for durable name.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/base/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 511b7d2fc916..964690572a89 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3754,6 +3754,7 @@ create_syslog_header(const struct device *dev, char *hdr, size_t hdrlen)
 {
 	const char *subsys;
 	size_t pos = 0;
+	int dlen;
 
 	if (dev->class)
 		subsys = dev->class->name;
@@ -3796,6 +3797,10 @@ create_syslog_header(const struct device *dev, char *hdr, size_t hdrlen)
 				"DEVICE=+%s:%s", subsys, dev_name(dev));
 	}
 
+	dlen = dev_durable_name(dev, hdr + (pos + 1), hdrlen - (pos + 1));
+	if (dlen)
+		pos += dlen + 1;
+
 	if (pos >= hdrlen)
 		goto overflow;
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 03/11] dev_vprintk_emit: Increase hdr size
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
  2020-07-24 17:16 ` [v4 01/11] struct device: Add function callback durable_name Tony Asleson
  2020-07-24 17:16 ` [v4 02/11] create_syslog_header: Add durable name Tony Asleson
@ 2020-07-24 17:16 ` Tony Asleson
  2020-07-25 10:05   ` Andy Shevchenko
  2020-07-24 17:16 ` [v4 04/11] scsi: Add durable_name for dev_printk Tony Asleson
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:16 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

With the addition of the device persistent id we have the possibility
of adding 154 more bytes to the hdr.  Thus if we assume the previous
size of 128 was sufficent we can simply add the 2 amounts and round
up.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/base/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 964690572a89..c2439d12608d 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3814,7 +3814,7 @@ create_syslog_header(const struct device *dev, char *hdr, size_t hdrlen)
 int dev_vprintk_emit(int level, const struct device *dev,
 		     const char *fmt, va_list args)
 {
-	char hdr[128];
+	char hdr[288];
 	size_t hdrlen;
 
 	hdrlen = create_syslog_header(dev, hdr, sizeof(hdr));
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 04/11] scsi: Add durable_name for dev_printk
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (2 preceding siblings ...)
  2020-07-24 17:16 ` [v4 03/11] dev_vprintk_emit: Increase hdr size Tony Asleson
@ 2020-07-24 17:16 ` Tony Asleson
  2020-07-25 10:20   ` Andy Shevchenko
  2020-07-24 17:17 ` [v4 05/11] nvme: Add durable name " Tony Asleson
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:16 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Add the needed functions to fill out the durable_name function
call back for scsi based storage devices.  This allows calls
into dev_printk for scsi devices to have a persistent id
associated with them.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/scsi/scsi_lib.c    | 14 ++++++++++++++
 drivers/scsi/scsi_sysfs.c  | 23 +++++++++++++++++++++++
 drivers/scsi/sd.c          |  2 ++
 include/scsi/scsi_device.h |  3 +++
 4 files changed, 42 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 06c260f6cdae..9f6c41162c55 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3142,3 +3142,17 @@ int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
 	return group_id;
 }
 EXPORT_SYMBOL(scsi_vpd_tpg_id);
+
+int scsi_durable_name(struct scsi_device *sdev, char *buf, size_t len)
+{
+	int vpd_len = 0;
+
+	vpd_len = scsi_vpd_lun_id(sdev, buf, len);
+	if (vpd_len > 0 && vpd_len < len)
+		vpd_len++;
+	else
+		vpd_len = 0;
+
+	return vpd_len;
+}
+EXPORT_SYMBOL(scsi_durable_name);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 163dbcb741c1..f719b63f4b63 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1582,6 +1582,28 @@ static struct device_type scsi_dev_type = {
 	.groups =	scsi_sdev_attr_groups,
 };
 
+
+int dev_to_scsi_durable_name(const struct device *dev, char *buf, size_t len)
+{
+	struct scsi_device *sd_dev = NULL;
+
+	// When we go through dev_printk in the scsi layer, dev is embedded
+	// in a struct scsi_device.  When we go through the block layer,
+	// dev is embedded in struct genhd, thus we need different paths to
+	// retrieve the struct scsi_device to call scsi_durable_name.
+	if (dev->type == &scsi_dev_type) {
+		sd_dev = to_scsi_device(dev);
+	} else if (dev->parent && dev->parent->type == &scsi_dev_type) {
+		sd_dev = to_scsi_device(dev->parent);
+	} else {
+		// We have a pointer to something else, bail
+		return 0;
+	}
+
+	return scsi_durable_name(sd_dev, buf, len);
+}
+EXPORT_SYMBOL(dev_to_scsi_durable_name);
+
 void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 {
 	unsigned long flags;
@@ -1591,6 +1613,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 	device_initialize(&sdev->sdev_gendev);
 	sdev->sdev_gendev.bus = &scsi_bus_type;
 	sdev->sdev_gendev.type = &scsi_dev_type;
+	sdev->sdev_gendev.durable_name = dev_to_scsi_durable_name;
 	dev_set_name(&sdev->sdev_gendev, "%d:%d:%d:%llu",
 		     sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a793cb08d025..f40e4cb4a5f6 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3360,6 +3360,8 @@ static int sd_probe(struct device *dev)
 	gd->private_data = &sdkp->driver;
 	gd->queue = sdkp->device->request_queue;
 
+	disk_to_dev(gd)->durable_name = dev_to_scsi_durable_name;
+
 	/* defaults, until the device tells us otherwise */
 	sdp->sector_size = 512;
 	sdkp->capacity = 0;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index c3cba2aaf934..7be5861565f7 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -461,6 +461,9 @@ extern void sdev_disable_disk_events(struct scsi_device *sdev);
 extern void sdev_enable_disk_events(struct scsi_device *sdev);
 extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
 extern int scsi_vpd_tpg_id(struct scsi_device *, int *);
+extern int dev_to_scsi_durable_name(const struct device *dev, char *buf,
+					size_t len);
+extern int scsi_durable_name(struct scsi_device *sdev, char *buf, size_t len);
 
 #ifdef CONFIG_PM
 extern int scsi_autopm_get_device(struct scsi_device *);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 05/11] nvme: Add durable name for dev_printk
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (3 preceding siblings ...)
  2020-07-24 17:16 ` [v4 04/11] scsi: Add durable_name for dev_printk Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-25  9:05   ` Sergei Shtylyov
  2020-07-25 10:23   ` Andy Shevchenko
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Corrections from Keith Busch review comments.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/nvme/host/core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f3c037f5a9ba..f2e5b91668a1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2667,6 +2667,22 @@ static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
 	return true;
 }
 
+static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
+			char *buf);
+
+static int dev_to_nvme_durable_name(const struct device *dev, char *buf, size_t len)
+{
+	char serial[144];	/* Max 141 for wwid_show */
+	ssize_t serial_len = wwid_show((struct device *)dev, NULL, serial);
+
+	if (serial_len > 0 && serial_len < len) {
+		serial_len -= 1;  /* Remove the '\n' from the string */
+		strncpy(buf, serial, serial_len);
+		return serial_len;
+	}
+	return 0;
+}
+
 static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 {
 	struct nvme_subsystem *subsys, *found;
@@ -3616,6 +3632,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 	disk->queue = ns->queue;
 	disk->flags = flags;
 	memcpy(disk->disk_name, disk_name, DISK_NAME_LEN);
+	disk_to_dev(disk)->durable_name = dev_to_nvme_durable_name;
+
 	ns->disk = disk;
 
 	__nvme_revalidate_disk(disk, id);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 06/11] libata: Add ata_scsi_durable_name
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (4 preceding siblings ...)
  2020-07-24 17:17 ` [v4 05/11] nvme: Add durable name " Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-25  0:48   ` kernel test robot
                     ` (4 more replies)
  2020-07-24 17:17 ` [v4 07/11] Add durable_name_printk Tony Asleson
                   ` (5 subsequent siblings)
  11 siblings, 5 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Function used to create the durable name for ata scsi.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/ata/libata-scsi.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 36e588d88b95..ec1f6e406ceb 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1091,6 +1091,14 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
 	return 0;
 }
 
+int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
+{
+	struct ata_device *ata_dev = container_of(dev, struct ata_device, tdev);
+
+	return scsi_durable_name(ata_dev->sdev, buf, len);
+}
+
+
 /**
  *	ata_scsi_slave_config - Set SCSI device attributes
  *	@sdev: SCSI device to examine
@@ -1111,8 +1119,11 @@ int ata_scsi_slave_config(struct scsi_device *sdev)
 
 	ata_scsi_sdev_config(sdev);
 
-	if (dev)
+	if (dev) {
 		rc = ata_scsi_dev_config(sdev, dev);
+		if (!rc)
+			dev->tdev.durable_name = ata_scsi_durable_name;
+	}
 
 	return rc;
 }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 07/11] Add durable_name_printk
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (5 preceding siblings ...)
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-24 17:17 ` [v4 08/11] libata: use durable_name_printk Tony Asleson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Ideally block related code would standardize on using dev_printk,
but dev_printk does change the user visible messages which is
questionable.  Adding this function which adds the structured
key/value durable name to the log entry.  It has the
same signature as dev_printk.  In the future, code that
is using this could easily transition to dev_printk when that
becomes workable.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/base/core.c        | 15 +++++++++++++++
 include/linux/dev_printk.h |  5 +++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index c2439d12608d..aaf7b8256712 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3865,6 +3865,21 @@ void dev_printk(const char *level, const struct device *dev,
 }
 EXPORT_SYMBOL(dev_printk);
 
+void durable_name_printk(const char *level, const struct device *dev,
+		const char *fmt, ...)
+{
+	size_t dictlen;
+	va_list args;
+	char dict[288];
+
+	dictlen = dev_durable_name(dev, dict, sizeof(dict));
+
+	va_start(args, fmt);
+	vprintk_emit(0, level[1] - '0', dict, dictlen, fmt, args);
+	va_end(args);
+}
+EXPORT_SYMBOL(durable_name_printk);
+
 #define define_dev_printk_level(func, kern_level)		\
 void func(const struct device *dev, const char *fmt, ...)	\
 {								\
diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index 5aad06b4ca7b..502cf9fd7fe7 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -32,6 +32,11 @@ int dev_printk_emit(int level, const struct device *dev, const char *fmt, ...);
 __printf(3, 4) __cold
 void dev_printk(const char *level, const struct device *dev,
 		const char *fmt, ...);
+
+__printf(3, 4) __cold
+void durable_name_printk(const char *level, const struct device *dev,
+			const char *fmt, ...);
+
 __printf(2, 3) __cold
 void _dev_emerg(const struct device *dev, const char *fmt, ...);
 __printf(2, 3) __cold
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 08/11] libata: use durable_name_printk
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (6 preceding siblings ...)
  2020-07-24 17:17 ` [v4 07/11] Add durable_name_printk Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-24 17:17 ` [v4 09/11] Add durable_name_printk_ratelimited Tony Asleson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Utilize durable_name_printk to associate the durable name
with the log message via structured data.  The user visible
portion of the log message is unchanged.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 drivers/ata/libata-core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index beca5f91bb4c..468aa3f7eaad 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6444,7 +6444,8 @@ void ata_port_printk(const struct ata_port *ap, const char *level,
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	printk("%sata%u: %pV", level, ap->print_id, &vaf);
+	durable_name_printk(level, &ap->tdev, "ata%u: %pV",
+		ap->print_id, &vaf);
 
 	va_end(args);
 }
@@ -6462,11 +6463,11 @@ void ata_link_printk(const struct ata_link *link, const char *level,
 	vaf.va = &args;
 
 	if (sata_pmp_attached(link->ap) || link->ap->slave_link)
-		printk("%sata%u.%02u: %pV",
-		       level, link->ap->print_id, link->pmp, &vaf);
+		durable_name_printk(level, &link->tdev, "ata%u.%02u: %pV",
+		       link->ap->print_id, link->pmp, &vaf);
 	else
-		printk("%sata%u: %pV",
-		       level, link->ap->print_id, &vaf);
+		durable_name_printk(level, &link->tdev, "ata%u: %pV",
+		       link->ap->print_id, &vaf);
 
 	va_end(args);
 }
@@ -6483,9 +6484,9 @@ void ata_dev_printk(const struct ata_device *dev, const char *level,
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	printk("%sata%u.%02u: %pV",
-	       level, dev->link->ap->print_id, dev->link->pmp + dev->devno,
-	       &vaf);
+	durable_name_printk(level, &dev->tdev, "ata%u.%02u: %pV",
+		dev->link->ap->print_id, dev->link->pmp + dev->devno,
+		&vaf);
 
 	va_end(args);
 }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 09/11] Add durable_name_printk_ratelimited
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (7 preceding siblings ...)
  2020-07-24 17:17 ` [v4 08/11] libata: use durable_name_printk Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-24 17:17 ` [v4 10/11] print_req_error: Use durable_name_printk_ratelimited Tony Asleson
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Create a rate limited macro for durable_name_printk so that
we can use this for printk_ratelimited usage in the block
layers and add the durable name key/value to the log
message.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 include/linux/dev_printk.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index 502cf9fd7fe7..931977f5c85e 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -37,6 +37,15 @@ __printf(3, 4) __cold
 void durable_name_printk(const char *level, const struct device *dev,
 			const char *fmt, ...);
 
+#define durable_name_printk_ratelimited(level, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+					DEFAULT_RATELIMIT_INTERVAL,	\
+					DEFAULT_RATELIMIT_BURST);	\
+	if (__ratelimit(&_rs))						\
+		durable_name_printk(level, dev, fmt, ##__VA_ARGS__);	\
+} while (0)
+
 __printf(2, 3) __cold
 void _dev_emerg(const struct device *dev, const char *fmt, ...);
 __printf(2, 3) __cold
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 10/11] print_req_error: Use durable_name_printk_ratelimited
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (8 preceding siblings ...)
  2020-07-24 17:17 ` [v4 09/11] Add durable_name_printk_ratelimited Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-25  9:15   ` Sergei Shtylyov
  2020-07-24 17:17 ` [v4 11/11] buffer_io_error: " Tony Asleson
  2020-07-26 15:10 ` [v4 00/11] Add persistent durable identifier to storage log messages Christoph Hellwig
  11 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Replace printk_ratelimited with one that adds the key/value
durable name to log entry.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 block/blk-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 9bfaee050c82..a1f35e3e21d8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -213,12 +213,15 @@ EXPORT_SYMBOL_GPL(blk_status_to_errno);
 static void print_req_error(struct request *req, blk_status_t status,
 		const char *caller)
 {
+	struct device *dev;
 	int idx = (__force int)status;
 
 	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors)))
 		return;
 
-	printk_ratelimited(KERN_ERR
+	dev = (req->rq_disk) ? disk_to_dev(req->rq_disk) : NULL;
+
+	durable_name_printk_ratelimited(KERN_ERR, dev,
 		"%s: %s error, dev %s, sector %llu op 0x%x:(%s) flags 0x%x "
 		"phys_seg %u prio class %u\n",
 		caller, blk_errors[idx].name,
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [v4 11/11] buffer_io_error: Use durable_name_printk_ratelimited
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (9 preceding siblings ...)
  2020-07-24 17:17 ` [v4 10/11] print_req_error: Use durable_name_printk_ratelimited Tony Asleson
@ 2020-07-24 17:17 ` Tony Asleson
  2020-07-25 10:29   ` Andy Shevchenko
  2020-07-26 15:10 ` [v4 00/11] Add persistent durable identifier to storage log messages Christoph Hellwig
  11 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-24 17:17 UTC (permalink / raw)
  To: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Replace printk_ratelimited with one that adds the key/value
durable name to log entry.

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 fs/buffer.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index a60f60396cfa..f35eaaafce0e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -133,10 +133,16 @@ __clear_page_buffers(struct page *page)
 
 static void buffer_io_error(struct buffer_head *bh, char *msg)
 {
-	if (!test_bit(BH_Quiet, &bh->b_state))
-		printk_ratelimited(KERN_ERR
+	if (!test_bit(BH_Quiet, &bh->b_state)) {
+		struct device *gendev;
+
+		gendev = (bh->b_bdev->bd_disk) ?
+			disk_to_dev(bh->b_bdev->bd_disk) : NULL;
+
+		durable_name_printk_ratelimited(KERN_ERR, gendev,
 			"Buffer I/O error on dev %pg, logical block %llu%s\n",
 			bh->b_bdev, (unsigned long long)bh->b_blocknr, msg);
+	}
 }
 
 /*
-- 
2.26.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 06/11] libata: Add ata_scsi_durable_name
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
@ 2020-07-25  0:48   ` kernel test robot
  2020-07-25  1:07   ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-07-25  0:48 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe
  Cc: kbuild-all


[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

Hi Tony,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162]

url:    https://github.com/0day-ci/linux/commits/Tony-Asleson/Add-persistent-durable-identifier-to-storage-log-messages/20200725-011936
base:    3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/ata/libata-scsi.c:1094:5: warning: no previous prototype for 'ata_scsi_durable_name' [-Wmissing-prototypes]
    1094 | int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
         |     ^~~~~~~~~~~~~~~~~~~~~

vim +/ata_scsi_durable_name +1094 drivers/ata/libata-scsi.c

  1093	
> 1094	int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
  1095	{
  1096		struct ata_device *ata_dev = container_of(dev, struct ata_device, tdev);
  1097	
  1098		return scsi_durable_name(ata_dev->sdev, buf, len);
  1099	}
  1100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72863 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 06/11] libata: Add ata_scsi_durable_name
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
  2020-07-25  0:48   ` kernel test robot
@ 2020-07-25  1:07   ` kernel test robot
  2020-07-25  1:07   ` [RFC PATCH] libata: ata_scsi_durable_name() can be static kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-07-25  1:07 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe
  Cc: kbuild-all


[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

Hi Tony,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162]

url:    https://github.com/0day-ci/linux/commits/Tony-Asleson/Add-persistent-durable-identifier-to-storage-log-messages/20200725-011936
base:    3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
config: x86_64-randconfig-s022-20200724 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-93-g4c6cbe55-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/ata/libata-scsi.c:1094:5: sparse: sparse: symbol 'ata_scsi_durable_name' was not declared. Should it be static?
   drivers/ata/libata-scsi.c:1803:13: sparse: sparse: context imbalance in 'ata_scsi_rbuf_get' - wrong count at exit
   drivers/ata/libata-scsi.c:1833:31: sparse: sparse: context imbalance in 'ata_scsi_rbuf_fill' - unexpected unlock

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32757 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [RFC PATCH] libata: ata_scsi_durable_name() can be static
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
  2020-07-25  0:48   ` kernel test robot
  2020-07-25  1:07   ` kernel test robot
@ 2020-07-25  1:07   ` kernel test robot
  2020-07-25  1:18   ` [v4 06/11] libata: Add ata_scsi_durable_name kernel test robot
  2020-07-25 10:26   ` Andy Shevchenko
  4 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-07-25  1:07 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe
  Cc: kbuild-all


Signed-off-by: kernel test robot <lkp@intel.com>
---
 libata-scsi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index ec1f6e406ceb2..32bd2426ca4f5 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1091,7 +1091,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
 	return 0;
 }
 
-int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
+static int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
 {
 	struct ata_device *ata_dev = container_of(dev, struct ata_device, tdev);
 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 06/11] libata: Add ata_scsi_durable_name
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
                     ` (2 preceding siblings ...)
  2020-07-25  1:07   ` [RFC PATCH] libata: ata_scsi_durable_name() can be static kernel test robot
@ 2020-07-25  1:18   ` kernel test robot
  2020-07-25 10:26   ` Andy Shevchenko
  4 siblings, 0 replies; 30+ messages in thread
From: kernel test robot @ 2020-07-25  1:18 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe
  Cc: kbuild-all, clang-built-linux


[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]

Hi Tony,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162]

url:    https://github.com/0day-ci/linux/commits/Tony-Asleson/Add-persistent-durable-identifier-to-storage-log-messages/20200725-011936
base:    3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
config: x86_64-randconfig-a005-20200724 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1d09ecf36175f7910ffedd6d497c07b5c74c22fb)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/ata/libata-scsi.c:1094:5: warning: no previous prototype for function 'ata_scsi_durable_name' [-Wmissing-prototypes]
   int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
       ^
   drivers/ata/libata-scsi.c:1094:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
   ^
   static 
   1 warning generated.

vim +/ata_scsi_durable_name +1094 drivers/ata/libata-scsi.c

  1093	
> 1094	int ata_scsi_durable_name(const struct device *dev, char *buf, size_t len)
  1095	{
  1096		struct ata_device *ata_dev = container_of(dev, struct ata_device, tdev);
  1097	
  1098		return scsi_durable_name(ata_dev->sdev, buf, len);
  1099	}
  1100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26996 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 05/11] nvme: Add durable name for dev_printk
  2020-07-24 17:17 ` [v4 05/11] nvme: Add durable name " Tony Asleson
@ 2020-07-25  9:05   ` Sergei Shtylyov
  2020-07-25 10:23   ` Andy Shevchenko
  1 sibling, 0 replies; 30+ messages in thread
From: Sergei Shtylyov @ 2020-07-25  9:05 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

Hello!

On 24.07.2020 20:17, Tony Asleson wrote:

> Corrections from Keith Busch review comments.
> 
> Signed-off-by: Tony Asleson <tasleson@redhat.com>
> ---
>   drivers/nvme/host/core.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f3c037f5a9ba..f2e5b91668a1 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2667,6 +2667,22 @@ static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
>   	return true;
>   }
>   
> +static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
> +			char *buf);
> +
> +static int dev_to_nvme_durable_name(const struct device *dev, char *buf, size_t len)
> +{
> +	char serial[144];	/* Max 141 for wwid_show */
> +	ssize_t serial_len = wwid_show((struct device *)dev, NULL, serial);
> +
> +	if (serial_len > 0 && serial_len < len) {
> +		serial_len -= 1;  /* Remove the '\n' from the string */

    serial_len-- instead?

> +		strncpy(buf, serial, serial_len);
> +		return serial_len;
> +	}
> +	return 0;
> +}
> +
>   static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   {
>   	struct nvme_sub
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 10/11] print_req_error: Use durable_name_printk_ratelimited
  2020-07-24 17:17 ` [v4 10/11] print_req_error: Use durable_name_printk_ratelimited Tony Asleson
@ 2020-07-25  9:15   ` Sergei Shtylyov
  0 siblings, 0 replies; 30+ messages in thread
From: Sergei Shtylyov @ 2020-07-25  9:15 UTC (permalink / raw)
  To: Tony Asleson, linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 24.07.2020 20:17, Tony Asleson wrote:

> Replace printk_ratelimited with one that adds the key/value
> durable name to log entry.
> 
> Signed-off-by: Tony Asleson <tasleson@redhat.com>
> ---
>   block/blk-core.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 9bfaee050c82..a1f35e3e21d8 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -213,12 +213,15 @@ EXPORT_SYMBOL_GPL(blk_status_to_errno);
>   static void print_req_error(struct request *req, blk_status_t status,
>   		const char *caller)
>   {
> +	struct device *dev;
>   	int idx = (__force int)status;
>   
>   	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors)))
>   		return;
>   
> -	printk_ratelimited(KERN_ERR
> +	dev = (req->rq_disk) ? disk_to_dev(req->rq_disk) : NULL;

    The 1st () not necessary, here iand in the next patch...

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 03/11] dev_vprintk_emit: Increase hdr size
  2020-07-24 17:16 ` [v4 03/11] dev_vprintk_emit: Increase hdr size Tony Asleson
@ 2020-07-25 10:05   ` Andy Shevchenko
  2020-08-26 18:26     ` Tony Asleson
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2020-07-25 10:05 UTC (permalink / raw)
  To: Tony Asleson
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
>
> With the addition of the device persistent id we have the possibility
> of adding 154 more bytes to the hdr.  Thus if we assume the previous
> size of 128 was sufficent we can simply add the 2 amounts and round

sufficient

> up.

...

> -       char hdr[128];
> +       char hdr[288];

This is quite a drastic change for the stack.
Can you refactor to avoid this?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 04/11] scsi: Add durable_name for dev_printk
  2020-07-24 17:16 ` [v4 04/11] scsi: Add durable_name for dev_printk Tony Asleson
@ 2020-07-25 10:20   ` Andy Shevchenko
  0 siblings, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-07-25 10:20 UTC (permalink / raw)
  To: Tony Asleson
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
>
> Add the needed functions to fill out the durable_name function
> call back for scsi based storage devices.  This allows calls
> into dev_printk for scsi devices to have a persistent id
> associated with them.

...

> +int scsi_durable_name(struct scsi_device *sdev, char *buf, size_t len)
> +{
> +       int vpd_len = 0;

What is the purpose of the assignment?

> +       vpd_len = scsi_vpd_lun_id(sdev, buf, len);
> +       if (vpd_len > 0 && vpd_len < len)
> +               vpd_len++;
> +       else
> +               vpd_len = 0;
> +
> +       return vpd_len;

int vpd_len;

vpd_len = ...(...);
if (vpd_len ...)
  return vpd_len + 1;
return 0;

> +}

...

>  };
>
> +

One blank line is enough.

> +int dev_to_scsi_durable_name(const struct device *dev, char *buf, size_t len)
> +{
> +       struct scsi_device *sd_dev = NULL;

Redundant assignment.

> +       // When we go through dev_printk in the scsi layer, dev is embedded
> +       // in a struct scsi_device.  When we go through the block layer,
> +       // dev is embedded in struct genhd, thus we need different paths to
> +       // retrieve the struct scsi_device to call scsi_durable_name.
> +       if (dev->type == &scsi_dev_type) {
> +               sd_dev = to_scsi_device(dev);

This...

> +       } else if (dev->parent && dev->parent->type == &scsi_dev_type) {
> +               sd_dev = to_scsi_device(dev->parent);

...and this along with pieces in scsi_bus_uevent(), scsi_bus_match()
are candidates for a good helper (perhaps with utilization of
scsi_is_sdev_device(), but maybe not necessary).

static struct scsi_device *dev_to_scsi_device(struct device *dev)
{
  return dev->type == &scsi_dev_type ? to_scsi_device(dev) : NULL;
}

sdev = dev_to_scsi_device(dev);
if (!sdev)
  sdev = dev_to_scsi_device(dev->parent);
if (!sdev)
  return 0;

And when introducing a helper change users at the same time.

> +       } else {
> +               // We have a pointer to something else, bail
> +               return 0;
> +       }
> +
> +       return scsi_durable_name(sd_dev, buf, len);
> +}

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 05/11] nvme: Add durable name for dev_printk
  2020-07-24 17:17 ` [v4 05/11] nvme: Add durable name " Tony Asleson
  2020-07-25  9:05   ` Sergei Shtylyov
@ 2020-07-25 10:23   ` Andy Shevchenko
  1 sibling, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-07-25 10:23 UTC (permalink / raw)
  To: Tony Asleson
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
>
> Corrections from Keith Busch review comments.

Good! And where is the commit message?



> +static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
> +                       char *buf);
> +
> +static int dev_to_nvme_durable_name(const struct device *dev, char *buf, size_t len)
> +{
> +       char serial[144];       /* Max 141 for wwid_show */
> +       ssize_t serial_len = wwid_show((struct device *)dev, NULL, serial);
> +
> +       if (serial_len > 0 && serial_len < len) {
> +               serial_len -= 1;  /* Remove the '\n' from the string */
> +               strncpy(buf, serial, serial_len);
> +               return serial_len;
> +       }
> +       return 0;
> +}

Sorry, but this is ugly. Can we rather get some common code from
wwid_show() and reuse it there and here w/o above dances with \n?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 06/11] libata: Add ata_scsi_durable_name
  2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
                     ` (3 preceding siblings ...)
  2020-07-25  1:18   ` [v4 06/11] libata: Add ata_scsi_durable_name kernel test robot
@ 2020-07-25 10:26   ` Andy Shevchenko
  4 siblings, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-07-25 10:26 UTC (permalink / raw)
  To: Tony Asleson
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
>
> Function used to create the durable name for ata scsi.

https://chris.beams.io/posts/git-commit/

> Signed-off-by: Tony Asleson <tasleson@redhat.com>

...

> +}
> +
> +

One is enough, really!

>  /**

...

> -       if (dev)
> +       if (dev) {
>                 rc = ata_scsi_dev_config(sdev, dev);
> +               if (!rc)
> +                       dev->tdev.durable_name = ata_scsi_durable_name;
> +       }

Can we stick to our usual pattern?

rc = ...
if (rc)
  return rc;
...

>         return rc;
>  }


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 11/11] buffer_io_error: Use durable_name_printk_ratelimited
  2020-07-24 17:17 ` [v4 11/11] buffer_io_error: " Tony Asleson
@ 2020-07-25 10:29   ` Andy Shevchenko
  0 siblings, 0 replies; 30+ messages in thread
From: Andy Shevchenko @ 2020-07-25 10:29 UTC (permalink / raw)
  To: Tony Asleson
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
>
> Replace printk_ratelimited with one that adds the key/value
> durable name to log entry.

>  static void buffer_io_error(struct buffer_head *bh, char *msg)
>  {
> -       if (!test_bit(BH_Quiet, &bh->b_state))
> -               printk_ratelimited(KERN_ERR
> +       if (!test_bit(BH_Quiet, &bh->b_state)) {
> +               struct device *gendev;
> +
> +               gendev = (bh->b_bdev->bd_disk) ?
> +                       disk_to_dev(bh->b_bdev->bd_disk) : NULL;

Besides unneeded parentheses as Sergey noticed...

> +
> +               durable_name_printk_ratelimited(KERN_ERR, gendev,
>                         "Buffer I/O error on dev %pg, logical block %llu%s\n",
>                         bh->b_bdev, (unsigned long long)bh->b_blocknr, msg);
> +       }

...can we drop indentation level?

  if (test_bit(...))
   return;
  ...

>  }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
                   ` (10 preceding siblings ...)
  2020-07-24 17:17 ` [v4 11/11] buffer_io_error: " Tony Asleson
@ 2020-07-26 15:10 ` Christoph Hellwig
  2020-07-27 15:45   ` Tony Asleson
  11 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2020-07-26 15:10 UTC (permalink / raw)
  To: Tony Asleson; +Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

FYI, I think these identifiers are absolutely horrible and have no
business in dmesg:

Nacked-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-26 15:10 ` [v4 00/11] Add persistent durable identifier to storage log messages Christoph Hellwig
@ 2020-07-27 15:45   ` Tony Asleson
  2020-07-27 16:46     ` Hannes Reinecke
  0 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-27 15:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 7/26/20 10:10 AM, Christoph Hellwig wrote:
> FYI, I think these identifiers are absolutely horrible and have no
> business in dmesg:

The identifiers are structured data, they're not visible unless you go
looking for them.

I'm open to other suggestions on how we can positively identify storage
devices over time, across reboots, replacement, and dynamic reconfiguration.

My home system has 4 disks, 2 are identical except for serial number.
Even with this simple configuration, it's not trivial to identify which
message goes with which disk across reboots.

-Tony


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-27 15:45   ` Tony Asleson
@ 2020-07-27 16:46     ` Hannes Reinecke
  2020-07-27 17:42       ` Tony Asleson
  0 siblings, 1 reply; 30+ messages in thread
From: Hannes Reinecke @ 2020-07-27 16:46 UTC (permalink / raw)
  To: tasleson, Christoph Hellwig
  Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 7/27/20 5:45 PM, Tony Asleson wrote:
> On 7/26/20 10:10 AM, Christoph Hellwig wrote:
>> FYI, I think these identifiers are absolutely horrible and have no
>> business in dmesg:
> 
> The identifiers are structured data, they're not visible unless you go
> looking for them.
> 
> I'm open to other suggestions on how we can positively identify storage
> devices over time, across reboots, replacement, and dynamic reconfiguration.
> 
> My home system has 4 disks, 2 are identical except for serial number.
> Even with this simple configuration, it's not trivial to identify which
> message goes with which disk across reboots.
> 
Well; the more important bits would be to identify the physical location 
where these disks reside.
If one goes bad it doesn't really help you if have a persistent 
identification in the OS; what you really need is a physical indicator 
or a physical location allowing you to identify which disk to pull.

Which isn't addressed at all with this patchset (nor should it; the 
physical location it typically found via other means).

And for the other use-cases: We do have persistent device links, do we 
not? They provide even more information that you'd extract with this 
this patchset, and don't require any kernel changes whatsoever.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-27 16:46     ` Hannes Reinecke
@ 2020-07-27 17:42       ` Tony Asleson
  2020-07-27 19:17         ` Douglas Gilbert
  0 siblings, 1 reply; 30+ messages in thread
From: Tony Asleson @ 2020-07-27 17:42 UTC (permalink / raw)
  To: Hannes Reinecke, Christoph Hellwig
  Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 7/27/20 11:46 AM, Hannes Reinecke wrote:
> On 7/27/20 5:45 PM, Tony Asleson wrote:
>> On 7/26/20 10:10 AM, Christoph Hellwig wrote:
>>> FYI, I think these identifiers are absolutely horrible and have no
>>> business in dmesg:
>>
>> The identifiers are structured data, they're not visible unless you go
>> looking for them.
>>
>> I'm open to other suggestions on how we can positively identify storage
>> devices over time, across reboots, replacement, and dynamic
>> reconfiguration.
>>
>> My home system has 4 disks, 2 are identical except for serial number.
>> Even with this simple configuration, it's not trivial to identify which
>> message goes with which disk across reboots.
>>
> Well; the more important bits would be to identify the physical location
> where these disks reside.
> If one goes bad it doesn't really help you if have a persistent
> identification in the OS; what you really need is a physical indicator
> or a physical location allowing you to identify which disk to pull.

In my use case I have no slot information.  I have no SCSI enclosure
services to toggle identification LEDs or fault LEDs for the drive sled.
 For some users the device might be a virtual one in a storage server,
vpd helps.

In my case the in kernel vpd (WWN) data can be used to correlate with
the sticker on the disk as the disks have the WWN printed on them.  I
would think this is true for most disks/storage devices, but obviously I
can't make that statement with 100% certainty as I have a small sample size.

> Which isn't addressed at all with this patchset (nor should it; the
> physical location it typically found via other means).
> 
> And for the other use-cases: We do have persistent device links, do we
> not?

How does /dev/disk/by-* help when you are looking at the journal from 1
or more reboots ago and the only thing you have in your journal is
something like:

blk_update_request: critical medium error, dev sde, sector 43578 op
0x0:(READ) flags 0x0 phys_seg 1 prio class 0

The links are only valid for right now.

-Tony


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-27 17:42       ` Tony Asleson
@ 2020-07-27 19:17         ` Douglas Gilbert
  2020-07-27 20:27           ` Tony Asleson
  0 siblings, 1 reply; 30+ messages in thread
From: Douglas Gilbert @ 2020-07-27 19:17 UTC (permalink / raw)
  To: tasleson, Hannes Reinecke, Christoph Hellwig
  Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 2020-07-27 1:42 p.m., Tony Asleson wrote:
> On 7/27/20 11:46 AM, Hannes Reinecke wrote:
>> On 7/27/20 5:45 PM, Tony Asleson wrote:
>>> On 7/26/20 10:10 AM, Christoph Hellwig wrote:
>>>> FYI, I think these identifiers are absolutely horrible and have no
>>>> business in dmesg:
>>>
>>> The identifiers are structured data, they're not visible unless you go
>>> looking for them.
>>>
>>> I'm open to other suggestions on how we can positively identify storage
>>> devices over time, across reboots, replacement, and dynamic
>>> reconfiguration.
>>>
>>> My home system has 4 disks, 2 are identical except for serial number.
>>> Even with this simple configuration, it's not trivial to identify which
>>> message goes with which disk across reboots.
>>>
>> Well; the more important bits would be to identify the physical location
>> where these disks reside.
>> If one goes bad it doesn't really help you if have a persistent
>> identification in the OS; what you really need is a physical indicator
>> or a physical location allowing you to identify which disk to pull.
> 
> In my use case I have no slot information.  I have no SCSI enclosure
> services to toggle identification LEDs or fault LEDs for the drive sled.
>   For some users the device might be a virtual one in a storage server,
> vpd helps.
> 
> In my case the in kernel vpd (WWN) data can be used to correlate with
> the sticker on the disk as the disks have the WWN printed on them.  I
> would think this is true for most disks/storage devices, but obviously I
> can't make that statement with 100% certainty as I have a small sample size.
> 
>> Which isn't addressed at all with this patchset (nor should it; the
>> physical location it typically found via other means).
>>
>> And for the other use-cases: We do have persistent device links, do we
>> not?
> 
> How does /dev/disk/by-* help when you are looking at the journal from 1
> or more reboots ago and the only thing you have in your journal is
> something like:
> 
> blk_update_request: critical medium error, dev sde, sector 43578 op
> 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
> 
> The links are only valid for right now.

Does:
    lsscsi -U
or
    lsscsi -UU

solve your problem, or come close?

Example:
# lsscsi -UU
[1:0:0:0]    disk    naa.5000cca02b38d0b8  /dev/sda
[1:0:1:0]    disk    naa.5000c5003011cb2b  /dev/sdb
[1:0:2:0]    enclosu naa.5001b4d516ecc03f  -
[N:0:1:1]    disk    eui.e8238fa6bf530001001b448b46bd5525    /dev/nvme0n1

The first two (SAS SSDs) NAAs are printed on the disk labels. I don't
think either that enclosure or the M2 NVMe SSD have their numbers
visible (i.e. the last two lines of output).

If it is what you want, then perhaps you could arrange for its output
to be sent to the log when the system has stabilized after a reboot. That
would only leave disk hotplug events exposed.

Faced with the above medium error I would try:
    dd if=<all_possibles> bs=512 skip=43578 iflag=direct of=/dev/null count=1
and look for noise in the logs. Change 'bs=512' up to 4096 if that is
the logical block size. For <all_possibles> use /dev/sde (and /dev/sdf and
/dev/dev/sdg or whatever) IOWs the _whole_ disk device name.

Doug Gilbert

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 00/11] Add persistent durable identifier to storage log messages
  2020-07-27 19:17         ` Douglas Gilbert
@ 2020-07-27 20:27           ` Tony Asleson
  0 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-07-27 20:27 UTC (permalink / raw)
  To: dgilbert, Hannes Reinecke, Christoph Hellwig
  Cc: linux-block, linux-ide, linux-scsi, b.zolnierkie, axboe

On 7/27/20 2:17 PM, Douglas Gilbert wrote:
> On 2020-07-27 1:42 p.m., Tony Asleson wrote:
>> On 7/27/20 11:46 AM, Hannes Reinecke wrote:
>>> On 7/27/20 5:45 PM, Tony Asleson wrote:
>>>> On 7/26/20 10:10 AM, Christoph Hellwig wrote:
>>>>> FYI, I think these identifiers are absolutely horrible and have no
>>>>> business in dmesg:
>>>>
>>>> The identifiers are structured data, they're not visible unless you go
>>>> looking for them.
>>>>
>>>> I'm open to other suggestions on how we can positively identify storage
>>>> devices over time, across reboots, replacement, and dynamic
>>>> reconfiguration.
>>>>
>>>> My home system has 4 disks, 2 are identical except for serial number.
>>>> Even with this simple configuration, it's not trivial to identify which
>>>> message goes with which disk across reboots.
>>>>
>>> Well; the more important bits would be to identify the physical location
>>> where these disks reside.
>>> If one goes bad it doesn't really help you if have a persistent
>>> identification in the OS; what you really need is a physical indicator
>>> or a physical location allowing you to identify which disk to pull.
>>
>> In my use case I have no slot information.  I have no SCSI enclosure
>> services to toggle identification LEDs or fault LEDs for the drive sled.
>>   For some users the device might be a virtual one in a storage server,
>> vpd helps.
>>
>> In my case the in kernel vpd (WWN) data can be used to correlate with
>> the sticker on the disk as the disks have the WWN printed on them.  I
>> would think this is true for most disks/storage devices, but obviously I
>> can't make that statement with 100% certainty as I have a small sample
>> size.
>>
>>> Which isn't addressed at all with this patchset (nor should it; the
>>> physical location it typically found via other means).
>>>
>>> And for the other use-cases: We do have persistent device links, do we
>>> not?
>>
>> How does /dev/disk/by-* help when you are looking at the journal from 1
>> or more reboots ago and the only thing you have in your journal is
>> something like:
>>
>> blk_update_request: critical medium error, dev sde, sector 43578 op
>> 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
>>
>> The links are only valid for right now.
> 
> Does:
>    lsscsi -U
> or
>    lsscsi -UU
> 
> solve your problem, or come close?
> 
> Example:
> # lsscsi -UU
> [1:0:0:0]    disk    naa.5000cca02b38d0b8  /dev/sda
> [1:0:1:0]    disk    naa.5000c5003011cb2b  /dev/sdb
> [1:0:2:0]    enclosu naa.5001b4d516ecc03f  -
> [N:0:1:1]    disk    eui.e8238fa6bf530001001b448b46bd5525    /dev/nvme0n1
> 
> The first two (SAS SSDs) NAAs are printed on the disk labels. I don't
> think either that enclosure or the M2 NVMe SSD have their numbers
> visible (i.e. the last two lines of output).
> 
> If it is what you want, then perhaps you could arrange for its output
> to be sent to the log when the system has stabilized after a reboot. That
> would only leave disk hotplug events exposed.

Yes, if we write a new udev rule or script we could place bread crumbs
in the journal so we can correlate
sda == naa.5000cca02b38d0b8 at the time of the error.  However, none of
the existing tooling will allow you to see all the log messages that
pertain to the device easily.  The user is still left with a bunch of
log messages that have different ways to refer to the same device
attachment eg. sda, ata1.00, sd 0:0:0:0.  For them to understand which
messages go with which device is not trivial.  Even if someone writes a
tool to parse the messages, looking for the string that contains the ID
and has the needed decoder information to associate it with the correct
piece of hardware, it's only good until the message changes in the kernel.

If we stuff the defacto ID into the message itself when it occurs, the
ambiguity of what device is associated with a message is removed.

I would like to know, why this is so horrible?  Is it processing time in
an error path?  Stack usage holding the data in flight, wasted disk
space on disk?  Unique identifiers are just too long and terse?

The only valid reason I can think of is someone working with very
sensitive data and not wanting the unique ID of a removable or network
storage device to be in their logs.  Of course we could add a disable
boot time option for that or make the default off for those that don't
want/care.

> Faced with the above medium error I would try:
>    dd if=<all_possibles> bs=512 skip=43578 iflag=direct of=/dev/null
> count=1
> and look for noise in the logs. Change 'bs=512' up to 4096 if that is
> the logical block size. For <all_possibles> use /dev/sde (and /dev/sdf and
> /dev/dev/sdg or whatever) IOWs the _whole_ disk device name.

Assuming the error reproduces, this would work.  However, I think this
solution speaks volumes for how difficult it is to simply identify what
device an error originated from.

-Tony


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [v4 03/11] dev_vprintk_emit: Increase hdr size
  2020-07-25 10:05   ` Andy Shevchenko
@ 2020-08-26 18:26     ` Tony Asleson
  0 siblings, 0 replies; 30+ messages in thread
From: Tony Asleson @ 2020-08-26 18:26 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-block, linux-ide, linux-scsi, Bartlomiej Zolnierkiewicz,
	Jens Axboe

On 7/25/20 5:05 AM, Andy Shevchenko wrote:
> On Fri, Jul 24, 2020 at 8:19 PM Tony Asleson <tasleson@redhat.com> wrote:
> 
>> -       char hdr[128];
>> +       char hdr[288];
> 
> This is quite a drastic change for the stack.
> Can you refactor to avoid this?

The only thing I can think of is using a hash of the identifier instead
of the value itself.  This could drastically reduce the stack usage and
data stored in journal, but it makes it kind of clumsy for using.

Would placing this on the heap be acceptable?


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, back to index

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-24 17:16 [v4 00/11] Add persistent durable identifier to storage log messages Tony Asleson
2020-07-24 17:16 ` [v4 01/11] struct device: Add function callback durable_name Tony Asleson
2020-07-24 17:16 ` [v4 02/11] create_syslog_header: Add durable name Tony Asleson
2020-07-24 17:16 ` [v4 03/11] dev_vprintk_emit: Increase hdr size Tony Asleson
2020-07-25 10:05   ` Andy Shevchenko
2020-08-26 18:26     ` Tony Asleson
2020-07-24 17:16 ` [v4 04/11] scsi: Add durable_name for dev_printk Tony Asleson
2020-07-25 10:20   ` Andy Shevchenko
2020-07-24 17:17 ` [v4 05/11] nvme: Add durable name " Tony Asleson
2020-07-25  9:05   ` Sergei Shtylyov
2020-07-25 10:23   ` Andy Shevchenko
2020-07-24 17:17 ` [v4 06/11] libata: Add ata_scsi_durable_name Tony Asleson
2020-07-25  0:48   ` kernel test robot
2020-07-25  1:07   ` kernel test robot
2020-07-25  1:07   ` [RFC PATCH] libata: ata_scsi_durable_name() can be static kernel test robot
2020-07-25  1:18   ` [v4 06/11] libata: Add ata_scsi_durable_name kernel test robot
2020-07-25 10:26   ` Andy Shevchenko
2020-07-24 17:17 ` [v4 07/11] Add durable_name_printk Tony Asleson
2020-07-24 17:17 ` [v4 08/11] libata: use durable_name_printk Tony Asleson
2020-07-24 17:17 ` [v4 09/11] Add durable_name_printk_ratelimited Tony Asleson
2020-07-24 17:17 ` [v4 10/11] print_req_error: Use durable_name_printk_ratelimited Tony Asleson
2020-07-25  9:15   ` Sergei Shtylyov
2020-07-24 17:17 ` [v4 11/11] buffer_io_error: " Tony Asleson
2020-07-25 10:29   ` Andy Shevchenko
2020-07-26 15:10 ` [v4 00/11] Add persistent durable identifier to storage log messages Christoph Hellwig
2020-07-27 15:45   ` Tony Asleson
2020-07-27 16:46     ` Hannes Reinecke
2020-07-27 17:42       ` Tony Asleson
2020-07-27 19:17         ` Douglas Gilbert
2020-07-27 20:27           ` Tony Asleson

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git