[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH net-next v3] xen-netback: Adding debugfs "io_ring_qX" files



On 08/07/14 18:39, Konrad Rzeszutek Wilk wrote:



+       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;
+
+       if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
+               return;

I am curious to how you tested this patch, as my reading of
the code above would imply that when xen_netback_dbg_root is
initialized - we won't continue within this function?
Indeed, I've just copy-pasted that snippet you wrote in your prev mail and I haven't tried it out, as it was a very small change. I'll fix it.

+
  static int netback_remove(struct xenbus_device *dev)
  {
        struct backend_info *be = dev_get_drvdata(&dev->dev);
@@ -246,8 +413,12 @@ 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
+               xenvif_debugfs_delif(be->vif);
+#endif /* CONFIG_DEBUG_FS */

Why don't you just leave it as it (without the #ifdef) and add an
empty function for the #else CONFIG_DEBUG_FS like:

#else
static inline void xenvif_debugfs_addif(struct xenvif_queue *queue) {}
static inline void xenvif_debugfs_delif(struct xenvif *vif) {}
#endif
It wouldn't change the end result, but from the code reader's point of view the current way is a little bit better, as (s)he doesn't need to check the declaration to realize it has effect only if that config option is enabled.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.