Thread (10 messages) 10 messages, 3 authors, 2011-09-27

Re: [PATCH] ehea: Remove sleep at .ndo_get_stats

flat view

From: Eric Dumazet <hidden>
Date: 2011-09-26 19:43:06

Le lundi 26 septembre 2011 à 16:21 -0300, brenohl@br.ibm.com a écrit :
quoted hunk ↗ jump to hunk
Currently ehea ndo_get_stats can sleep in two places, in a hcall
and in a GFP_KERNEL alloc, which is not correct.
This patch creates a delayed workqueue that grabs the information each 1
sec from the hardware, and place it into the device structure, so that,
.ndo_get_stats quickly returns the device structure statistics block.

Signed-off-by: Breno Leitao <redacted>
---
 drivers/net/ethernet/ibm/ehea/ehea.h      |    1 +
 drivers/net/ethernet/ibm/ehea/ehea_main.c |   53 ++++++++++++++++------------
 2 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ehea/ehea.h b/drivers/net/ethernet/ibm/ehea/ehea.h
index 7dd5e6a..0b8e6a9 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea.h
+++ b/drivers/net/ethernet/ibm/ehea/ehea.h
@@ -459,6 +459,7 @@ struct ehea_port {
 	struct ehea_mc_list *mc_list;	 /* Multicast MAC addresses */
 	struct ehea_eq *qp_eq;
 	struct work_struct reset_task;
+	struct delayed_work stats_work;
 	struct mutex port_lock;
 	char int_aff_name[EHEA_IRQ_NAME_SIZE];
 	int allmulti;			 /* Indicates IFF_ALLMULTI state */
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 583bcd3..976988d 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -331,16 +331,34 @@ static struct net_device_stats *ehea_get_stats(struct net_device *dev)
 {
 	struct ehea_port *port = netdev_priv(dev);
 	struct net_device_stats *stats = &port->stats;
-	struct hcp_ehea_port_cb2 *cb2;
-	u64 hret, rx_packets, tx_packets, rx_bytes = 0, tx_bytes = 0;
 	int i;
 
-	memset(stats, 0, sizeof(*stats));
+	for (i = 0; i < port->num_def_qps; i++) {
+		stats->rx_packets += port->port_res[i].rx_packets;
+		stats->rx_bytes   += port->port_res[i].rx_bytes;
+	}
+
No.

You _really_ need the temporary variables, and set stats->field once
final result is known. Right now, you are adding data over and over.

Please test your patches :(
quoted hunk ↗ jump to hunk
+	for (i = 0; i < port->num_def_qps + port->num_add_tx_qps; i++) {
+		stats->tx_packets += port->port_res[i].tx_packets;
+		stats->tx_bytes   += port->port_res[i].tx_bytes;
+	}
+
+	return &port->stats;
	return stats;
+}
+
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help