netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics
@ 2022-02-13 19:12 Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool Colin Foster
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Colin Foster @ 2022-02-13 19:12 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jakub Kicinski, David S. Miller, UNGLinuxDriver,
	Alexandre Belloni, Claudiu Manoil, Vladimir Oltean

Ocelot loops over memory regions to gather stats on different ports.
These regions are mostly continuous, and are ordered. This patch set
uses that information to break the stats reads into regions that can get
read in bulk.

The motiviation is for general cleanup, but also for SPI. Performing two
back-to-back reads on a SPI bus require toggling the CS line, holding,
re-toggling the CS line, sending 3 address bytes, sending N padding
bytes, then actually performing the read. Bulk reads could reduce almost
all of that overhead, but require that the reads are performed via
regmap_bulk_read.

Verified with eth0 hooked up to the CPU port:
# ethtool -S eth0 | grep -v ": 0"
NIC statistics:
     Good Rx Frames: 905
     Rx Octets: 78848
     Good Tx Frames: 691
     Tx Octets: 52516
     Rx + Tx 65-127 Octet Frames: 1574
     Rx + Tx 128-255 Octet Frames: 22
     Net Octets: 131364
     Rx DMA chan 0: head_enqueue: 1
     Rx DMA chan 0: tail_enqueue: 1032
     Rx DMA chan 0: busy_dequeue: 628
     Rx DMA chan 0: good_dequeue: 905
     Tx DMA chan 0: head_enqueue: 346
     Tx DMA chan 0: tail_enqueue: 345
     Tx DMA chan 0: misqueued: 345
     Tx DMA chan 0: empty_dequeue: 346
     Tx DMA chan 0: good_dequeue: 691
     p00_rx_octets: 52516
     p00_rx_unicast: 691
     p00_rx_frames_65_to_127_octets: 691
     p00_tx_octets: 78848
     p00_tx_unicast: 905
     p00_tx_frames_65_to_127_octets: 883
     p00_tx_frames_128_255_octets: 22
     p00_tx_green_prio_0: 905

And with swp2 connected to swp3 with STP enabled:
# ethtool -S swp2 | grep -v ": 0"
NIC statistics:
     tx_packets: 379
     tx_bytes: 19708
     rx_packets: 1
     rx_bytes: 46
     rx_octets: 64
     rx_multicast: 1
     rx_frames_below_65_octets: 1
     rx_classified_drops: 1
     tx_octets: 44630
     tx_multicast: 387
     tx_broadcast: 290
     tx_frames_below_65_octets: 379
     tx_frames_65_to_127_octets: 294
     tx_frames_128_255_octets: 4
     tx_green_prio_0: 298
     tx_green_prio_7: 379
# ethtool -S swp3 | grep -v ": 0"
NIC statistics:
     tx_packets: 1
     tx_bytes: 52
     rx_packets: 713
     rx_bytes: 34148
     rx_octets: 46982
     rx_multicast: 407
     rx_broadcast: 306
     rx_frames_below_65_octets: 399
     rx_frames_65_to_127_octets: 310
     rx_frames_128_to_255_octets: 4
     rx_classified_drops: 399
     rx_green_prio_0: 314
     tx_octets: 64
     tx_multicast: 1
     tx_frames_below_65_octets: 1
     tx_green_prio_7: 1


v1 > v2: reword commit messages
v2 > v3: correctly mark this for net-next when sending
v3 > v4: calloc array instead of zalloc per review
v4 > v5:
    Apply CR suggestions for whitespace
    Fix calloc / zalloc mixup
    Properly destroy workqueues
    Add third commit to split long macros
v5 > v6:
    Fix functionality - v5 was improperly tested
    Add bugfix for ethtool mutex lock
    Remove unnecessary ethtool stats reads
v6 > v7:
    Remove mutex bug patch that was applied via net
    Rename function based on CR
    Add missed error check



Colin Foster (4):
  net: mscc: ocelot: remove unnecessary stat reading from ethtool
  net: ocelot: align macros for consistency
  net: mscc: ocelot: add ability to perform bulk reads
  net: mscc: ocelot: use bulk reads for stats

 drivers/net/ethernet/mscc/ocelot.c    | 95 ++++++++++++++++++++++-----
 drivers/net/ethernet/mscc/ocelot_io.c | 13 ++++
 include/soc/mscc/ocelot.h             | 57 +++++++++++-----
 3 files changed, 133 insertions(+), 32 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v7 net-next 1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool
  2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
