[PATCH 01/15] staging: unisys: Remove num_visornic_open array
Benjamin Romer
benjamin.romer at unisys.com
Tue Jul 21 13:55:35 UTC 2015
From: Neil Horman <nhorman at redhat.com>
As pointed out in a recent review, the num_visornic_open array didn't do
anything useful, and it exposed a potential race in the visornic code that
could arise while taking down a net interface while reading from the
debugfs files. Fix that by removing the array entirely, and just iterating
over all the registered netdevs in a given namespace, filtering on them
having visornic ops (to identify which are ours), and having their queues
not be stopped (identifying that they are up). This should prevent any oops
conditions happening due to changing state in that array, and save us a
bunch of code too.
Signed-off-by: Neil Horman <nhorman at redhat.com>
Signed-off-by: Benjamin Romer <benjamin.romer at unisys.com>
---
drivers/staging/unisys/visornic/visornic_main.c | 341 +++++++++++-------------
1 file changed, 154 insertions(+), 187 deletions(-)
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index c28a347..5bb6cf4 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -196,8 +196,6 @@ struct visornic_devdata {
struct chanstat chstat;
};
-/* array of open devices maintained by open() and close() */
-static struct net_device *num_visornic_open[VISORNICSOPENMAX];
/* List of all visornic_devdata structs,
* linked via the list_all member
@@ -325,178 +323,15 @@ static void visor_thread_stop(struct visor_thread_info *thrinfo)
thrinfo->id = 0;
}
-/* DebugFS code */
-static ssize_t info_debugfs_read(struct file *file, char __user *buf,
- size_t len, loff_t *offset)
-{
- int i;
- ssize_t bytes_read = 0;
- int str_pos = 0;
- struct visornic_devdata *devdata;
- char *vbuf;
-
- if (len > MAX_BUF)
- len = MAX_BUF;
- vbuf = kzalloc(len, GFP_KERNEL);
- if (!vbuf)
- return -ENOMEM;
-
- /* for each vnic channel
- * dump out channel specific data
- */
- for (i = 0; i < VISORNICSOPENMAX; i++) {
- if (!num_visornic_open[i])
- continue;
-
- devdata = netdev_priv(num_visornic_open[i]);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- "Vnic i = %d\n", i);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- "netdev = %s (0x%p), MAC Addr %pM\n",
- num_visornic_open[i]->name,
- num_visornic_open[i],
- num_visornic_open[i]->dev_addr);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- "VisorNic Dev Info = 0x%p\n", devdata);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " num_rcv_bufs = %d\n",
- devdata->num_rcv_bufs);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " max_oustanding_next_xmits = %d\n",
- devdata->max_outstanding_net_xmits);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " upper_threshold_net_xmits = %d\n",
- devdata->upper_threshold_net_xmits);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " lower_threshold_net_xmits = %d\n",
- devdata->lower_threshold_net_xmits);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " queuefullmsg_logged = %d\n",
- devdata->queuefullmsg_logged);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.got_rcv = %lu\n",
- devdata->chstat.got_rcv);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.got_enbdisack = %lu\n",
- devdata->chstat.got_enbdisack);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.got_xmit_done = %lu\n",
- devdata->chstat.got_xmit_done);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.xmit_fail = %lu\n",
- devdata->chstat.xmit_fail);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.sent_enbdis = %lu\n",
- devdata->chstat.sent_enbdis);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.sent_promisc = %lu\n",
- devdata->chstat.sent_promisc);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.sent_post = %lu\n",
- devdata->chstat.sent_post);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.sent_xmit = %lu\n",
- devdata->chstat.sent_xmit);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.reject_count = %lu\n",
- devdata->chstat.reject_count);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " chstat.extra_rcvbufs_sent = %lu\n",
- devdata->chstat.extra_rcvbufs_sent);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_rcv0 = %lu\n", devdata->n_rcv0);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_rcv1 = %lu\n", devdata->n_rcv1);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_rcv2 = %lu\n", devdata->n_rcv2);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_rcvx = %lu\n", devdata->n_rcvx);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " num_rcvbuf_in_iovm = %d\n",
- atomic_read(&devdata->num_rcvbuf_in_iovm));
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " alloc_failed_in_if_needed_cnt = %lu\n",
- devdata->alloc_failed_in_if_needed_cnt);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " alloc_failed_in_repost_rtn_cnt = %lu\n",
- devdata->alloc_failed_in_repost_rtn_cnt);
- /* str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- * " inner_loop_limit_reached_cnt = %lu\n",
- * devdata->inner_loop_limit_reached_cnt);
- */
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " found_repost_rcvbuf_cnt = %lu\n",
- devdata->found_repost_rcvbuf_cnt);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " repost_found_skb_cnt = %lu\n",
- devdata->repost_found_skb_cnt);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_repost_deficit = %lu\n",
- devdata->n_repost_deficit);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " bad_rcv_buf = %lu\n",
- devdata->bad_rcv_buf);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " n_rcv_packets_not_accepted = %lu\n",
- devdata->n_rcv_packets_not_accepted);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " interrupts_rcvd = %llu\n",
- devdata->interrupts_rcvd);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " interrupts_notme = %llu\n",
- devdata->interrupts_notme);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " interrupts_disabled = %llu\n",
- devdata->interrupts_disabled);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " busy_cnt = %llu\n",
- devdata->busy_cnt);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " flow_control_upper_hits = %llu\n",
- devdata->flow_control_upper_hits);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " flow_control_lower_hits = %llu\n",
- devdata->flow_control_lower_hits);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " thread_wait_ms = %d\n",
- devdata->thread_wait_ms);
- str_pos += scnprintf(vbuf + str_pos, len - str_pos,
- " netif_queue = %s\n",
- netif_queue_stopped(devdata->netdev) ?
- "stopped" : "running");
- }
- bytes_read = simple_read_from_buffer(buf, len, offset, vbuf, str_pos);
- kfree(vbuf);
- return bytes_read;
-}
-
static ssize_t enable_ints_write(struct file *file,
const char __user *buffer,
size_t count, loff_t *ppos)
{
- char buf[4];
- int i, new_value;
- struct visornic_devdata *devdata;
-
- if (count >= ARRAY_SIZE(buf))
- return -EINVAL;
-
- buf[count] = '\0';
- if (copy_from_user(buf, buffer, count))
- return -EFAULT;
-
- i = kstrtoint(buf, 10, &new_value);
- if (i != 0)
- return -EFAULT;
-
- /* set all counts to new_value usually 0 */
- for (i = 0; i < VISORNICSOPENMAX; i++) {
- if (num_visornic_open[i]) {
- devdata = netdev_priv(num_visornic_open[i]);
- /* TODO update features bit in channel */
- }
- }
-
+ /*
+ * Don't want to break ABI here by having a debugfs
+ * file that no longer exists or is writable, so
+ * lets just make this a vestigual function
+ */
return count;
}
@@ -760,13 +595,6 @@ visornic_disable_with_timeout(struct net_device *netdev, const int timeout)
}
}
- /* remove references from array */
- for (i = 0; i < VISORNICSOPENMAX; i++)
- if (num_visornic_open[i] == netdev) {
- num_visornic_open[i] = NULL;
- break;
- }
-
return 0;
}
@@ -882,16 +710,6 @@ visornic_enable_with_timeout(struct net_device *netdev, const int timeout)
return -EIO;
}
- /* find an open slot in the array to save off VisorNic references
- * for debug
- */
- for (i = 0; i < VISORNICSOPENMAX; i++) {
- if (!num_visornic_open[i]) {
- num_visornic_open[i] = netdev;
- break;
- }
- }
-
return 0;
}
@@ -1642,6 +1460,155 @@ static const struct net_device_ops visornic_dev_ops = {
.ndo_set_rx_mode = visornic_set_multi,
};
+/* DebugFS code */
+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 visornic_devdata *devdata;
+ struct net_device *dev;
+ char *vbuf;
+
+ if (len > MAX_BUF)
+ len = MAX_BUF;
+ vbuf = kzalloc(len, GFP_KERNEL);
+ if (!vbuf)
+ return -ENOMEM;
+
+ /* for each vnic channel
+ * dump out channel specific data
+ */
+ rcu_read_lock();
+ for_each_netdev_rcu(current->nsproxy->net_ns, dev) {
+ /*
+ * Only consider netdevs that are visornic, and are open
+ */
+ if ((dev->netdev_ops != &visornic_dev_ops) ||
+ (!netif_queue_stopped(dev)))
+ continue;
+
+ devdata = netdev_priv(dev);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ "netdev = %s (0x%p), MAC Addr %pM\n",
+ dev->name,
+ dev,
+ dev->dev_addr);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ "VisorNic Dev Info = 0x%p\n", devdata);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " num_rcv_bufs = %d\n",
+ devdata->num_rcv_bufs);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " max_oustanding_next_xmits = %d\n",
+ devdata->max_outstanding_net_xmits);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " upper_threshold_net_xmits = %d\n",
+ devdata->upper_threshold_net_xmits);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " lower_threshold_net_xmits = %d\n",
+ devdata->lower_threshold_net_xmits);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " queuefullmsg_logged = %d\n",
+ devdata->queuefullmsg_logged);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.got_rcv = %lu\n",
+ devdata->chstat.got_rcv);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.got_enbdisack = %lu\n",
+ devdata->chstat.got_enbdisack);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.got_xmit_done = %lu\n",
+ devdata->chstat.got_xmit_done);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.xmit_fail = %lu\n",
+ devdata->chstat.xmit_fail);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.sent_enbdis = %lu\n",
+ devdata->chstat.sent_enbdis);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.sent_promisc = %lu\n",
+ devdata->chstat.sent_promisc);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.sent_post = %lu\n",
+ devdata->chstat.sent_post);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.sent_xmit = %lu\n",
+ devdata->chstat.sent_xmit);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.reject_count = %lu\n",
+ devdata->chstat.reject_count);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " chstat.extra_rcvbufs_sent = %lu\n",
+ devdata->chstat.extra_rcvbufs_sent);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_rcv0 = %lu\n", devdata->n_rcv0);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_rcv1 = %lu\n", devdata->n_rcv1);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_rcv2 = %lu\n", devdata->n_rcv2);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_rcvx = %lu\n", devdata->n_rcvx);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " num_rcvbuf_in_iovm = %d\n",
+ atomic_read(&devdata->num_rcvbuf_in_iovm));
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " alloc_failed_in_if_needed_cnt = %lu\n",
+ devdata->alloc_failed_in_if_needed_cnt);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " alloc_failed_in_repost_rtn_cnt = %lu\n",
+ devdata->alloc_failed_in_repost_rtn_cnt);
+ /* str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ * " inner_loop_limit_reached_cnt = %lu\n",
+ * devdata->inner_loop_limit_reached_cnt);
+ */
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " found_repost_rcvbuf_cnt = %lu\n",
+ devdata->found_repost_rcvbuf_cnt);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " repost_found_skb_cnt = %lu\n",
+ devdata->repost_found_skb_cnt);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_repost_deficit = %lu\n",
+ devdata->n_repost_deficit);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " bad_rcv_buf = %lu\n",
+ devdata->bad_rcv_buf);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " n_rcv_packets_not_accepted = %lu\n",
+ devdata->n_rcv_packets_not_accepted);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " interrupts_rcvd = %llu\n",
+ devdata->interrupts_rcvd);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " interrupts_notme = %llu\n",
+ devdata->interrupts_notme);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " interrupts_disabled = %llu\n",
+ devdata->interrupts_disabled);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " busy_cnt = %llu\n",
+ devdata->busy_cnt);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " flow_control_upper_hits = %llu\n",
+ devdata->flow_control_upper_hits);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " flow_control_lower_hits = %llu\n",
+ devdata->flow_control_lower_hits);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " thread_wait_ms = %d\n",
+ devdata->thread_wait_ms);
+ str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+ " netif_queue = %s\n",
+ netif_queue_stopped(devdata->netdev) ?
+ "stopped" : "running");
+ }
+ rcu_read_unlock();
+ bytes_read = simple_read_from_buffer(buf, len, offset, vbuf, str_pos);
+ kfree(vbuf);
+ return bytes_read;
+}
+
/**
* send_rcv_posts_if_needed
* @devdata: visornic device
--
2.1.4
More information about the devel
mailing list