This patch adds debugfs capabilities to netback. There used to be a similar
patch floating around for classic kernel, but it used procfs. It is based on a
very similar blkback patch.
It creates xen-netback/[vifname]/io_ring_q[queueno] files, reading them output
various ring variables etc. Writing "kick" into it imitates an interrupt
happened, it can be useful to check whether the ring is just stalled due to a
missed interrupt.
Signed-off-by: Zoltan Kiss <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
v2:
- use sequential files
- put everything behind the right config option
- fix error handling
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 2532ce8..a2ba1f2 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -44,6 +44,7 @@
#include <xen/interface/grant_table.h>
#include <xen/grant_table.h>
#include <xen/xenbus.h>
+#include <linux/debugfs.h>
typedef unsigned int pending_ring_idx_t;
#define INVALID_PENDING_RING_IDX (~0U)
@@ -224,6 +225,10 @@ struct xenvif {
struct xenvif_queue *queues;
unsigned int num_queues; /* active queues, resource allocated */
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *xenvif_dbg_root;
+#endif /* CONFIG_DEBUG_FS */
+
/* Miscellaneous private stuff. */
struct net_device *dev;
};
@@ -297,10 +302,16 @@ static inline pending_ring_idx_t nr_pending_reqs(struct xenvif_queue *queue)
/* Callback from stack when TX packet can be released */
void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
+irqreturn_t xenvif_interrupt(int irq, void *dev_id);
+
extern bool separate_tx_rx_irq;
extern unsigned int rx_drain_timeout_msecs;
extern unsigned int rx_drain_timeout_jiffies;
extern unsigned int xenvif_max_queues;
+#ifdef CONFIG_DEBUG_FS
+extern struct dentry *xen_netback_dbg_root;
+#endif /* CONFIG_DEBUG_FS */
+
#endif /* __XEN_NETBACK__COMMON_H__ */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 9e97c7c..ef75b45 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -102,7 +102,7 @@ static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
+irqreturn_t xenvif_interrupt(int irq, void *dev_id)
{
xenvif_tx_interrupt(irq, dev_id);
xenvif_rx_interrupt(irq, dev_id);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 1844a47..77127ca 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1987,6 +1987,13 @@ static int __init netback_init(void)
rx_drain_timeout_jiffies = msecs_to_jiffies(rx_drain_timeout_msecs);
+#ifdef CONFIG_DEBUG_FS
+ xen_netback_dbg_root = debugfs_create_dir("xen-netback", NULL);
+ if (IS_ERR_OR_NULL(xen_netback_dbg_root))
+ pr_warn("Init of debugfs returned %ld!\n",
+ PTR_ERR(xen_netback_dbg_root));
+#endif /* CONFIG_DEBUG_FS */
+
return 0;
failed_init:
@@ -1997,6 +2004,10 @@ module_init(netback_init);
static void __exit netback_fini(void)
{
+#ifdef CONFIG_DEBUG_FS
+ if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
+ debugfs_remove_recursive(xen_netback_dbg_root);
+#endif /* CONFIG_DEBUG_FS */
xenvif_xenbus_fini();
}
module_exit(netback_fini);
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 3d85acd..657884d 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -44,6 +44,164 @@ static void unregister_hotplug_status_watch(struct backend_info *be);
static void set_backend_state(struct backend_info *be,
enum xenbus_state state);
+#ifdef CONFIG_DEBUG_FS
+struct dentry *xen_netback_dbg_root = NULL;
+
+static int xenvif_read_io_ring(struct seq_file *m, void *v)
+{
+ struct xenvif_queue *queue = m->private;
+ struct xen_netif_tx_back_ring *tx_ring = &queue->tx;
+ struct xen_netif_rx_back_ring *rx_ring = &queue->rx;
+
+ if (tx_ring->sring) {
+ struct xen_netif_tx_sring *sring = tx_ring->sring;
+
+ seq_printf(m, "Queue %d\nTX: nr_ents %u\n", queue->id,
+ tx_ring->nr_ents);
+ seq_printf(m, "req prod %u (%d) cons %u (%d) event %u (%d)\n",
+ sring->req_prod,
+ sring->req_prod - sring->rsp_prod,
+ tx_ring->req_cons,
+ tx_ring->req_cons - sring->rsp_prod,
+ sring->req_event,
+ sring->req_event - sring->rsp_prod);
+ seq_printf(m, "rsp prod %u (base) pvt %u (%d) event %u (%d)\n",
+ sring->rsp_prod,
+ tx_ring->rsp_prod_pvt,
+ tx_ring->rsp_prod_pvt - sring->rsp_prod,
+ sring->rsp_event,
+ sring->rsp_event - sring->rsp_prod);
+ seq_printf(m, "pending prod %u pending cons %u nr_pending_reqs %u\n",
+ queue->pending_prod,
+ queue->pending_cons,
+ nr_pending_reqs(queue));
+ seq_printf(m, "dealloc prod %u dealloc cons %u dealloc_queue %u\n\n",
+ queue->dealloc_prod,
+ queue->dealloc_cons,
+ queue->dealloc_prod - queue->dealloc_cons);
+ }
+
+ if (rx_ring->sring) {
+ struct xen_netif_rx_sring *sring = rx_ring->sring;
+
+ seq_printf(m, "RX: nr_ents %u\n", rx_ring->nr_ents);
+ seq_printf(m, "req prod %u (%d) cons %u (%d) event %u (%d)\n",
+ sring->req_prod,
+ sring->req_prod - sring->rsp_prod,
+ rx_ring->req_cons,
+ rx_ring->req_cons - sring->rsp_prod,
+ sring->req_event,
+ sring->req_event - sring->rsp_prod);
+ seq_printf(m, "rsp prod %u (base) pvt %u (%d) event %u (%d)\n\n",
+ sring->rsp_prod,
+ rx_ring->rsp_prod_pvt,
+ rx_ring->rsp_prod_pvt - sring->rsp_prod,
+ sring->rsp_event,
+ sring->rsp_event - sring->rsp_prod);
+ }
+
+ seq_printf(m, "NAPI state: %lx NAPI weight: %d TX queue len %u\n"
+ "Credit timer_pending: %d, credit: %lu, usec: %lu\n"
+ "remaining: %lu, expires: %lu, now: %lu\n",
+ queue->napi.state, queue->napi.weight,
+ skb_queue_len(&queue->tx_queue),
+ timer_pending(&queue->credit_timeout),
+ queue->credit_bytes,
+ queue->credit_usec,
+ queue->remaining_credit,
+ queue->credit_timeout.expires,
+ jiffies);
+
+ return 0;
+}
+
+static ssize_t
+xenvif_write_io_ring(struct file *filp, const char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ struct xenvif_queue *queue =
+ ((struct seq_file *)filp->private_data)->private;
+ int len;
+ char write[sizeof("kick")];
+
+ /* don't allow partial writes and check the length */
+ if (*ppos != 0)
+ return 0;
+ if (count < sizeof("kick") - 1)
+ return -ENOSPC;
+
+ len = simple_write_to_buffer(write,
+ sizeof(write),
+ ppos,
+ buf,
+ count);
+ if (len < 0)
+ return len;
+
+ if (!strncmp(write, "kick", sizeof("kick") - 1))
+ xenvif_interrupt(0, (void *)queue);
+ else
+ pr_warn("Unknown command to io_ring. Available: kick\n");
+ return count;
+}
+
+static int xenvif_dump_open(struct inode *inode, struct file *filp)
+{
+ int ret;
+ void *queue = NULL;
+
+ if (inode->i_private)
+ queue = inode->i_private;
+ ret = single_open(filp, xenvif_read_io_ring, queue);
+ filp->f_mode |= FMODE_PWRITE;
+ return ret;
+}
+
+static const struct file_operations xenvif_dbg_io_ring_ops_fops = {
+ .owner = THIS_MODULE,
+ .open = xenvif_dump_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = xenvif_write_io_ring,
+};
+
+static void xenvif_debugfs_addif(struct xenvif_queue *queue)
+{
+ struct dentry *pfile;
+ struct xenvif *vif = queue->vif;
+ int i;
+
+ vif->xenvif_dbg_root = debugfs_create_dir(vif->dev->name,
+ xen_netback_dbg_root);
+ if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root)) {
+ for (i = 0; i < vif->num_queues; ++i) {
+ char filename[sizeof("io_ring_q") + 4];
+
+ snprintf(filename, sizeof(filename), "io_ring_q%d", i);
+ pfile = debugfs_create_file(filename,
+ 0600,
+ vif->xenvif_dbg_root,
+ &vif->queues[i],
+ &xenvif_dbg_io_ring_ops_fops);
+ if (IS_ERR_OR_NULL(pfile))
+ pr_warn("Creation of io_ring file returned %ld!\n",
+ PTR_ERR(pfile));
+ }
+ } else
+ netdev_warn(vif->dev,
+ "Creation of vif debugfs dir returned %ld!\n",
+ PTR_ERR(vif->xenvif_dbg_root));
+}
+
+static void xenvif_debugfs_delif(struct xenvif *vif)
+{
+ if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root))
+ debugfs_remove_recursive(vif->xenvif_dbg_root);
+ vif->xenvif_dbg_root = NULL;
+}
+#endif /* CONFIG_DEBUG_FS */
+
static int netback_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
@@ -246,8 +404,13 @@ static void backend_create_xenvif(struct backend_info *be)
static void backend_disconnect(struct backend_info *be)
{
- if (be->vif)
+ if (be->vif) {
+#ifdef CONFIG_DEBUG_FS
+ if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
+ xenvif_debugfs_delif(be->vif);
+#endif /* CONFIG_DEBUG_FS */
xenvif_disconnect(be->vif);
+ }
}
static void backend_connect(struct backend_info *be)
@@ -560,6 +723,10 @@ static void connect(struct backend_info *be)
be->vif->num_queues = queue_index;
goto err;
}
+#ifdef CONFIG_DEBUG_FS
+ if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
+ xenvif_debugfs_addif(queue);
+#endif /* CONFIG_DEBUG_FS */
}
/* Initialisation completed, tell core driver the number of
On Wed, Jul 02, 2014 at 08:53:50PM +0100, Zoltan Kiss wrote:
> This patch adds debugfs capabilities to netback. There used to be a similar
> patch floating around for classic kernel, but it used procfs. It is based on a
> very similar blkback patch.
> It creates xen-netback/[vifname]/io_ring_q[queueno] files, reading them output
> various ring variables etc. Writing "kick" into it imitates an interrupt
> happened, it can be useful to check whether the ring is just stalled due to a
> missed interrupt.
>
> Signed-off-by: Zoltan Kiss <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> v2:
> - use sequential files
> - put everything behind the right config option
> - fix error handling
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 2532ce8..a2ba1f2 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -44,6 +44,7 @@
> #include <xen/interface/grant_table.h>
> #include <xen/grant_table.h>
> #include <xen/xenbus.h>
> +#include <linux/debugfs.h>
>
> typedef unsigned int pending_ring_idx_t;
> #define INVALID_PENDING_RING_IDX (~0U)
> @@ -224,6 +225,10 @@ struct xenvif {
> struct xenvif_queue *queues;
> unsigned int num_queues; /* active queues, resource allocated */
>
> +#ifdef CONFIG_DEBUG_FS
> + struct dentry *xenvif_dbg_root;
> +#endif /* CONFIG_DEBUG_FS */
> +
> /* Miscellaneous private stuff. */
> struct net_device *dev;
> };
> @@ -297,10 +302,16 @@ static inline pending_ring_idx_t nr_pending_reqs(struct xenvif_queue *queue)
> /* Callback from stack when TX packet can be released */
> void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success);
>
> +irqreturn_t xenvif_interrupt(int irq, void *dev_id);
> +
> extern bool separate_tx_rx_irq;
>
> extern unsigned int rx_drain_timeout_msecs;
> extern unsigned int rx_drain_timeout_jiffies;
> extern unsigned int xenvif_max_queues;
>
> +#ifdef CONFIG_DEBUG_FS
> +extern struct dentry *xen_netback_dbg_root;
> +#endif /* CONFIG_DEBUG_FS */
> +
> #endif /* __XEN_NETBACK__COMMON_H__ */
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 9e97c7c..ef75b45 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -102,7 +102,7 @@ static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> +irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> {
> xenvif_tx_interrupt(irq, dev_id);
> xenvif_rx_interrupt(irq, dev_id);
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 1844a47..77127ca 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1987,6 +1987,13 @@ static int __init netback_init(void)
>
> rx_drain_timeout_jiffies = msecs_to_jiffies(rx_drain_timeout_msecs);
>
> +#ifdef CONFIG_DEBUG_FS
> + xen_netback_dbg_root = debugfs_create_dir("xen-netback", NULL);
> + if (IS_ERR_OR_NULL(xen_netback_dbg_root))
> + pr_warn("Init of debugfs returned %ld!\n",
> + PTR_ERR(xen_netback_dbg_root));
> +#endif /* CONFIG_DEBUG_FS */
> +
> return 0;
>
> failed_init:
> @@ -1997,6 +2004,10 @@ module_init(netback_init);
>
> static void __exit netback_fini(void)
> {
> +#ifdef CONFIG_DEBUG_FS
> + if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
> + debugfs_remove_recursive(xen_netback_dbg_root);
> +#endif /* CONFIG_DEBUG_FS */
> xenvif_xenbus_fini();
> }
> module_exit(netback_fini);
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 3d85acd..657884d 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -44,6 +44,164 @@ static void unregister_hotplug_status_watch(struct backend_info *be);
> static void set_backend_state(struct backend_info *be,
> enum xenbus_state state);
>
> +#ifdef CONFIG_DEBUG_FS
> +struct dentry *xen_netback_dbg_root = NULL;
> +
> +static int xenvif_read_io_ring(struct seq_file *m, void *v)
> +{
> + struct xenvif_queue *queue = m->private;
> + struct xen_netif_tx_back_ring *tx_ring = &queue->tx;
> + struct xen_netif_rx_back_ring *rx_ring = &queue->rx;
> +
> + if (tx_ring->sring) {
> + struct xen_netif_tx_sring *sring = tx_ring->sring;
> +
> + seq_printf(m, "Queue %d\nTX: nr_ents %u\n", queue->id,
> + tx_ring->nr_ents);
> + seq_printf(m, "req prod %u (%d) cons %u (%d) event %u (%d)\n",
> + sring->req_prod,
> + sring->req_prod - sring->rsp_prod,
> + tx_ring->req_cons,
> + tx_ring->req_cons - sring->rsp_prod,
> + sring->req_event,
> + sring->req_event - sring->rsp_prod);
> + seq_printf(m, "rsp prod %u (base) pvt %u (%d) event %u (%d)\n",
> + sring->rsp_prod,
> + tx_ring->rsp_prod_pvt,
> + tx_ring->rsp_prod_pvt - sring->rsp_prod,
> + sring->rsp_event,
> + sring->rsp_event - sring->rsp_prod);
> + seq_printf(m, "pending prod %u pending cons %u nr_pending_reqs %u\n",
> + queue->pending_prod,
> + queue->pending_cons,
> + nr_pending_reqs(queue));
> + seq_printf(m, "dealloc prod %u dealloc cons %u dealloc_queue %u\n\n",
> + queue->dealloc_prod,
> + queue->dealloc_cons,
> + queue->dealloc_prod - queue->dealloc_cons);
> + }
> +
> + if (rx_ring->sring) {
> + struct xen_netif_rx_sring *sring = rx_ring->sring;
> +
> + seq_printf(m, "RX: nr_ents %u\n", rx_ring->nr_ents);
> + seq_printf(m, "req prod %u (%d) cons %u (%d) event %u (%d)\n",
> + sring->req_prod,
> + sring->req_prod - sring->rsp_prod,
> + rx_ring->req_cons,
> + rx_ring->req_cons - sring->rsp_prod,
> + sring->req_event,
> + sring->req_event - sring->rsp_prod);
> + seq_printf(m, "rsp prod %u (base) pvt %u (%d) event %u (%d)\n\n",
> + sring->rsp_prod,
> + rx_ring->rsp_prod_pvt,
> + rx_ring->rsp_prod_pvt - sring->rsp_prod,
> + sring->rsp_event,
> + sring->rsp_event - sring->rsp_prod);
> + }
> +
> + seq_printf(m, "NAPI state: %lx NAPI weight: %d TX queue len %u\n"
> + "Credit timer_pending: %d, credit: %lu, usec: %lu\n"
> + "remaining: %lu, expires: %lu, now: %lu\n",
> + queue->napi.state, queue->napi.weight,
> + skb_queue_len(&queue->tx_queue),
> + timer_pending(&queue->credit_timeout),
> + queue->credit_bytes,
> + queue->credit_usec,
> + queue->remaining_credit,
> + queue->credit_timeout.expires,
> + jiffies);
> +
> + return 0;
> +}
> +
> +static ssize_t
> +xenvif_write_io_ring(struct file *filp, const char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + struct xenvif_queue *queue =
> + ((struct seq_file *)filp->private_data)->private;
> + int len;
> + char write[sizeof("kick")];
<blinks>
> +
> + /* don't allow partial writes and check the length */
> + if (*ppos != 0)
> + return 0;
> + if (count < sizeof("kick") - 1)
Um, could you just use a #define please?
> + return -ENOSPC;
> +
> + len = simple_write_to_buffer(write,
> + sizeof(write),
> + ppos,
> + buf,
> + count);
> + if (len < 0)
> + return len;
> +
> + if (!strncmp(write, "kick", sizeof("kick") - 1))
> + xenvif_interrupt(0, (void *)queue);
> + else
> + pr_warn("Unknown command to io_ring. Available: kick\n");
It is 'io_ring_q%d'? Why don't you split that back out to the user
instead of the console?
> + return count;
> +}
> +
> +static int xenvif_dump_open(struct inode *inode, struct file *filp)
> +{
> + int ret;
> + void *queue = NULL;
> +
> + if (inode->i_private)
> + queue = inode->i_private;
> + ret = single_open(filp, xenvif_read_io_ring, queue);
> + filp->f_mode |= FMODE_PWRITE;
> + return ret;
> +}
> +
> +static const struct file_operations xenvif_dbg_io_ring_ops_fops = {
> + .owner = THIS_MODULE,
> + .open = xenvif_dump_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> + .write = xenvif_write_io_ring,
> +};
> +
> +static void xenvif_debugfs_addif(struct xenvif_queue *queue)
> +{
> + struct dentry *pfile;
> + struct xenvif *vif = queue->vif;
> + int i;
> +
> + vif->xenvif_dbg_root = debugfs_create_dir(vif->dev->name,
> + xen_netback_dbg_root);
> + if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root)) {
> + for (i = 0; i < vif->num_queues; ++i) {
> + char filename[sizeof("io_ring_q") + 4];
> +
> + snprintf(filename, sizeof(filename), "io_ring_q%d", i);
> + pfile = debugfs_create_file(filename,
> + 0600,
Pls use macros for that.
> + vif->xenvif_dbg_root,
> + &vif->queues[i],
> + &xenvif_dbg_io_ring_ops_fops);
> + if (IS_ERR_OR_NULL(pfile))
> + pr_warn("Creation of io_ring file returned %ld!\n",
> + PTR_ERR(pfile));
> + }
> + } else
> + netdev_warn(vif->dev,
> + "Creation of vif debugfs dir returned %ld!\n",
> + PTR_ERR(vif->xenvif_dbg_root));
> +}
> +
> +static void xenvif_debugfs_delif(struct xenvif *vif)
> +{
Could you add:
if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
return;
> + if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root))
> + debugfs_remove_recursive(vif->xenvif_dbg_root);
> + vif->xenvif_dbg_root = NULL;
> +}
> +#endif /* CONFIG_DEBUG_FS */
> +
> static int netback_remove(struct xenbus_device *dev)
> {
> struct backend_info *be = dev_get_drvdata(&dev->dev);
> @@ -246,8 +404,13 @@ static void backend_create_xenvif(struct backend_info *be)
>
> static void backend_disconnect(struct backend_info *be)
> {
> - if (be->vif)
> + if (be->vif) {
> +#ifdef CONFIG_DEBUG_FS
> + if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
> + xenvif_debugfs_delif(be->vif);
.. and then you can remove these 'if' here
> +#endif /* CONFIG_DEBUG_FS */
> xenvif_disconnect(be->vif);
> + }
> }
>
> static void backend_connect(struct backend_info *be)
> @@ -560,6 +723,10 @@ static void connect(struct backend_info *be)
> be->vif->num_queues = queue_index;
> goto err;
> }
> +#ifdef CONFIG_DEBUG_FS
> + if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
.. and here.
> + xenvif_debugfs_addif(queue);
> +#endif /* CONFIG_DEBUG_FS */
> }
>
> /* Initialisation completed, tell core driver the number of
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> http://lists.xen.org/xen-devel
On 02/07/14 21:02, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 02, 2014 at 08:53:50PM +0100, Zoltan Kiss wrote:
>> +static ssize_t
>> +xenvif_write_io_ring(struct file *filp, const char __user *buf, size_t count,
>> + loff_t *ppos)
>> +{
>> + struct xenvif_queue *queue =
>> + ((struct seq_file *)filp->private_data)->private;
>> + int len;
>> + char write[sizeof("kick")];
>
> <blinks>
>> +
>> + /* don't allow partial writes and check the length */
>> + if (*ppos != 0)
>> + return 0;
>> + if (count < sizeof("kick") - 1)
>
> Um, could you just use a #define please?
OK
>
>> + return -ENOSPC;
>> +
>> + len = simple_write_to_buffer(write,
>> + sizeof(write),
>> + ppos,
>> + buf,
>> + count);
>> + if (len < 0)
>> + return len;
>> +
>> + if (!strncmp(write, "kick", sizeof("kick") - 1))
>> + xenvif_interrupt(0, (void *)queue);
>> + else
>> + pr_warn("Unknown command to io_ring. Available: kick\n");
>
> It is 'io_ring_q%d'?
Yes
> Why don't you split that back out to the user
> instead of the console?
How do you mean? Printing it into buf? I don't think the user cares
about its content after doing the write syscall. More importantly, I
don't know if that buffer is big enough to hold the message.
>
>
>> + return count;
>> +}
>> +
>> +static int xenvif_dump_open(struct inode *inode, struct file *filp)
>> +{
>> + int ret;
>> + void *queue = NULL;
>> +
>> + if (inode->i_private)
>> + queue = inode->i_private;
>> + ret = single_open(filp, xenvif_read_io_ring, queue);
>> + filp->f_mode |= FMODE_PWRITE;
>> + return ret;
>> +}
>> +
>> +static const struct file_operations xenvif_dbg_io_ring_ops_fops = {
>> + .owner = THIS_MODULE,
>> + .open = xenvif_dump_open,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .release = single_release,
>> + .write = xenvif_write_io_ring,
>> +};
>> +
>> +static void xenvif_debugfs_addif(struct xenvif_queue *queue)
>> +{
>> + struct dentry *pfile;
>> + struct xenvif *vif = queue->vif;
>> + int i;
>> +
>> + vif->xenvif_dbg_root = debugfs_create_dir(vif->dev->name,
>> + xen_netback_dbg_root);
>> + if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root)) {
>> + for (i = 0; i < vif->num_queues; ++i) {
>> + char filename[sizeof("io_ring_q") + 4];
>> +
>> + snprintf(filename, sizeof(filename), "io_ring_q%d", i);
>> + pfile = debugfs_create_file(filename,
>> + 0600,
>
> Pls use macros for that.
OK
>> + vif->xenvif_dbg_root,
>> + &vif->queues[i],
>> + &xenvif_dbg_io_ring_ops_fops);
>> + if (IS_ERR_OR_NULL(pfile))
>> + pr_warn("Creation of io_ring file returned %ld!\n",
>> + PTR_ERR(pfile));
>> + }
>> + } else
>> + netdev_warn(vif->dev,
>> + "Creation of vif debugfs dir returned %ld!\n",
>> + PTR_ERR(vif->xenvif_dbg_root));
>> +}
>> +
>> +static void xenvif_debugfs_delif(struct xenvif *vif)
>> +{
>
> Could you add:
>
> if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
> return;
>
Ok
>> + if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root))
>> + debugfs_remove_recursive(vif->xenvif_dbg_root);
>> + vif->xenvif_dbg_root = NULL;
>> +}
>> +#endif /* CONFIG_DEBUG_FS */
>> +
>> static int netback_remove(struct xenbus_device *dev)
>> {
>> struct backend_info *be = dev_get_drvdata(&dev->dev);
>> @@ -246,8 +404,13 @@ static void backend_create_xenvif(struct backend_info *be)
>>
>> static void backend_disconnect(struct backend_info *be)
>> {
>> - if (be->vif)
>> + if (be->vif) {
>> +#ifdef CONFIG_DEBUG_FS
>> + if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
>> + xenvif_debugfs_delif(be->vif);
>
> .. and then you can remove these 'if' here
>> +#endif /* CONFIG_DEBUG_FS */
>> xenvif_disconnect(be->vif);
>> + }
>> }
>>
>> static void backend_connect(struct backend_info *be)
>> @@ -560,6 +723,10 @@ static void connect(struct backend_info *be)
>> be->vif->num_queues = queue_index;
>> goto err;
>> }
>> +#ifdef CONFIG_DEBUG_FS
>> + if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
>
> .. and here.
>> + xenvif_debugfs_addif(queue);
>> +#endif /* CONFIG_DEBUG_FS */
>> }
>>
>> /* Initialisation completed, tell core driver the number of
>>
>> _______________________________________________
>> Xen-devel mailing list
>> [email protected]
>> http://lists.xen.org/xen-devel
On Tue, Jul 08, 2014 at 03:33:50PM +0100, Zoltan Kiss wrote:
> On 02/07/14 21:02, Konrad Rzeszutek Wilk wrote:
> >On Wed, Jul 02, 2014 at 08:53:50PM +0100, Zoltan Kiss wrote:
> >>+static ssize_t
> >>+xenvif_write_io_ring(struct file *filp, const char __user *buf, size_t count,
> >>+ loff_t *ppos)
> >>+{
> >>+ struct xenvif_queue *queue =
> >>+ ((struct seq_file *)filp->private_data)->private;
> >>+ int len;
> >>+ char write[sizeof("kick")];
> >
> ><blinks>
> >>+
> >>+ /* don't allow partial writes and check the length */
> >>+ if (*ppos != 0)
> >>+ return 0;
> >>+ if (count < sizeof("kick") - 1)
> >
> >Um, could you just use a #define please?
> OK
>
> >
> >>+ return -ENOSPC;
> >>+
> >>+ len = simple_write_to_buffer(write,
> >>+ sizeof(write),
> >>+ ppos,
> >>+ buf,
> >>+ count);
> >>+ if (len < 0)
> >>+ return len;
> >>+
> >>+ if (!strncmp(write, "kick", sizeof("kick") - 1))
> >>+ xenvif_interrupt(0, (void *)queue);
> >>+ else
> >>+ pr_warn("Unknown command to io_ring. Available: kick\n");
> >
> >It is 'io_ring_q%d'?
> Yes
>
> >Why don't you split that back out to the user
> >instead of the console?
> How do you mean? Printing it into buf? I don't think the user cares about
> its content after doing the write syscall. More importantly, I don't know if
> that buffer is big enough to hold the message.
Isn't it 4K?
Either way I think you need to do 'count = -EINVAL' here.