@ 2022-02-13 19:12 ` Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 2/4] net: ocelot: align macros for consistency Colin Foster
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Colin Foster @ 2022-02-13 19:12 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jakub Kicinski, David S. Miller, UNGLinuxDriver,
	Alexandre Belloni, Claudiu Manoil, Vladimir Oltean

The ocelot_update_stats function only needs to read from one port, yet it
was updating the stats for all ports. Update to only read the stats that
are necessary.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 33 +++++++++++++++---------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index e6de86552df0..885288eb389c 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1746,27 +1746,24 @@ void ocelot_get_strings(struct ocelot *ocelot, int port, u32 sset, u8 *data)
 EXPORT_SYMBOL(ocelot_get_strings);
 
 /* Caller must hold &ocelot->stats_lock */
-static void ocelot_update_stats(struct ocelot *ocelot)
+static void ocelot_port_update_stats(struct ocelot *ocelot, int port)
 {
-	int i, j;
+	int j;
 
-	for (i = 0; i < ocelot->num_phys_ports; i++) {
-		/* Configure the port to read the stats from */
-		ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(i), SYS_STAT_CFG);
+	/* Configure the port to read the stats from */
+	ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(port), SYS_STAT_CFG);
 
-		for (j = 0; j < ocelot->num_stats; j++) {
-			u32 val;
-			unsigned int idx = i * ocelot->num_stats + j;
+	for (j = 0; j < ocelot->num_stats; j++) {
+		u32 val;
+		unsigned int idx = port * ocelot->num_stats + j;
 
-			val = ocelot_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
-					      ocelot->stats_layout[j].offset);
+		val = ocelot_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
+				      ocelot->stats_layout[j].offset);
 
-			if (val < (ocelot->stats[idx] & U32_MAX))
-				ocelot->stats[idx] += (u64)1 << 32;
+		if (val < (ocelot->stats[idx] & U32_MAX))
+			ocelot->stats[idx] += (u64)1 << 32;
 
-			ocelot->stats[idx] = (ocelot->stats[idx] &
-					      ~(u64)U32_MAX) + val;
-		}
+		ocelot->stats[idx] = (ocelot->stats[idx] & ~(u64)U32_MAX) + val;
 	}
 }
 
@@ -1775,9 +1772,11 @@ static void ocelot_check_stats_work(struct work_struct *work)
 	struct delayed_work *del_work = to_delayed_work(work);
 	struct ocelot *ocelot = container_of(del_work, struct ocelot,
 					     stats_work);
+	int i;
 
 	mutex_lock(&ocelot->stats_lock);
-	ocelot_update_stats(ocelot);
+	for (i = 0; i < ocelot->num_phys_ports; i++)
+		ocelot_port_update_stats(ocelot, i);
 	mutex_unlock(&ocelot->stats_lock);
 
 	queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
@@ -1791,7 +1790,7 @@ void ocelot_get_ethtool_stats(struct ocelot *ocelot, int port, u64 *data)
 	mutex_lock(&ocelot->stats_lock);
 
 	/* check and update now */
-	ocelot_update_stats(ocelot);
+	ocelot_port_update_stats(ocelot, port);
 
 	/* Copy all counters */
 	for (i = 0; i < ocelot->num_stats; i++)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 net-next 2/4] net: ocelot: align macros for consistency
  2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool Colin Foster
@ 2022-02-13 19:12 ` Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 3/4] net: mscc: ocelot: add ability to perform bulk reads Colin Foster
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Colin Foster @ 2022-02-13 19:12 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jakub Kicinski, David S. Miller, UNGLinuxDriver,
	Alexandre Belloni, Claudiu Manoil, Vladimir Oltean

In the ocelot.h file, several read / write macros were split across
multiple lines, while others weren't. Split all macros that exceed the 80
character column width and match the style of the rest of the file.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/soc/mscc/ocelot.h | 44 ++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 62cd61d4142e..14a6f4de8e1f 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -744,25 +744,39 @@ struct ocelot_policer {
 	u32 burst; /* bytes */
 };
 
