[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2839189-70f6-cbd7-aa61-dbf10041fce3@mellanox.com>
Date: Wed, 28 Mar 2018 23:51:58 +0300
From: Tal Gilboa <talgi@...lanox.com>
To: Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, jaedon.shin@...il.com, pgynther@...gle.com,
opendmb@...il.com, Michael Chan <michael.chan@...adcom.com>,
gospo@...adcom.com, saeedm@...lanox.com
Subject: Re: [PATCH net-next 1/3] net: systemport: Remove adaptive TX
coalescing
On 3/27/2018 10:47 PM, Florian Fainelli wrote:
> Adaptive TX coalescing is not currently giving us any advantages and
> ends up making the CPU spin more frequently until TX completion. Deny
> and disable adaptive TX coalescing for now and rely on static
> configuration, we can always add it back later.
>
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 61 ++++--------------------------
> drivers/net/ethernet/broadcom/bcmsysport.h | 1 -
> 2 files changed, 8 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 4e26f606a7f2..1e52bb7d822e 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -15,7 +15,6 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> -#include <linux/net_dim.h>
> #include <linux/etherdevice.h>
> #include <linux/platform_device.h>
> #include <linux/of.h>
> @@ -588,7 +587,8 @@ static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
> rdma_writel(priv, reg, RDMA_MBDONE_INTR);
> }
>
> -static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
> +static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring,
> + struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = ring->priv;
> u32 reg;
> @@ -596,8 +596,8 @@ static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
> reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(ring->index));
> reg &= ~(RING_INTR_THRESH_MASK |
> RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
> - reg |= ring->dim.coal_pkts;
> - reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192) <<
> + reg |= ec->tx_max_coalesced_frames;
> + reg |= DIV_ROUND_UP(ec->tx_coalesce_usecs * 1000, 8192) <<
> RING_TIMEOUT_SHIFT;
> tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(ring->index));
> }
> @@ -606,18 +606,12 @@ static int bcm_sysport_get_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> - struct bcm_sysport_tx_ring *ring;
> - unsigned int i;
> u32 reg;
>
> reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(0));
>
> ec->tx_coalesce_usecs = (reg >> RING_TIMEOUT_SHIFT) * 8192 / 1000;
> ec->tx_max_coalesced_frames = reg & RING_INTR_THRESH_MASK;
> - for (i = 0; i < dev->num_tx_queues; i++) {
> - ring = &priv->tx_rings[i];
> - ec->use_adaptive_tx_coalesce |= ring->dim.use_dim;
> - }
>
> reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>
> @@ -632,7 +626,6 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> - struct bcm_sysport_tx_ring *ring;
> unsigned int i;
>
> /* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -646,20 +639,12 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
> return -EINVAL;
>
> if ((ec->tx_coalesce_usecs == 0 && ec->tx_max_coalesced_frames == 0) ||
> - (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0))
> + (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0) ||
> + ec->use_adaptive_tx_coalesce)
> return -EINVAL;
>
> - for (i = 0; i < dev->num_tx_queues; i++) {
> - ring = &priv->tx_rings[i];
> - ring->dim.coal_pkts = ec->tx_max_coalesced_frames;
> - ring->dim.coal_usecs = ec->tx_coalesce_usecs;
> - if (!ec->use_adaptive_tx_coalesce && ring->dim.use_dim) {
> - ring->dim.coal_pkts = 1;
> - ring->dim.coal_usecs = 0;
> - }
> - ring->dim.use_dim = ec->use_adaptive_tx_coalesce;
> - bcm_sysport_set_tx_coalesce(ring);
> - }
> + for (i = 0; i < dev->num_tx_queues; i++)
> + bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>
> priv->dim.coal_usecs = ec->rx_coalesce_usecs;
> priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
> @@ -940,8 +925,6 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
> ring->packets += pkts_compl;
> ring->bytes += bytes_compl;
> u64_stats_update_end(&priv->syncp);
> - ring->dim.packets = pkts_compl;
> - ring->dim.bytes = bytes_compl;
>
> ring->c_index = c_index;
>
> @@ -987,7 +970,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
> {
> struct bcm_sysport_tx_ring *ring =
> container_of(napi, struct bcm_sysport_tx_ring, napi);
> - struct net_dim_sample dim_sample;
> unsigned int work_done = 0;
>
> work_done = bcm_sysport_tx_reclaim(ring->priv, ring);
> @@ -1004,12 +986,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
> return 0;
> }
>
> - if (ring->dim.use_dim) {
> - net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
> - ring->dim.bytes, &dim_sample);
> - net_dim(&ring->dim.dim, dim_sample);
> - }
> -
> return budget;
> }
>
> @@ -1089,23 +1065,6 @@ static void bcm_sysport_dim_work(struct work_struct *work)
> dim->state = NET_DIM_START_MEASURE;
> }
>
> -static void bcm_sysport_dim_tx_work(struct work_struct *work)
> -{
> - struct net_dim *dim = container_of(work, struct net_dim, work);
> - struct bcm_sysport_net_dim *ndim =
> - container_of(dim, struct bcm_sysport_net_dim, dim);
> - struct bcm_sysport_tx_ring *ring =
> - container_of(ndim, struct bcm_sysport_tx_ring, dim);
> - struct net_dim_cq_moder cur_profile =
> - net_dim_get_profile(dim->mode, dim->profile_ix);
> -
> - ring->dim.coal_usecs = cur_profile.usec;
> - ring->dim.coal_pkts = cur_profile.pkts;
> -
> - bcm_sysport_set_tx_coalesce(ring);
> - dim->state = NET_DIM_START_MEASURE;
> -}
> -
> /* RX and misc interrupt routine */
> static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
> {
> @@ -1152,7 +1111,6 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
> continue;
>
> txr = &priv->tx_rings[ring];
> - txr->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&txr->napi))) {
> intrl2_0_mask_set(priv, ring_bit);
> @@ -1185,7 +1143,6 @@ static irqreturn_t bcm_sysport_tx_isr(int irq, void *dev_id)
> continue;
>
> txr = &priv->tx_rings[ring];
> - txr->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&txr->napi))) {
> intrl2_1_mask_set(priv, BIT(ring));
> @@ -1551,7 +1508,6 @@ static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
> reg |= (1 << index);
> tdma_writel(priv, reg, TDMA_TIER1_ARB_0_QUEUE_EN);
>
> - bcm_sysport_init_dim(&ring->dim, bcm_sysport_dim_tx_work);
> napi_enable(&ring->napi);
>
> netif_dbg(priv, hw, priv->netdev,
> @@ -1582,7 +1538,6 @@ static void bcm_sysport_fini_tx_ring(struct bcm_sysport_priv *priv,
> return;
>
> napi_disable(&ring->napi);
> - cancel_work_sync(&ring->dim.dim.work);
> netif_napi_del(&ring->napi);
>
> bcm_sysport_tx_clean(priv, ring);
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index e1c97d4a82b4..57e18ef8f206 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -723,7 +723,6 @@ struct bcm_sysport_tx_ring {
> struct bcm_sysport_priv *priv; /* private context backpointer */
> unsigned long packets; /* packets statistics */
> unsigned long bytes; /* bytes statistics */
> - struct bcm_sysport_net_dim dim; /* Net DIM context */
> unsigned int switch_queue; /* switch port queue number */
> unsigned int switch_port; /* switch port queue number */
> bool inspect; /* inspect switch port and queue */
>
Reviewed-by: Tal Gilboa <talgi@...lanox.com>
Powered by blists - more mailing lists