Johnny Hughes
2019-02-04 c1f36c28393a7bb126cbf436cd6a4077a5b5c313
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
From 7dc5b326b4430a292de27c404b2f36fcdb5ec67f Mon Sep 17 00:00:00 2001
From: Miquel Raynal <miquel.raynal@free-electrons.com>
Date: Wed, 8 Nov 2017 08:59:40 +0100
Subject: [PATCH 09/46] net: mvpp2: fix GOP statistics loop start and stop
 conditions
 
GOP statistics from all ports of one instance of the driver are gathered
with one work recalled in loop in a workqueue. The loop is started when
a port is up, and stopped when a port is down. This last condition is
obviously wrong.
 
Fix this by having a work per port. This way, starting and stoping it
when the port is up or down will be fine, while minimizing unnecessary
CPU usage.
 
Fixes: 118d6298f6f0 ("net: mvpp2: add ethtool GOP statistics")
Reported-by: Stefan Chulski <stefanc@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit e5c500eb298a9f5ef9b80d16fcea9662c89467b7)
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 62 +++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 32 deletions(-)
 
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index a79d2ff..6c20e81 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -885,9 +885,7 @@ struct mvpp2 {
     /* Maximum number of RXQs per port */
     unsigned int max_port_rxqs;
 
-    /* Workqueue to gather hardware statistics with its lock */
-    struct mutex gather_stats_lock;
-    struct delayed_work stats_work;
+    /* Workqueue to gather hardware statistics */
     char queue_name[30];
     struct workqueue_struct *stats_queue;
 };
@@ -955,6 +953,10 @@ struct mvpp2_port {
     struct mvpp2_pcpu_stats __percpu *stats;
     u64 *ethtool_stats;
 
+    /* Per-port work and its lock to gather hardware statistics */
+    struct mutex gather_stats_lock;
+    struct delayed_work stats_work;
+
     phy_interface_t phy_interface;
     struct device_node *phy_node;
     struct phy *comphy;
@@ -4895,32 +4897,25 @@ static void mvpp2_ethtool_get_strings(struct net_device *netdev, u32 sset,
 static void mvpp2_gather_hw_statistics(struct work_struct *work)
 {
     struct delayed_work *del_work = to_delayed_work(work);
-    struct mvpp2 *priv = container_of(del_work, struct mvpp2, stats_work);
-    struct mvpp2_port *port;
+    struct mvpp2_port *port = container_of(del_work, struct mvpp2_port,
+                           stats_work);
     u64 *pstats;
-    int i, j;
-
-    mutex_lock(&priv->gather_stats_lock);
+    int i;
 
-    for (i = 0; i < priv->port_count; i++) {
-        if (!priv->port_list[i])
-            continue;
+    mutex_lock(&port->gather_stats_lock);
 
-        port = priv->port_list[i];
-        pstats = port->ethtool_stats;
-        for (j = 0; j < ARRAY_SIZE(mvpp2_ethtool_regs); j++)
-            *pstats++ += mvpp2_read_count(port,
-                              &mvpp2_ethtool_regs[j]);
-    }
+    pstats = port->ethtool_stats;
+    for (i = 0; i < ARRAY_SIZE(mvpp2_ethtool_regs); i++)
+        *pstats++ += mvpp2_read_count(port, &mvpp2_ethtool_regs[i]);
 
     /* No need to read again the counters right after this function if it
      * was called asynchronously by the user (ie. use of ethtool).
      */
-    cancel_delayed_work(&priv->stats_work);
-    queue_delayed_work(priv->stats_queue, &priv->stats_work,
+    cancel_delayed_work(&port->stats_work);
+    queue_delayed_work(port->priv->stats_queue, &port->stats_work,
                MVPP2_MIB_COUNTERS_STATS_DELAY);
 
-    mutex_unlock(&priv->gather_stats_lock);
+    mutex_unlock(&port->gather_stats_lock);
 }
 
 static void mvpp2_ethtool_get_stats(struct net_device *dev,
@@ -4928,13 +4923,15 @@ static void mvpp2_ethtool_get_stats(struct net_device *dev,
 {
     struct mvpp2_port *port = netdev_priv(dev);
 
-    /* Update statistics for all ports, copy only those actually needed */
-    mvpp2_gather_hw_statistics(&port->priv->stats_work.work);
+    /* Update statistics for the given port, then take the lock to avoid
+     * concurrent accesses on the ethtool_stats structure during its copy.
+     */
+    mvpp2_gather_hw_statistics(&port->stats_work.work);
 
-    mutex_lock(&port->priv->gather_stats_lock);
+    mutex_lock(&port->gather_stats_lock);
     memcpy(data, port->ethtool_stats,
            sizeof(u64) * ARRAY_SIZE(mvpp2_ethtool_regs));
-    mutex_unlock(&port->priv->gather_stats_lock);
+    mutex_unlock(&port->gather_stats_lock);
 }
 
 static int mvpp2_ethtool_get_sset_count(struct net_device *dev, int sset)
@@ -7089,7 +7086,7 @@ static int mvpp2_open(struct net_device *dev)
         mvpp22_init_rss(port);
 
     /* Start hardware statistics gathering */
-    queue_delayed_work(priv->stats_queue, &priv->stats_work,
+    queue_delayed_work(priv->stats_queue, &port->stats_work,
                MVPP2_MIB_COUNTERS_STATS_DELAY);
 
     return 0;
@@ -7136,8 +7133,7 @@ static int mvpp2_stop(struct net_device *dev)
     mvpp2_cleanup_rxqs(port);
     mvpp2_cleanup_txqs(port);
 
-    cancel_delayed_work_sync(&priv->stats_work);
-    flush_workqueue(priv->stats_queue);
+    cancel_delayed_work_sync(&port->stats_work);
 
     return 0;
 }
@@ -7889,6 +7885,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
         goto err_free_stats;
     }
 
+    mutex_init(&port->gather_stats_lock);
+    INIT_DELAYED_WORK(&port->stats_work, mvpp2_gather_hw_statistics);
+
     mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
 
     port->tx_ring_size = MVPP2_MAX_TXD;
@@ -8356,7 +8355,6 @@ static int mvpp2_probe(struct platform_device *pdev)
      * smallest packets (64B) will overflow a 32-bit counter in less than
      * 30 seconds. Then, use a workqueue to fill 64-bit counters.
      */
-    mutex_init(&priv->gather_stats_lock);
     snprintf(priv->queue_name, sizeof(priv->queue_name),
          "stats-wq-%s%s", netdev_name(priv->port_list[0]->dev),
          priv->port_count > 1 ? "+" : "");
@@ -8366,8 +8364,6 @@ static int mvpp2_probe(struct platform_device *pdev)
         goto err_mg_clk;
     }
 
-    INIT_DELAYED_WORK(&priv->stats_work, mvpp2_gather_hw_statistics);
-
     platform_set_drvdata(pdev, priv);
     return 0;
 
@@ -8389,12 +8385,14 @@ static int mvpp2_remove(struct platform_device *pdev)
     struct device_node *port_node;
     int i = 0;
 
+    flush_workqueue(priv->stats_queue);
     destroy_workqueue(priv->stats_queue);
-    mutex_destroy(&priv->gather_stats_lock);
 
     for_each_available_child_of_node(dn, port_node) {
-        if (priv->port_list[i])
+        if (priv->port_list[i]) {
+            mutex_destroy(&priv->port_list[i]->gather_stats_lock);
             mvpp2_port_remove(priv->port_list[i]);
+        }
         i++;
     }
 
-- 
2.7.4