staging: unisys: visorbus: convert client_bus_info sysfs to debugfs

Previously, the sysfs entry (assuming traditional sysfs mountpoint):

    /sys/bus/visorbus/devices/visorbus<n>/client_bus_info

violated kernel conventions by printing more than one item.  This along
with the fact that the data emitted was diagnostic data (intended to
shadow the client driver info provided via s-Par livedumps) made it a
logical candidate for debugfs.  So this patch moves this sysfs entry to
debugfs as (assuming traditional debugfs mountpoint):

    /sys/kernel/debug/visorbus/visorbus<n>/client_bus_info

Data for this debugfs is emitted using the preferred seq_file interface,
which allowed a vastly-simplified version of vbuschannel_print_devinfo()
to format the individual output components.

Functionality was verified as follows:

  [root@sparguest visorbus]# mount | grep debug
  debugfs on /sys/kernel/debug type debugfs (rw)
  [root@sparguest visorbus]# pwd
  /sys/kernel/debug/visorbus
  [root@sparguest visorbus]# l visorbus1/
  total 0
  drwxr-xr-x 2 root root 0 Sep 28 16:36 .
  drwxr-xr-x 4 root root 0 Sep 28 16:36 ..
  -r--r----- 1 root root 0 Sep 28 16:36 client_bus_info
  [root@sparguest visorbus]# l visorbus2
  total 0
  drwxr-xr-x 2 root root 0 Sep 28 16:36 .
  drwxr-xr-x 4 root root 0 Sep 28 16:36 ..
  -r--r----- 1 root root 0 Sep 28 16:36 client_bus_info
  [root@sparguest visorbus]# cat visorbus1/client_bus_info
  Client device / client driver info for s-Par Console partition (vbus #1):
     chipset          visorchipset     kernel ver. 4.8.0-rc6-ARCH+
     clientbus        visorbus         kernel ver. 4.8.0-rc6-ARCH+
  [2]keyboard         visorinput       kernel ver. 4.8.0-rc6-ARCH+
  [3]mouse            visorinput       kernel ver. 4.8.0-rc6-ARCH+
  [root@sparguest visorbus]# cat visorbus2/client_bus_info
  Client device / client driver info for s-Par IOVM partition (vbus #2):
     chipset          visorchipset     kernel ver. 4.8.0-rc6-ARCH+
     clientbus        visorbus         kernel ver. 4.8.0-rc6-ARCH+
  [0]ultravnic        visornic         kernel ver. 4.8.0-rc6-ARCH+
  [1]ultravnic        visornic         kernel ver. 4.8.0-rc6-ARCH+
  [2]sparvhba         visorhba         kernel ver. 4.8.0-rc6-ARCH+

Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/staging/unisys/include/visorbus.h

@@ -166,6 +166,8 @@ struct visor_device {
 	struct controlvm_message_header *pending_msg_hdr;
 	void *vbus_hdr_info;
 	uuid_le partition_uuid;
+	struct dentry *debugfs_dir;
+	struct dentry *debugfs_client_bus_info;
 };
 
 #define to_visor_device(x) container_of(x, struct visor_device, device)

drivers/staging/unisys/visorbus/vbuschannel.h

@@ -23,6 +23,7 @@
  *  the client devices and client drivers for the server end to see.
  */
 #include <linux/uuid.h>
+#include <linux/ctype.h>
 #include "channel.h"
 
 /* {193b331b-c58f-11da-95a9-00e08161165f} */
@@ -72,199 +73,38 @@ struct ultra_vbus_deviceinfo {
 };
 
 /**
- * vbuschannel_sanitize_buffer() - remove non-printable chars from buffer
+ * vbuschannel_print_devinfo() - format a struct ultra_vbus_deviceinfo
- * @p: destination buffer where chars are written to
+ *                               and write it to a seq_file
- * @remain: number of bytes that can be written starting at #p
- * @src: pointer to source buffer
- * @srcmax: number of valid characters at #src
- *
- * Reads chars from the buffer at @src for @srcmax bytes, and writes to
- * the buffer at @p, which is @remain bytes long, ensuring never to
- * overflow the buffer at @p, using the following rules:
- * - printable characters are simply copied from the buffer at @src to the
- *   buffer at @p
- * - intervening streaks of non-printable characters in the buffer at @src
- *   are replaced with a single space in the buffer at @p
- * Note that we pay no attention to '\0'-termination.
- *
- * Pass @p == NULL and @remain == 0 for this special behavior -- In this
- * case, we simply return the number of bytes that WOULD HAVE been written
- * to a buffer at @p, had it been infinitely big.
- *
- * Return: the number of bytes written to @p (or WOULD HAVE been written to
- *         @p, as described in the previous paragraph)
- */
-static inline int
-vbuschannel_sanitize_buffer(char *p, int remain, char *src, int srcmax)
-{
-	int chars = 0;
-	int nonprintable_streak = 0;
-
-	while (srcmax > 0) {
-		if ((*src >= ' ') && (*src < 0x7f)) {
-			if (nonprintable_streak) {
-				if (remain > 0) {
-					*p = ' ';
-					p++;
-					remain--;
-					chars++;
-				} else if (!p) {
-					chars++;
-				}
-				nonprintable_streak = 0;
-			}
-			if (remain > 0) {
-				*p = *src;
-				p++;
-				remain--;
-				chars++;
-			} else if (!p) {
-				chars++;
-			}
-		} else {
-			nonprintable_streak = 1;
-		}
-		src++;
-		srcmax--;
-	}
-	return chars;
-}
-
-#define VBUSCHANNEL_ADDACHAR(ch, p, remain, chars) \
-	do {					   \
-		if (remain <= 0)		   \
-			break;			   \
-		*p = ch;			   \
-		p++;  chars++;  remain--;	   \
-	} while (0)
-
-/**
- * vbuschannel_itoa() - convert non-negative int to string
- * @p: destination string
- * @remain: max number of bytes that can be written to @p
- * @num: input int to convert
- *
- * Converts the non-negative value at @num to an ascii decimal string
- * at @p, writing at most @remain bytes.  Note there is NO '\0' termination
- * written to @p.
- *
- * Return: number of bytes written to @p
- *
- */
-static inline int
-vbuschannel_itoa(char *p, int remain, int num)
-{
-	int digits = 0;
-	char s[32];
-	int i;
-
-	if (num == 0) {
-		/* '0' is a special case */
-		if (remain <= 0)
-			return 0;
-		*p = '0';
-		return 1;
-	}
-	/* form a backwards decimal ascii string in <s> */
-	while (num > 0) {
-		if (digits >= (int)sizeof(s))
-			return 0;
-		s[digits++] = (num % 10) + '0';
-		num = num / 10;
-	}
-	if (remain < digits) {
-		/* not enough room left at <p> to hold number, so fill with
-		 * '?'
-		 */
-		for (i = 0; i < remain; i++, p++)
-			*p = '?';
-		return remain;
-	}
-	/* plug in the decimal ascii string representing the number, by */
-	/* reversing the string we just built in <s> */
-	i = digits;
-	while (i > 0) {
-		i--;
-		*p = s[i];
-		p++;
-	}
-	return digits;
-}
-
-/**
- * vbuschannel_devinfo_to_string() - format a struct ultra_vbus_deviceinfo
- *                                   to a printable string
  * @devinfo: the struct ultra_vbus_deviceinfo to format
- * @p: destination string area
+ * @seq: seq_file to write to
- * @remain: size of destination string area in bytes
  * @devix: the device index to be included in the output data, or -1 if no
  *         device index is to be included
  *
- * Reads @devInfo, and converts its contents to a printable string at @p,
+ * Reads @devInfo, and writes it in human-readable notation to @seq.
- * writing at most @remain bytes. Note there is NO '\0' termination
- * written to @p.
- *
- * Return: number of bytes written to @p
  */
-static inline int
+static inline void
-vbuschannel_devinfo_to_string(struct ultra_vbus_deviceinfo *devinfo,
+vbuschannel_print_devinfo(struct ultra_vbus_deviceinfo *devinfo,
-			      char *p, int remain, int devix)
+			  struct seq_file *seq, int devix)
 {
-	char *psrc;
+	if (!isprint(devinfo->devtype[0]))
-	int nsrc, x, i, pad;
+		return; /* uninitialized vbus device entry */
-	int chars = 0;
 
-	psrc = &devinfo->devtype[0];
+	if (devix >= 0)
-	nsrc = sizeof(devinfo->devtype);
+		seq_printf(seq, "[%d]", devix);
-	if (vbuschannel_sanitize_buffer(NULL, 0, psrc, nsrc) <= 0)
+	else
-		return 0;
+		/* vbus device entry is for bus or chipset */
+		seq_puts(seq, "   ");
 
-	/* emit device index */
+	/*
-	if (devix >= 0) {
+	 * Note: because the s-Par back-end is free to scribble in this area,
-		VBUSCHANNEL_ADDACHAR('[', p, remain, chars);
+	 * we never assume '\0'-termination.
-		x = vbuschannel_itoa(p, remain, devix);
+	 */
-		p += x;
+	seq_printf(seq, "%-*.*s ", (int)sizeof(devinfo->devtype),
-		remain -= x;
+		   (int)sizeof(devinfo->devtype), devinfo->devtype);
-		chars += x;
+	seq_printf(seq, "%-*.*s ", (int)sizeof(devinfo->drvname),
-		VBUSCHANNEL_ADDACHAR(']', p, remain, chars);
+		   (int)sizeof(devinfo->drvname), devinfo->drvname);
-	} else {
+	seq_printf(seq, "%.*s\n", (int)sizeof(devinfo->infostrs),
-		VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
+		   devinfo->infostrs);
-		VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-		VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-	}
-
-	/* emit device type */
-	x = vbuschannel_sanitize_buffer(p, remain, psrc, nsrc);
-	p += x;
-	remain -= x;
-	chars += x;
-	pad = 15 - x;		/* pad device type to be exactly 15 chars */
-	for (i = 0; i < pad; i++)
-		VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-	VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-
-	/* emit driver name */
-	psrc = &devinfo->drvname[0];
-	nsrc = sizeof(devinfo->drvname);
-	x = vbuschannel_sanitize_buffer(p, remain, psrc, nsrc);
-	p += x;
-	remain -= x;
-	chars += x;
-	pad = 15 - x;		/* pad driver name to be exactly 15 chars */
-	for (i = 0; i < pad; i++)
-		VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-	VBUSCHANNEL_ADDACHAR(' ', p, remain, chars);
-
-	/* emit strings */
-	psrc = &devinfo->infostrs[0];
-	nsrc = sizeof(devinfo->infostrs);
-	x = vbuschannel_sanitize_buffer(p, remain, psrc, nsrc);
-	p += x;
-	remain -= x;
-	chars += x;
-	VBUSCHANNEL_ADDACHAR('\n', p, remain, chars);
-
-	return chars;
 }
 
 struct spar_vbus_headerinfo {

drivers/staging/unisys/visorbus/visorbus_main.c

@@ -14,6 +14,7 @@
  * details.
  */
 
+#include <linux/debugfs.h>
 #include <linux/uuid.h>
 
 #include "visorbus.h"
@@ -33,6 +34,7 @@ static int visorbus_forcenomatch;
 #define POLLJIFFIES_NORMALCHANNEL     10
 
 static int busreg_rc = -ENODEV; /* stores the result from bus registration */
+static struct dentry *visorbus_debugfs_dir;
 
 /*
  * DEVICE type attributes
@@ -151,6 +153,8 @@ visorbus_release_busdevice(struct device *xdev)
 {
 	struct visor_device *dev = dev_get_drvdata(xdev);
 
+	debugfs_remove(dev->debugfs_client_bus_info);
+	debugfs_remove_recursive(dev->debugfs_dir);
 	kfree(dev);
 }
 
@@ -349,70 +353,6 @@ static ssize_t channel_id_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(channel_id);
 
-static ssize_t client_bus_info_show(struct device *dev,
-				    struct device_attribute *attr,
-				    char *buf) {
-	struct visor_device *vdev = to_visor_device(dev);
-	struct visorchannel *channel = vdev->visorchannel;
-
-	int i, shift, remain = PAGE_SIZE;
-	unsigned long off;
-	char *pos = buf;
-	u8 *partition_name;
-	struct ultra_vbus_deviceinfo dev_info;
-
-	partition_name = "";
-	if (channel) {
-		if (vdev->name)
-			partition_name = vdev->name;
-		shift = snprintf(pos, remain,
-				 "Client device / client driver info for %s partition (vbus #%u):\n",
-				 partition_name, vdev->chipset_bus_no);
-		pos += shift;
-		remain -= shift;
-		shift = visorchannel_read(channel,
-					  offsetof(struct
-						   spar_vbus_channel_protocol,
-						   chp_info),
-					  &dev_info, sizeof(dev_info));
-		if (shift >= 0) {
-			shift = vbuschannel_devinfo_to_string(&dev_info, pos,
-							      remain, -1);
-			pos += shift;
-			remain -= shift;
-		}
-		shift = visorchannel_read(channel,
-					  offsetof(struct
-						   spar_vbus_channel_protocol,
-						   bus_info),
-					  &dev_info, sizeof(dev_info));
-		if (shift >= 0) {
-			shift = vbuschannel_devinfo_to_string(&dev_info, pos,
-							      remain, -1);
-			pos += shift;
-			remain -= shift;
-		}
-		off = offsetof(struct spar_vbus_channel_protocol, dev_info);
-		i = 0;
-		while (off + sizeof(dev_info) <=
-		       visorchannel_get_nbytes(channel)) {
-			shift = visorchannel_read(channel,
-						  off, &dev_info,
-						  sizeof(dev_info));
-			if (shift >= 0) {
-				shift = vbuschannel_devinfo_to_string
-				    (&dev_info, pos, remain, i);
-				pos += shift;
-				remain -= shift;
-			}
-			off += sizeof(dev_info);
-			i++;
-		}
-	}
-	return PAGE_SIZE - remain;
-}
-static DEVICE_ATTR_RO(client_bus_info);
-
 static struct attribute *dev_attrs[] = {
 		&dev_attr_partition_handle.attr,
 		&dev_attr_partition_guid.attr,
@@ -420,7 +360,6 @@ static struct attribute *dev_attrs[] = {
 		&dev_attr_channel_addr.attr,
 		&dev_attr_channel_bytes.attr,
 		&dev_attr_channel_id.attr,
-		&dev_attr_client_bus_info.attr,
 		NULL
 };
 
@@ -433,6 +372,66 @@ static const struct attribute_group *visorbus_groups[] = {
 		NULL
 };
 
+/*
+ *  BUS debugfs entries
+ *
+ *  define & implement display of debugfs attributes under
+ *  /sys/kernel/debug/visorbus/visorbus<n>.
+ */
+
+static int client_bus_info_debugfs_show(struct seq_file *seq, void *v)
+{
+	struct visor_device *vdev = seq->private;
+	struct visorchannel *channel = vdev->visorchannel;
+
+	int i;
+	unsigned long off;
+	struct ultra_vbus_deviceinfo dev_info;
+
+	if (!channel)
+		return 0;
+
+	seq_printf(seq,
+		   "Client device / client driver info for %s partition (vbus #%u):\n",
+		   ((vdev->name) ? (char *)(vdev->name) : ""),
+		   vdev->chipset_bus_no);
+	if (visorchannel_read(channel,
+			      offsetof(struct spar_vbus_channel_protocol,
+				       chp_info),
+			      &dev_info, sizeof(dev_info)) >= 0)
+		vbuschannel_print_devinfo(&dev_info, seq, -1);
+	if (visorchannel_read(channel,
+			      offsetof(struct spar_vbus_channel_protocol,
+				       bus_info),
+			      &dev_info, sizeof(dev_info)) >= 0)
+		vbuschannel_print_devinfo(&dev_info, seq, -1);
+	off = offsetof(struct spar_vbus_channel_protocol, dev_info);
+	i = 0;
+	while (off + sizeof(dev_info) <= visorchannel_get_nbytes(channel)) {
+		if (visorchannel_read(channel, off, &dev_info,
+				      sizeof(dev_info)) >= 0)
+			vbuschannel_print_devinfo(&dev_info, seq, i);
+		off += sizeof(dev_info);
+		i++;
+	}
+
+	return 0;
+}
+
+static int client_bus_info_debugfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, client_bus_info_debugfs_show,
+			   inode->i_private);
+}
+
+static const struct file_operations client_bus_info_debugfs_fops = {
+	.owner = THIS_MODULE,
+	.open = client_bus_info_debugfs_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
 static void
 dev_periodic_work(unsigned long __opaque)
 {
@@ -964,6 +963,7 @@ static int
 create_bus_instance(struct visor_device *dev)
 {
 	int id = dev->chipset_bus_no;
+	int err;
 	struct spar_vbus_headerinfo *hdr_info;
 
 	POSTCODE_LINUX_2(BUS_CREATE_ENTRY_PC, POSTCODE_SEVERITY_INFO);
@@ -977,11 +977,26 @@ create_bus_instance(struct visor_device *dev)
 	dev->device.groups = visorbus_groups;
 	dev->device.release = visorbus_release_busdevice;
 
+	dev->debugfs_dir = debugfs_create_dir(dev_name(&dev->device),
+					      visorbus_debugfs_dir);
+	if (!dev->debugfs_dir) {
+		err = -ENOMEM;
+		goto err_hdr_info;
+	}
+	dev->debugfs_client_bus_info =
+		debugfs_create_file("client_bus_info", S_IRUSR | S_IRGRP,
+				    dev->debugfs_dir, dev,
+				    &client_bus_info_debugfs_fops);
+	if (!dev->debugfs_client_bus_info) {
+		err = -ENOMEM;
+		goto err_debugfs_dir;
+	}
+
 	if (device_register(&dev->device) < 0) {
 		POSTCODE_LINUX_3(DEVICE_CREATE_FAILURE_PC, id,
 				 POSTCODE_SEVERITY_ERR);
-		kfree(hdr_info);
+		err = -ENODEV;
-		return -ENODEV;
+		goto err_debugfs_created;
 	}
 
 	if (get_vbus_header_info(dev->visorchannel, hdr_info) >= 0) {
@@ -996,6 +1011,16 @@ create_bus_instance(struct visor_device *dev)
 	list_add_tail(&dev->list_all, &list_all_bus_instances);
 	dev_set_drvdata(&dev->device, dev);
 	return 0;
+
+err_debugfs_created:
+	debugfs_remove(dev->debugfs_client_bus_info);
+
+err_debugfs_dir:
+	debugfs_remove_recursive(dev->debugfs_dir);
+
+err_hdr_info:
+	kfree(hdr_info);
+	return err;
 }
 
 /**
@@ -1273,6 +1298,11 @@ visorbus_init(void)
 	int err;
 
 	POSTCODE_LINUX_3(DRIVER_ENTRY_PC, 0, POSTCODE_SEVERITY_INFO);
+
+	visorbus_debugfs_dir = debugfs_create_dir("visorbus", NULL);
+	if (!visorbus_debugfs_dir)
+		return -ENOMEM;
+
 	bus_device_info_init(&clientbus_driverinfo, "clientbus", "visorbus");
 
 	err = create_bus_type();
@@ -1304,6 +1334,7 @@ visorbus_exit(void)
 		remove_bus_instance(dev);
 	}
 	remove_bus_type();
+	debugfs_remove_recursive(visorbus_debugfs_dir);
 }
 
 module_param_named(forcematch, visorbus_forcematch, int, S_IRUGO);