-#define ocelot_read_ix(ocelot, reg, gi, ri) __ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
-#define ocelot_read_gix(ocelot, reg, gi) __ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi))
-#define ocelot_read_rix(ocelot, reg, ri) __ocelot_read_ix(ocelot, reg, reg##_RSZ * (ri))
-#define ocelot_read(ocelot, reg) __ocelot_read_ix(ocelot, reg, 0)
-
-#define ocelot_write_ix(ocelot, val, reg, gi, ri) __ocelot_write_ix(ocelot, val, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
-#define ocelot_write_gix(ocelot, val, reg, gi) __ocelot_write_ix(ocelot, val, reg, reg##_GSZ * (gi))
-#define ocelot_write_rix(ocelot, val, reg, ri) __ocelot_write_ix(ocelot, val, reg, reg##_RSZ * (ri))
+#define ocelot_read_ix(ocelot, reg, gi, ri) \
+	__ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
+#define ocelot_read_gix(ocelot, reg, gi) \
+	__ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi))
+#define ocelot_read_rix(ocelot, reg, ri) \
+	__ocelot_read_ix(ocelot, reg, reg##_RSZ * (ri))
+#define ocelot_read(ocelot, reg) \
+	__ocelot_read_ix(ocelot, reg, 0)
+
+#define ocelot_write_ix(ocelot, val, reg, gi, ri) \
+	__ocelot_write_ix(ocelot, val, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
+#define ocelot_write_gix(ocelot, val, reg, gi) \
+	__ocelot_write_ix(ocelot, val, reg, reg##_GSZ * (gi))
+#define ocelot_write_rix(ocelot, val, reg, ri) \
+	__ocelot_write_ix(ocelot, val, reg, reg##_RSZ * (ri))
 #define ocelot_write(ocelot, val, reg) __ocelot_write_ix(ocelot, val, reg, 0)
 
-#define ocelot_rmw_ix(ocelot, val, m, reg, gi, ri) __ocelot_rmw_ix(ocelot, val, m, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
-#define ocelot_rmw_gix(ocelot, val, m, reg, gi) __ocelot_rmw_ix(ocelot, val, m, reg, reg##_GSZ * (gi))
-#define ocelot_rmw_rix(ocelot, val, m, reg, ri) __ocelot_rmw_ix(ocelot, val, m, reg, reg##_RSZ * (ri))
+#define ocelot_rmw_ix(ocelot, val, m, reg, gi, ri) \
+	__ocelot_rmw_ix(ocelot, val, m, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
+#define ocelot_rmw_gix(ocelot, val, m, reg, gi) \
+	__ocelot_rmw_ix(ocelot, val, m, reg, reg##_GSZ * (gi))
+#define ocelot_rmw_rix(ocelot, val, m, reg, ri) \
+	__ocelot_rmw_ix(ocelot, val, m, reg, reg##_RSZ * (ri))
 #define ocelot_rmw(ocelot, val, m, reg) __ocelot_rmw_ix(ocelot, val, m, reg, 0)
 
-#define ocelot_field_write(ocelot, reg, val) regmap_field_write((ocelot)->regfields[(reg)], (val))
-#define ocelot_field_read(ocelot, reg, val) regmap_field_read((ocelot)->regfields[(reg)], (val))
-#define ocelot_fields_write(ocelot, id, reg, val) regmap_fields_write((ocelot)->regfields[(reg)], (id), (val))
-#define ocelot_fields_read(ocelot, id, reg, val) regmap_fields_read((ocelot)->regfields[(reg)], (id), (val))
+#define ocelot_field_write(ocelot, reg, val) \
+	regmap_field_write((ocelot)->regfields[(reg)], (val))
+#define ocelot_field_read(ocelot, reg, val) \
+	regmap_field_read((ocelot)->regfields[(reg)], (val))
+#define ocelot_fields_write(ocelot, id, reg, val) \
+	regmap_fields_write((ocelot)->regfields[(reg)], (id), (val))
+#define ocelot_fields_read(ocelot, id, reg, val) \
+	regmap_fields_read((ocelot)->regfields[(reg)], (id), (val))
 
 #define ocelot_target_read_ix(ocelot, target, reg, gi, ri) \
 	__ocelot_target_read_ix(ocelot, target, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 net-next 3/4] net: mscc: ocelot: add ability to perform bulk reads
  2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 2/4] net: ocelot: align macros for consistency Colin Foster
@ 2022-02-13 19:12 ` Colin Foster
  2022-02-13 19:12 ` [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats Colin Foster
  2022-02-14 13:40 ` [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: Colin Foster @ 2022-02-13 19:12 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jakub Kicinski, David S. Miller, UNGLinuxDriver,
	Alexandre Belloni, Claudiu Manoil, Vladimir Oltean

Regmap supports bulk register reads. Ocelot does not. This patch adds
support for Ocelot to invoke bulk regmap reads. That will allow any driver
that performs consecutive reads over memory regions to optimize that
access.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_io.c | 13 +++++++++++++
 include/soc/mscc/ocelot.h             |  5 +++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/mscc/ocelot_io.c b/drivers/net/ethernet/mscc/ocelot_io.c
index 7390fa3980ec..2067382d0ee1 100644
--- a/drivers/net/ethernet/mscc/ocelot_io.c
+++ b/drivers/net/ethernet/mscc/ocelot_io.c
@@ -10,6 +10,19 @@
 
 #include "ocelot.h"
 
+int __ocelot_bulk_read_ix(struct ocelot *ocelot, u32 reg, u32 offset, void *buf,
+			  int count)
+{
+	u16 target = reg >> TARGET_OFFSET;
+
+	WARN_ON(!target);
+
+	return regmap_bulk_read(ocelot->targets[target],
+				ocelot->map[target][reg & REG_MASK] + offset,
+				buf, count);
+}
+EXPORT_SYMBOL_GPL(__ocelot_bulk_read_ix);
+
 u32 __ocelot_read_ix(struct ocelot *ocelot, u32 reg, u32 offset)
 {
 	u16 target = reg >> TARGET_OFFSET;
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 14a6f4de8e1f..312b72558659 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -744,6 +744,9 @@ struct ocelot_policer {
 	u32 burst; /* bytes */
 };
 
+#define ocelot_bulk_read_rix(ocelot, reg, ri, buf, count) \
+	__ocelot_bulk_read_ix(ocelot, reg, reg##_RSZ * (ri), buf, count)
+
 #define ocelot_read_ix(ocelot, reg, gi, ri) \
 	__ocelot_read_ix(ocelot, reg, reg##_GSZ * (gi) + reg##_RSZ * (ri))
 #define ocelot_read_gix(ocelot, reg, gi) \
@@ -800,6 +803,8 @@ struct ocelot_policer {
 u32 ocelot_port_readl(struct ocelot_port *port, u32 reg);
 void ocelot_port_writel(struct ocelot_port *port, u32 val, u32 reg);
 void ocelot_port_rmwl(struct ocelot_port *port, u32 val, u32 mask, u32 reg);
+int __ocelot_bulk_read_ix(struct ocelot *ocelot, u32 reg, u32 offset, void *buf,
+			  int count);
 u32 __ocelot_read_ix(struct ocelot *ocelot, u32 reg, u32 offset);
 void __ocelot_write_ix(struct ocelot *ocelot, u32 val, u32 reg, u32 offset);
 void __ocelot_rmw_ix(struct ocelot *ocelot, u32 val, u32 mask, u32 reg,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats
  2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
                   ` (2 preceding siblings ...)
  2022-02-13 19:12 ` [PATCH v7 net-next 3/4] net: mscc: ocelot: add ability to perform bulk reads Colin Foster
@ 2022-02-13 19:12 ` Colin Foster
  2022-02-14  9:51   ` Vladimir Oltean
  2022-02-14 13:40 ` [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics patchwork-bot+netdevbpf
  4 siblings, 1 reply; 7+ messages in thread
From: Colin Foster @ 2022-02-13 19:12 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jakub Kicinski, David S. Miller, UNGLinuxDriver,
	Alexandre Belloni, Claudiu Manoil, Vladimir Oltean

Create and utilize bulk regmap reads instead of single access for gathering
stats. The background reading of statistics happens frequently, and over
a few contiguous memory regions.

High speed PCIe buses and MMIO access will probably see negligible
performance increase. Lower speed buses like SPI and I2C could see
significant performance increase, since the bus configuration and register
access times account for a large percentage of data transfer time.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 92 +++++++++++++++++++++++++-----
 include/soc/mscc/ocelot.h          |  8 +++
 2 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 885288eb389c..637fd79402e0 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1746,25 +1746,36 @@ void ocelot_get_strings(struct ocelot *ocelot, int port, u32 sset, u8 *data)
 EXPORT_SYMBOL(ocelot_get_strings);
 
 /* Caller must hold &ocelot->stats_lock */
-static void ocelot_port_update_stats(struct ocelot *ocelot, int port)
+static int ocelot_port_update_stats(struct ocelot *ocelot, int port)
 {
-	int j;
+	unsigned int idx = port * ocelot->num_stats;
+	struct ocelot_stats_region *region;
+	int err, j;
 
 	/* Configure the port to read the stats from */
 	ocelot_write(ocelot, SYS_STAT_CFG_STAT_VIEW(port), SYS_STAT_CFG);
 
-	for (j = 0; j < ocelot->num_stats; j++) {
-		u32 val;
-		unsigned int idx = port * ocelot->num_stats + j;
+	list_for_each_entry(region, &ocelot->stats_regions, node) {
+		err = ocelot_bulk_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
+					   region->offset, region->buf,
+					   region->count);
+		if (err)
+			return err;
 
-		val = ocelot_read_rix(ocelot, SYS_COUNT_RX_OCTETS,
-				      ocelot->stats_layout[j].offset);
+		for (j = 0; j < region->count; j++) {
+			u64 *stat = &ocelot->stats[idx + j];
+			u64 val = region->buf[j];
 
-		if (val < (ocelot->stats[idx] & U32_MAX))
-			ocelot->stats[idx] += (u64)1 << 32;
+			if (val < (*stat & U32_MAX))
+				*stat += (u64)1 << 32;
 
-		ocelot->stats[idx] = (ocelot->stats[idx] & ~(u64)U32_MAX) + val;
+			*stat = (*stat & ~(u64)U32_MAX) + val;
+		}
+
+		idx += region->count;
 	}
+
+	return err;
 }
 
 static void ocelot_check_stats_work(struct work_struct *work)
@@ -1772,31 +1783,40 @@ static void ocelot_check_stats_work(struct work_struct *work)
 	struct delayed_work *del_work = to_delayed_work(work);
 	struct ocelot *ocelot = container_of(del_work, struct ocelot,
 					     stats_work);
-	int i;
+	int i, err;
 
 	mutex_lock(&ocelot->stats_lock);
-	for (i = 0; i < ocelot->num_phys_ports; i++)
-		ocelot_port_update_stats(ocelot, i);
+	for (i = 0; i < ocelot->num_phys_ports; i++) {
+		err = ocelot_port_update_stats(ocelot, i);
+		if (err)
+			break;
+	}
 	mutex_unlock(&ocelot->stats_lock);
 
+	if (err)
+		dev_err(ocelot->dev, "Error %d updating ethtool stats\n",  err);
+
 	queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
 			   OCELOT_STATS_CHECK_DELAY);
 }
 
 void ocelot_get_ethtool_stats(struct ocelot *ocelot, int port, u64 *data)
 {
-	int i;
+	int i, err;
 
 	mutex_lock(&ocelot->stats_lock);
 
 	/* check and update now */
-	ocelot_port_update_stats(ocelot, port);
+	err = ocelot_port_update_stats(ocelot, port);
 
 	/* Copy all counters */
 	for (i = 0; i < ocelot->num_stats; i++)
 		*data++ = ocelot->stats[port * ocelot->num_stats + i];
 
 	mutex_unlock(&ocelot->stats_lock);
+
+	if (err)
+		dev_err(ocelot->dev, "Error %d updating ethtool stats\n", err);
 }
 EXPORT_SYMBOL(ocelot_get_ethtool_stats);
 
@@ -1809,6 +1829,41 @@ int ocelot_get_sset_count(struct ocelot *ocelot, int port, int sset)
 }
 EXPORT_SYMBOL(ocelot_get_sset_count);
 
+static int ocelot_prepare_stats_regions(struct ocelot *ocelot)
+{
+	struct ocelot_stats_region *region = NULL;
+	unsigned int last;
+	int i;
+
+	INIT_LIST_HEAD(&ocelot->stats_regions);
+
+	for (i = 0; i < ocelot->num_stats; i++) {
+		if (region && ocelot->stats_layout[i].offset == last + 1) {
+			region->count++;
+		} else {
+			region = devm_kzalloc(ocelot->dev, sizeof(*region),
+					      GFP_KERNEL);
+			if (!region)
+				return -ENOMEM;
+
+			region->offset = ocelot->stats_layout[i].offset;
+			region->count = 1;
+			list_add_tail(&region->node, &ocelot->stats_regions);
+		}
+
+		last = ocelot->stats_layout[i].offset;
+	}
+
+	list_for_each_entry(region, &ocelot->stats_regions, node) {
+		region->buf = devm_kcalloc(ocelot->dev, region->count,
+					   sizeof(*region->buf), GFP_KERNEL);
+		if (!region->buf)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
 int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 		       struct ethtool_ts_info *info)
 {
@@ -2809,6 +2864,13 @@ int ocelot_init(struct ocelot *ocelot)
 				 ANA_CPUQ_8021_CFG_CPUQ_BPDU_VAL(6),
 				 ANA_CPUQ_8021_CFG, i);
 
+	ret = ocelot_prepare_stats_regions(ocelot);
+	if (ret) {
+		destroy_workqueue(ocelot->stats_queue);
+		destroy_workqueue(ocelot->owq);
+		return ret;
+	}
+
 	INIT_DELAYED_WORK(&ocelot->stats_work, ocelot_check_stats_work);
 	queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
 			   OCELOT_STATS_CHECK_DELAY);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 312b72558659..d3291a5f7e88 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -542,6 +542,13 @@ struct ocelot_stat_layout {
 	char name[ETH_GSTRING_LEN];
 };
 
+struct ocelot_stats_region {
+	struct list_head node;
+	u32 offset;
+	int count;
+	u32 *buf;
+};
+
 enum ocelot_tag_prefix {
 	OCELOT_TAG_PREFIX_DISABLED	= 0,
 	OCELOT_TAG_PREFIX_NONE,
@@ -673,6 +680,7 @@ struct ocelot {
 	struct regmap_field		*regfields[REGFIELD_MAX];
 	const u32 *const		*map;
 	const struct ocelot_stat_layout	*stats_layout;
+	struct list_head		stats_regions;
 	unsigned int			num_stats;
 
 	u32				pool_size[OCELOT_SB_NUM][OCELOT_SB_POOL_NUM];
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats
  2022-02-13 19:12 ` [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats Colin Foster
@ 2022-02-14  9:51   ` Vladimir Oltean
  0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Oltean @ 2022-02-14  9:51 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Jakub Kicinski, David S. Miller, UNGLinuxDriver@microchip.com,
	Alexandre Belloni, Claudiu Manoil

On Sun, Feb 13, 2022 at 11:12:54AM -0800, Colin Foster wrote:
> Create and utilize bulk regmap reads instead of single access for gathering
> stats. The background reading of statistics happens frequently, and over
> a few contiguous memory regions.
> 
> High speed PCIe buses and MMIO access will probably see negligible
> performance increase. Lower speed buses like SPI and I2C could see
> significant performance increase, since the bus configuration and register
> access times account for a large percentage of data transfer time.
> 
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics
  2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
                   ` (3 preceding siblings ...)
  2022-02-13 19:12 ` [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats Colin Foster
@ 2022-02-14 13:40 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-14 13:40 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-kernel, netdev, kuba, davem, UNGLinuxDriver,
	alexandre.belloni, claudiu.manoil, vladimir.oltean

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Sun, 13 Feb 2022 11:12:50 -0800 you wrote:
> Ocelot loops over memory regions to gather stats on different ports.
> These regions are mostly continuous, and are ordered. This patch set
> uses that information to break the stats reads into regions that can get
> read in bulk.
> 
> The motiviation is for general cleanup, but also for SPI. Performing two
> back-to-back reads on a SPI bus require toggling the CS line, holding,
> re-toggling the CS line, sending 3 address bytes, sending N padding
> bytes, then actually performing the read. Bulk reads could reduce almost
> all of that overhead, but require that the reads are performed via
> regmap_bulk_read.
> 
> [...]

Here is the summary with links:
  - [v7,net-next,1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool
    https://git.kernel.org/netdev/net-next/c/e27d785e60b6
  - [v7,net-next,2/4] net: ocelot: align macros for consistency
    https://git.kernel.org/netdev/net-next/c/65c53595bc2a
  - [v7,net-next,3/4] net: mscc: ocelot: add ability to perform bulk reads
    https://git.kernel.org/netdev/net-next/c/40f3a5c81555
  - [v7,net-next,4/4] net: mscc: ocelot: use bulk reads for stats
    https://git.kernel.org/netdev/net-next/c/d87b1c08f38a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-02-14 13:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-13 19:12 [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics Colin Foster
2022-02-13 19:12 ` [PATCH v7 net-next 1/4] net: mscc: ocelot: remove unnecessary stat reading from ethtool Colin Foster
2022-02-13 19:12 ` [PATCH v7 net-next 2/4] net: ocelot: align macros for consistency Colin Foster
2022-02-13 19:12 ` [PATCH v7 net-next 3/4] net: mscc: ocelot: add ability to perform bulk reads Colin Foster
2022-02-13 19:12 ` [PATCH v7 net-next 4/4] net: mscc: ocelot: use bulk reads for stats Colin Foster
2022-02-14  9:51   ` Vladimir Oltean
2022-02-14 13:40 ` [PATCH v7 net-next 0/4] use bulk reads for ocelot statistics patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).