On Fri, Jul 11, 2014 at 07:11:45PM -0400, Erik Arfvidson wrote:
> This patch adds the virtpci debugfs directory and the info entry
> inside of it.
>
> Signed-off-by: Erik Arfvidson <erik.arfvidson@xxxxxxxxxx>
> Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
> ---
> v3: Fixed formating and comments. Also added debufs_remove_recursive() and
> simple simple_read_from_buffer()
> v2: fixed comments and applied Dan Carpenter suggestions
> drivers/staging/unisys/virtpci/virtpci.c | 108 ++++++++++++++++++++++++++++++-
> 1 file changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c
> index 7d840b0..6ea29f5 100644
> --- a/drivers/staging/unisys/virtpci/virtpci.c
> +++ b/drivers/staging/unisys/virtpci/virtpci.c
> @@ -36,6 +36,7 @@
> #include <linux/mod_devicetable.h>
> #include <linux/if_ether.h>
> #include <linux/version.h>
> +#include <linux/debugfs.h>
> #include "version.h"
> #include "guestlinuxdebug.h"
> #include "timskmodutils.h"
> @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
> static int virtpci_device_probe(struct device *dev);
> static int virtpci_device_remove(struct device *dev);
>
> +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
> + size_t len, loff_t *offset);
> +
> +static const struct file_operations debugfs_info_fops = {
> + .read = info_debugfs_read,
> +};
>
> /*****************************************************/
> /* Globals */
> @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
>
> /* filled in with info about this driver, wrt it servicing client busses */
> static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
> -
> +/*****************************************************/
> +/* DebugFS entries */
> +/*****************************************************/
> +/* dentry is used to create the debugfs entry directory
> + * for virtpci
> + */
> +static struct dentry *virtpci_debugfs_dir;
> +static struct dentry *info_debugfs_entry;
> +/* info_debugfs_entry is used to tell virtpci to display current info
> + * kept in the driver
> + */
You don't need this variable as all you do is set it, and then never do
anything with it. Worse case, make it a local variable. As you never
check it, just don't even do that...
> +#define DIR_DEBUGFS_ENTRY "virtpci"
> +#define INFO_DEBUGFS_ENTRY_FN "info"
You only reference these #defines once, no need for them at all.
>
> struct virtpci_busdev {
> struct device virtpci_bus_device;
> @@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver *drv)
> DBGINF("Leaving\n");
> }
> EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
> +/*****************************************************/
> +/* debugfs filesystem functions */
> +/*****************************************************/
> +struct print_vbus_info {
> + int *length;
> + char *buf;
> +};
> +
> +static int print_vbus(struct device *vbus, void *data)
> +{
> + struct print_vbus_info *p = (struct print_vbus_info *)data;
> +
> + *p->length += sprintf(p->buf + *p->length, "bus_id:%s\n",
> + dev_name(vbus));
> + return 0;
> +}
> +
> +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
> + size_t len, loff_t *offset)
> +{
> + ssize_t bytes_read = 0;
> + int str_pos = 0;
> + struct virtpci_dev *tmpvpcidev;
> + unsigned long flags;
> + struct print_vbus_info printparam;
> + char *vbuf;
> +
> + vbuf = kzalloc(len, GFP_KERNEL);
Sorry I missed this last time, but what happens if userspace asks for
10Gb? Nasty problems will happen if you try to allocate that. Or
100Mb. Set a reasonable upper bound here please.
Remember:
All input is evil.
> + if (!vbuf)
> + return -ENOMEM;
> +
> + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
> + " Virtual PCI Bus devices\n");
Why the leading ' '?
> + printparam.length = &str_pos;
> + printparam.buf = vbuf;
> + if (bus_for_each_dev(&virtpci_bus_type, NULL,
> + (void *) &printparam, print_vbus))
> + LOGERR("Failed to find bus\n");
> +
> + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
> + "\n Virtual PCI devices\n");
> + read_lock_irqsave(&VpcidevListLock, flags);
> + tmpvpcidev = VpcidevListHead;
> + while (tmpvpcidev) {
> + if (tmpvpcidev->devtype == VIRTHBA_TYPE) {
> + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
> + "[%d:%d] VHba:%08x:%08x max-config:%d-%d-%d-%d",
> + tmpvpcidev->busNo, tmpvpcidev->deviceNo,
> + tmpvpcidev->scsi.wwnn.wwnn1,
> + tmpvpcidev->scsi.wwnn.wwnn2,
> + tmpvpcidev->scsi.max.max_channel,
> + tmpvpcidev->scsi.max.max_id,
> + tmpvpcidev->scsi.max.max_lun,
> + tmpvpcidev->scsi.max.cmd_per_lun);
> + } else {
> + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
> + "[%d:%d] VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d",
> + tmpvpcidev->busNo, tmpvpcidev->deviceNo,
> + tmpvpcidev->net.mac_addr[0],
> + tmpvpcidev->net.mac_addr[1],
> + tmpvpcidev->net.mac_addr[2],
> + tmpvpcidev->net.mac_addr[3],
> + tmpvpcidev->net.mac_addr[4],
> + tmpvpcidev->net.mac_addr[5],
> + tmpvpcidev->net.num_rcv_bufs,
> + tmpvpcidev->net.mtu);
> + }
> + str_pos += scnprintf(vbuf + str_pos,
> + len - str_pos, " chanptr:%p\n",
> + tmpvpcidev->queueinfo.chan);
> + tmpvpcidev = tmpvpcidev->next;
> + }
> + read_unlock_irqrestore(&VpcidevListLock, flags);
> +
> + str_pos += scnprintf(vbuf + str_pos, len - str_pos, "\n");
> + bytes_read = simple_read_from_buffer(buf, len, offset, vbuf, str_pos);
> + kfree(vbuf);
> + return bytes_read;
> +}
>
> /*****************************************************/
> /* Module Init & Exit functions */
> @@ -1426,7 +1524,10 @@ static int __init virtpci_mod_init(void)
>
> LOGINF("successfully registered virtpci_ctrlchan_func (0x%p) as callback.\n",
> (void *) &virtpci_ctrlchan_func);
> -
> + /* create debugfs directory. */
And the file in it...
> + virtpci_debugfs_dir = debugfs_create_dir(DIR_DEBUGFS_ENTRY, NULL);
> + info_debugfs_entry = debugfs_create_file(INFO_DEBUGFS_ENTRY_FN, S_IRUSR,
> + virtpci_debugfs_dir, NULL, &debugfs_info_fops);
> LOGINF("Leaving\n");
> POSTCODE_LINUX_2(VPCI_CREATE_EXIT_PC, POSTCODE_SEVERITY_INFO);
> return 0;
> @@ -1442,7 +1543,8 @@ static void __exit virtpci_mod_exit(void)
>
> device_unregister(&virtpci_rootbus_device);
> bus_unregister(&virtpci_bus_type);
> -
> + /* debug_fs file and directory removal. */
No '_' in "debugfs".
> + debugfs_remove(virtpci_debugfs_dir);
Again, I thought I pointed out that this is the wrong call...
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
