* Re: [PATCH net 00/18] Update ENA driver to version 1.1.2
From: David Miller @ 2016-11-20 15:24 UTC (permalink / raw)
To: netanel
Cc: linux-kernel, netdev, dwmw, zorik, alex, saeed, msw, aliguori,
nafea
In-Reply-To: <1479631547-29354-1-git-send-email-netanel@annapurnalabs.com>
From: Netanel Belgazal <netanel@annapurnalabs.com>
Date: Sun, 20 Nov 2016 10:45:29 +0200
> Update Amazon's Elastic Network Adapter (ENA) driver version from 1.0.2 to 1.1.2
This is insufficient.
You must explain what this patch series is doing, how it is doing it,
and why it is doing it that way.
This is the message that people will look at to learn what is
contained in this series of patches, and they might be looking for
keywords or explanations as to why a decision was made to add a
feature, turn a feature off, or make some other important high level
change to the driver.
^ permalink raw reply
* Re: [PATCH net-next] mlx4: avoid unnecessary dirtying of critical fields
From: Tariq Toukan @ 2016-11-20 15:14 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Tariq Toukan
In-Reply-To: <1479500133.8455.295.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
Thanks for your patch.
On 18/11/2016 10:15 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> While stressing a 40Gbit mlx4 NIC with busy polling, I found false
> sharing in mlx4 driver that can be easily avoided.
>
> This patch brings an additional 7 % performance improvement in UDP_RR
> workload.
>
> 1) If we received no frame during one mlx4_en_process_rx_cq()
> invocation, no need to call mlx4_cq_set_ci() and/or dirty ring->cons
>
> 2) Do not refill rx buffers if we have plenty of them.
> This avoids false sharing and allows some bulk/batch optimizations.
> Page allocator and its locks will thank us.
>
> Finally, mlx4_en_poll_rx_cq() should not return 0 if it determined
> cpu handling NIC IRQ should be changed. We should return budget-1
> instead, to not fool net_rx_action() and its netdev_budget.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 51 +++++++++++--------
> 1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 22f08f9ef464..2112494ff43b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -688,18 +688,23 @@ static void validate_loopback(struct mlx4_en_priv *priv, struct sk_buff *skb)
> dev_kfree_skb_any(skb);
> }
>
> -static void mlx4_en_refill_rx_buffers(struct mlx4_en_priv *priv,
> - struct mlx4_en_rx_ring *ring)
> +static bool mlx4_en_refill_rx_buffers(struct mlx4_en_priv *priv,
> + struct mlx4_en_rx_ring *ring)
> {
> - int index = ring->prod & ring->size_mask;
> + u32 missing = ring->actual_size - (ring->prod - ring->cons);
>
> - while ((u32) (ring->prod - ring->cons) < ring->actual_size) {
> - if (mlx4_en_prepare_rx_desc(priv, ring, index,
> + /* Try to batch allocations, but not too much. */
> + if (missing < 8)
> + return false;
> + do {
> + if (mlx4_en_prepare_rx_desc(priv, ring,
> + ring->prod & ring->size_mask,
> GFP_ATOMIC | __GFP_COLD))
> break;
> ring->prod++;
> - index = ring->prod & ring->size_mask;
> - }
> + } while (--missing);
> +
> + return true;
> }
>
> /* When hardware doesn't strip the vlan, we need to calculate the checksum
> @@ -1081,15 +1086,20 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>
> out:
> rcu_read_unlock();
> - if (doorbell_pending)
> - mlx4_en_xmit_doorbell(priv->tx_ring[TX_XDP][cq->ring]);
> -
> - AVG_PERF_COUNTER(priv->pstats.rx_coal_avg, polled);
> - mlx4_cq_set_ci(&cq->mcq);
> - wmb(); /* ensure HW sees CQ consumer before we post new buffers */
> - ring->cons = cq->mcq.cons_index;
> - mlx4_en_refill_rx_buffers(priv, ring);
> - mlx4_en_update_rx_prod_db(ring);
> +
> + if (polled) {
> + if (doorbell_pending)
> + mlx4_en_xmit_doorbell(priv->tx_ring[TX_XDP][cq->ring]);
> +
> + AVG_PERF_COUNTER(priv->pstats.rx_coal_avg, polled);
Keep this perf stats update out of the if block.
> + mlx4_cq_set_ci(&cq->mcq);
> + wmb(); /* ensure HW sees CQ consumer before we post new buffers */
> + ring->cons = cq->mcq.cons_index;
> + }
> +
> + if (mlx4_en_refill_rx_buffers(priv, ring))
> + mlx4_en_update_rx_prod_db(ring);
> +
> return polled;
> }
>
> @@ -1131,10 +1141,13 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
> return budget;
>
> /* Current cpu is not according to smp_irq_affinity -
> - * probably affinity changed. need to stop this NAPI
> - * poll, and restart it on the right CPU
> + * probably affinity changed. Need to stop this NAPI
> + * poll, and restart it on the right CPU.
> + * Try to avoid returning a too small value (like 0),
> + * to not fool net_rx_action() and its netdev_budget
> */
> - done = 0;
> + if (done)
> + done--;
> }
> /* Done for now */
> if (napi_complete_done(napi, done))
>
>
It looks good to me, just the one comment aforementioned.
Regards,
Tariq
^ permalink raw reply
* Re: [PATCH] [v2] net: phy: phy drivers should not set SUPPORTED_[Asym_]Pause
From: Timur Tabi @ 2016-11-20 16:08 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Timur Tabi, David Miller, jon.mason, netdev, Andrew Lunn
In-Reply-To: <cf18464c-e225-af5d-9251-f2553b93b9ff@gmail.com>
On Mon, Nov 14, 2016 at 12:35 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> if (!(drv->features & (SUPPORTED_Pause | SUPPORTED_AsymPause))
> phydev->supported |= SUPPORTED_Pause | SUPPORTED_AsymPause;
How about, if either bit is set in drv->features, then assume the phy
driver really knows what it's doing, and just copy those bits to
phydev->supported?
if (drv->features & (SUPPORTED_Pause | SUPPORTED_AsymPause)) {
phydev->supported &= ~(SUPPORTED_Pause | SUPPORTED_AsymPause);
phydev->supported |= drv->features & (SUPPORTED_Pause |
SUPPORTED_AsymPause);
} else
phydev->supported |= SUPPORTED_Pause | SUPPORTED_AsymPause;
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH net-next] mlx4: avoid unnecessary dirtying of critical fields
From: Eric Dumazet @ 2016-11-20 17:15 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Miller, netdev, Tariq Toukan
In-Reply-To: <62f9c9ee-d710-7493-1094-e8ffe8aaa1ca@gmail.com>
On Sun, 2016-11-20 at 17:14 +0200, Tariq Toukan wrote:
> Hi Eric,
>
> Thanks for your patch.
>
> On 18/11/2016 10:15 PM, Eric Dumazet wrote:
> > +
> > + AVG_PERF_COUNTER(priv->pstats.rx_coal_avg, polled);
> Keep this perf stats update out of the if block.
This perf stat would be useless then with busy polling.
And this would be the the only source of false sharing in the driver.
Not that I particularly care, since AVG_PERF_COUNTER can not be enabled
without modifying / recompiling the driver.
^ permalink raw reply
* [PATCH v2 net-next] mlx4: avoid unnecessary dirtying of critical fields
From: Eric Dumazet @ 2016-11-20 17:24 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Miller, netdev, Tariq Toukan
In-Reply-To: <1479662102.8455.361.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
While stressing a 40Gbit mlx4 NIC with busy polling, I found false
sharing in mlx4 driver that can be easily avoided.
This patch brings an additional 7 % performance improvement in UDP_RR
workload.
1) If we received no frame during one mlx4_en_process_rx_cq()
invocation, no need to call mlx4_cq_set_ci() and/or dirty ring->cons
2) Do not refill rx buffers if we have plenty of them.
This avoids false sharing and allows some bulk/batch optimizations.
Page allocator and its locks will thank us.
Finally, mlx4_en_poll_rx_cq() should not return 0 if it determined
cpu handling NIC IRQ should be changed. We should return budget-1
instead, to not fool net_rx_action() and its netdev_budget.
v2: keep AVG_PERF_COUNTER(... polled) even if polled is 0
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 47 ++++++++++++-------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 22f08f9ef4645869359783823127c0432fc7a591..6562f78b07f4370b5c1ea2c5e3a4221d7ebaeba8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -688,18 +688,23 @@ static void validate_loopback(struct mlx4_en_priv *priv, struct sk_buff *skb)
dev_kfree_skb_any(skb);
}
-static void mlx4_en_refill_rx_buffers(struct mlx4_en_priv *priv,
- struct mlx4_en_rx_ring *ring)
+static bool mlx4_en_refill_rx_buffers(struct mlx4_en_priv *priv,
+ struct mlx4_en_rx_ring *ring)
{
- int index = ring->prod & ring->size_mask;
+ u32 missing = ring->actual_size - (ring->prod - ring->cons);
- while ((u32) (ring->prod - ring->cons) < ring->actual_size) {
- if (mlx4_en_prepare_rx_desc(priv, ring, index,
+ /* Try to batch allocations, but not too much. */
+ if (missing < 8)
+ return false;
+ do {
+ if (mlx4_en_prepare_rx_desc(priv, ring,
+ ring->prod & ring->size_mask,
GFP_ATOMIC | __GFP_COLD))
break;
ring->prod++;
- index = ring->prod & ring->size_mask;
- }
+ } while (--missing);
+
+ return true;
}
/* When hardware doesn't strip the vlan, we need to calculate the checksum
@@ -1081,15 +1086,20 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
out:
rcu_read_unlock();
- if (doorbell_pending)
- mlx4_en_xmit_doorbell(priv->tx_ring[TX_XDP][cq->ring]);
+ if (polled) {
+ if (doorbell_pending)
+ mlx4_en_xmit_doorbell(priv->tx_ring[TX_XDP][cq->ring]);
+
+ mlx4_cq_set_ci(&cq->mcq);
+ wmb(); /* ensure HW sees CQ consumer before we post new buffers */
+ ring->cons = cq->mcq.cons_index;
+ }
AVG_PERF_COUNTER(priv->pstats.rx_coal_avg, polled);
- mlx4_cq_set_ci(&cq->mcq);
- wmb(); /* ensure HW sees CQ consumer before we post new buffers */
- ring->cons = cq->mcq.cons_index;
- mlx4_en_refill_rx_buffers(priv, ring);
- mlx4_en_update_rx_prod_db(ring);
+
+ if (mlx4_en_refill_rx_buffers(priv, ring))
+ mlx4_en_update_rx_prod_db(ring);
+
return polled;
}
@@ -1131,10 +1141,13 @@ int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget)
return budget;
/* Current cpu is not according to smp_irq_affinity -
- * probably affinity changed. need to stop this NAPI
- * poll, and restart it on the right CPU
+ * probably affinity changed. Need to stop this NAPI
+ * poll, and restart it on the right CPU.
+ * Try to avoid returning a too small value (like 0),
+ * to not fool net_rx_action() and its netdev_budget
*/
- done = 0;
+ if (done)
+ done--;
}
/* Done for now */
if (napi_complete_done(napi, done))
^ permalink raw reply related
* Re: [PATCH v2 net-next] mlx4: avoid unnecessary dirtying of critical fields
From: Eric Dumazet @ 2016-11-20 17:26 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Miller, netdev, Tariq Toukan
In-Reply-To: <1479662676.8455.364.camel@edumazet-glaptop3.roam.corp.google.com>
On Sun, 2016-11-20 at 09:24 -0800, Eric Dumazet wrote:
> /* Current cpu is not according to smp_irq_affinity -
> - * probably affinity changed. need to stop this NAPI
> - * poll, and restart it on the right CPU
> + * probably affinity changed. Need to stop this NAPI
> + * poll, and restart it on the right CPU.
> + * Try to avoid returning a too small value (like 0),
> + * to not fool net_rx_action() and its netdev_budget
> */
> - done = 0;
> + if (done)
> + done--;
Note : This could have been a net candidate, but bug is minor and I
prefer to avoid a merge conflict, since net-next has the additional if
around the napi_complete_done() call.
> }
> /* Done for now */
> if (napi_complete_done(napi, done))
>
^ permalink raw reply
* Re: [PATCH] [v2] net: phy: phy drivers should not set SUPPORTED_[Asym_]Pause
From: Florian Fainelli @ 2016-11-20 17:32 UTC (permalink / raw)
To: Timur Tabi; +Cc: David Miller, jon.mason, netdev, Andrew Lunn
In-Reply-To: <CAOZdJXX5j9EVB8UtJK7u4-q4yFFtPBdWnWhL5aO_gGvL7fLTUg@mail.gmail.com>
On 11/20/2016 08:08 AM, Timur Tabi wrote:
> On Mon, Nov 14, 2016 at 12:35 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> if (!(drv->features & (SUPPORTED_Pause | SUPPORTED_AsymPause))
>> phydev->supported |= SUPPORTED_Pause | SUPPORTED_AsymPause;
>
> How about, if either bit is set in drv->features, then assume the phy
> driver really knows what it's doing, and just copy those bits to
> phydev->supported?
Yes, that seems completely reasonable to me. Thanks!
>
> if (drv->features & (SUPPORTED_Pause | SUPPORTED_AsymPause)) {
> phydev->supported &= ~(SUPPORTED_Pause | SUPPORTED_AsymPause);
> phydev->supported |= drv->features & (SUPPORTED_Pause |
> SUPPORTED_AsymPause);
> } else
> phydev->supported |= SUPPORTED_Pause | SUPPORTED_AsymPause;
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/1] driver: macvlan: Remove duplicated IFF_UP condition check in macvlan_forward_source_one
From: Eric Dumazet @ 2016-11-20 17:37 UTC (permalink / raw)
To: fgao; +Cc: davem, kaber, netdev, gfree.wind
In-Reply-To: <1479640879-30733-1-git-send-email-fgao@ikuai8.com>
On Sun, 2016-11-20 at 19:21 +0800, fgao@ikuai8.com wrote:
> From: Gao Feng <gfree.wind@gmail.com>
>
> The condition check "dev->flags & IFF_UP" is duplicated in
> macvlan_forward_source_one, because its caller macvlan_forward_source
> has already checked this flag.
>
> Signed-off-by: Gao Feng <gfree.wind@gmail.com>
> ---
> drivers/net/macvlan.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 13b7e0b..95a5ffc 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -375,9 +375,6 @@ static void macvlan_forward_source_one(struct sk_buff *skb,
> int ret;
>
> dev = vlan->dev;
> - if (unlikely(!(dev->flags & IFF_UP)))
> - return;
> -
> nskb = skb_clone(skb, GFP_ATOMIC);
> if (!nskb)
> return;
If you don't mind, I would rather remove the test in the caller and
leave it here. It is likely compiler inlines the function anyway, so
there is no performance change.
^ permalink raw reply
* [PATCH v2] ethernet: stmmac: make DWMAC_STM32 depend on it's associated SoC
From: Peter Robinson @ 2016-11-20 17:22 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Maxime Coquelin, netdev
Cc: Peter Robinson
There's not much point, except compile test, enabling the stmmac
platform drivers unless the STM32 SoC is enabled. It's not
useful without it.
Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 3818c5e..4b78168 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -107,7 +107,7 @@ config DWMAC_STI
config DWMAC_STM32
tristate "STM32 DWMAC support"
default ARCH_STM32
- depends on OF && HAS_IOMEM
+ depends on OF && HAS_IOMEM && (ARCH_STM32 || COMPILE_TEST)
select MFD_SYSCON
---help---
Support for ethernet controller on STM32 SOCs.
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net 00/18] Update ENA driver to version 1.1.2
From: Netanel Belgazal @ 2016-11-20 18:53 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, netdev, dwmw, zorik, alex, saeed, msw, aliguori,
nafea
In-Reply-To: <20161120.102422.733701532971988272.davem@davemloft.net>
Hi David,
Sorry for not being clear on my first patch set.
Those changes introduce some bug fixes, new features and some cleanups that matching the driver to the upstream standard.
Bug Fixes:
*net/ena: remove RFS support from device feature list
*net/ena: fix queues number calculation
*net/ena: fix ethtool RSS flow configuration
*net/ena: refactor ena_get_stats64 to be atomic context safe
*net/ena: fix potential access to freed memory during device reset
*net/ena: remove redundant logic in napi callback for busy poll mode
*net/ena: fix error handling when probe fails
*net/ena: fix NULL dereference when removing the driver after device
reset faild
*net/ena: change driver's default timeouts and increase driver version
New Features:
*net/ena: add hardware hints capability to the driver
*net/ena: change condition for host attribute configuration
*net/ena: add IPv6 extended protocols to ena_admin_flow_hash_proto
*net/ena: remove affinity hint from the driver
Clean ups:
*net/ena: use napi_schedule_irqoff when possible
*net/ena: reduce the severity of ena printouts
*net/ena: change sizeof() argument to be the type pointer
*net/ena: use READ_ONCE to access completion descriptors
*net/ena: refactor skb allocation
I'll add the above description in V2 (I would like to wait a couple of days to collect more feedback about those patches).
Regards,
Netanel
On 11/20/2016 05:24 PM, David Miller wrote:
> From: Netanel Belgazal <netanel@annapurnalabs.com>
> Date: Sun, 20 Nov 2016 10:45:29 +0200
>
>> Update Amazon's Elastic Network Adapter (ENA) driver version from 1.0.2 to 1.1.2
> This is insufficient.
>
> You must explain what this patch series is doing, how it is doing it,
> and why it is doing it that way.
>
> This is the message that people will look at to learn what is
> contained in this series of patches, and they might be looking for
> keywords or explanations as to why a decision was made to add a
> feature, turn a feature off, or make some other important high level
> change to the driver.
^ permalink raw reply
* Re: [PATCH net 07/18] net/ena: refactor ena_get_stats64 to be atomic context safe
From: Netanel Belgazal @ 2016-11-20 18:59 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, linux-kernel, davem, netdev, dwmw, zorik, alex, saeed,
msw, aliguori, nafea
In-Reply-To: <201611201710.x7wUdghv%fengguang.wu@intel.com>
This warning isn't a real issue since adapter->num_queues can't be zero.
Anyway, I'll use the adapter's syncp in V2 to avoid this warning.
On 11/20/2016 12:09 PM, kbuild test robot wrote:
> Hi Netanel,
>
> [auto build test WARNING on net/master]
>
> url: https://github.com/0day-ci/linux/commits/Netanel-Belgazal/Update-ENA-driver-to-version-1-1-2/20161120-165649
> config: i386-randconfig-x009-201647 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> All warnings (new ones prefixed by >>):
>
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_napi_enable_all
> Cyclomatic Complexity 1 include/linux/dma-mapping.h:dma_map_single_attrs
> Cyclomatic Complexity 1 include/linux/dynamic_queue_limits.h:dql_queued
> Cyclomatic Complexity 3 include/linux/netdevice.h:netdev_tx_sent_queue
> Cyclomatic Complexity 1 include/linux/netdevice.h:dev_kfree_skb_any
> Cyclomatic Complexity 1 include/linux/netdevice.h:netdev_tx_reset_queue
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_free_tx_bufs
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_free_all_tx_bufs
> Cyclomatic Complexity 3 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_free_rx_page
> Cyclomatic Complexity 3 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_free_rx_bufs
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_free_all_rx_bufs
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_down
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_close
> Cyclomatic Complexity 3 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_update_on_link_change
> Cyclomatic Complexity 2 include/linux/netdevice.h:napi_schedule_irqoff
> Cyclomatic Complexity 1 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_intr_msix_io
> Cyclomatic Complexity 4 include/linux/cpumask.h:cpumask_check
> Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_set_cpu
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_io_intr
> Cyclomatic Complexity 1 include/linux/interrupt.h:request_irq
> Cyclomatic Complexity 9 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_request_io_irq
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_request_mgmnt_irq
> Cyclomatic Complexity 8 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_tx_resources
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_all_tx_resources
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_rx_resources
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_all_rx_resources
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_create_io_tx_queue
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_create_all_io_tx_queues
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_create_io_rx_queue
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_create_all_io_rx_queues
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_init_napi
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_eth_com.h:ena_com_update_dev_comp_head
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_eth_com.h:ena_com_write_sq_doorbell
> Cyclomatic Complexity 4 include/linux/netdevice.h:netdev_tx_completed_queue
> Cyclomatic Complexity 14 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_clean_tx_irq
> Cyclomatic Complexity 2 include/linux/netdevice.h:netif_tx_unlock
> Cyclomatic Complexity 1 include/linux/skbuff.h:__netdev_alloc_skb_ip_align
> Cyclomatic Complexity 1 include/linux/skbuff.h:netdev_alloc_skb_ip_align
> Cyclomatic Complexity 16 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_rx_skb
> Cyclomatic Complexity 1 include/linux/gfp.h:__alloc_pages
> Cyclomatic Complexity 1 include/linux/gfp.h:__alloc_pages_node
> Cyclomatic Complexity 2 include/linux/gfp.h:alloc_pages_node
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_alloc_rx_page
> Cyclomatic Complexity 8 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_refill_rx_bufs
> Cyclomatic Complexity 11 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_clean_rx_irq
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_refill_all_rx_bufs
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_update_ring_numa_node
> Cyclomatic Complexity 8 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_change_mtu
> Cyclomatic Complexity 2 include/linux/netdevice.h:napi_schedule
> Cyclomatic Complexity 1 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_device_io_resume
> Cyclomatic Complexity 1 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_device_io_suspend
> Cyclomatic Complexity 2 include/linux/seqlock.h:seqcount_lockdep_reader_access
> Cyclomatic Complexity 1 include/linux/seqlock.h:read_seqcount_begin
> Cyclomatic Complexity 1 include/linux/u64_stats_sync.h:__u64_stats_fetch_begin
> Cyclomatic Complexity 1 include/linux/u64_stats_sync.h:u64_stats_fetch_begin_irq
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_get_stats64
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_notification
> Cyclomatic Complexity 2 include/linux/skbuff.h:__skb_linearize
> Cyclomatic Complexity 2 include/linux/skbuff.h:skb_linearize
> Cyclomatic Complexity 5 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_check_and_linearize_skb
> Cyclomatic Complexity 3 include/linux/skbuff.h:sw_tx_timestamp
> Cyclomatic Complexity 1 include/linux/skbuff.h:skb_tx_timestamp
> Cyclomatic Complexity 22 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_start_xmit
> Cyclomatic Complexity 1 arch/x86/include/asm/io.h:ioremap
> Cyclomatic Complexity 4 include/linux/dma-mapping.h:dma_set_mask
> Cyclomatic Complexity 1 include/linux/pci-dma-compat.h:pci_set_dma_mask
> Cyclomatic Complexity 2 include/linux/dma-mapping.h:dma_set_coherent_mask
> Cyclomatic Complexity 1 include/linux/pci-dma-compat.h:pci_set_consistent_dma_mask
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_config_host_info
> Cyclomatic Complexity 12 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_device_init
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_calc_io_queue_num
> Cyclomatic Complexity 69 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_calc_queue_size
> Cyclomatic Complexity 1 include/linux/etherdevice.h:eth_random_addr
> Cyclomatic Complexity 1 include/linux/etherdevice.h:eth_hw_addr_random
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_set_conf_feat_params
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_init_io_rings
> Cyclomatic Complexity 1 include/linux/cpu_rmap.h:alloc_irq_cpu_rmap
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_init_rx_cpu_rmap
> Cyclomatic Complexity 14 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_enable_msix
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_intr_msix_mgmnt
> Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_first
> Cyclomatic Complexity 1 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_setup_mgmnt_intr
> Cyclomatic Complexity 3 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_enable_msix_and_set_admin_interrupts
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_rss_init_default
> Cyclomatic Complexity 7 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_rss_configure
> Cyclomatic Complexity 3 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_up_complete
> Cyclomatic Complexity 9 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_up
> Cyclomatic Complexity 7 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_fw_reset_device
> Cyclomatic Complexity 6 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_open
> Cyclomatic Complexity 8 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_config_debug_area
> Cyclomatic Complexity 21 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_probe
> Cyclomatic Complexity 2 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_init
> Cyclomatic Complexity 1 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_adjust_intr_moderation
> Cyclomatic Complexity 4 drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_io_poll
> In file included from include/linux/linkage.h:4:0,
> from include/linux/kernel.h:6,
> from include/linux/cpumask.h:9,
> from include/linux/cpu_rmap.h:13,
> from drivers/net/ethernet/amazon/ena/ena_netdev.c:36:
> drivers/net/ethernet/amazon/ena/ena_netdev.c: In function 'ena_get_stats64':
>>> include/linux/compiler.h:231:26: warning: 'rx_ring' may be used uninitialized in this function [-Wmaybe-uninitialized]
> case 4: *(__u32 *)res = *(volatile __u32 *)p; break; \
> ^
> drivers/net/ethernet/amazon/ena/ena_netdev.c:2188:19: note: 'rx_ring' was declared here
> struct ena_ring *rx_ring, *tx_ring;
> ^~~~~~~
>
> vim +/rx_ring +231 include/linux/compiler.h
>
> fe8c8a12 Cesar Eduardo Barros 2013-11-25 215 #ifndef OPTIMIZER_HIDE_VAR
> fe8c8a12 Cesar Eduardo Barros 2013-11-25 216 #define OPTIMIZER_HIDE_VAR(var) barrier()
> fe8c8a12 Cesar Eduardo Barros 2013-11-25 217 #endif
> fe8c8a12 Cesar Eduardo Barros 2013-11-25 218
> 6f33d587 Rusty Russell 2012-11-22 219 /* Not-quite-unique ID. */
> 6f33d587 Rusty Russell 2012-11-22 220 #ifndef __UNIQUE_ID
> 6f33d587 Rusty Russell 2012-11-22 221 # define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __LINE__)
> 6f33d587 Rusty Russell 2012-11-22 222 #endif
> 6f33d587 Rusty Russell 2012-11-22 223
> 230fa253 Christian Borntraeger 2014-11-25 224 #include <uapi/linux/types.h>
> 230fa253 Christian Borntraeger 2014-11-25 225
> d976441f Andrey Ryabinin 2015-10-19 226 #define __READ_ONCE_SIZE \
> d976441f Andrey Ryabinin 2015-10-19 227 ({ \
> d976441f Andrey Ryabinin 2015-10-19 228 switch (size) { \
> d976441f Andrey Ryabinin 2015-10-19 229 case 1: *(__u8 *)res = *(volatile __u8 *)p; break; \
> d976441f Andrey Ryabinin 2015-10-19 230 case 2: *(__u16 *)res = *(volatile __u16 *)p; break; \
> d976441f Andrey Ryabinin 2015-10-19 @231 case 4: *(__u32 *)res = *(volatile __u32 *)p; break; \
> d976441f Andrey Ryabinin 2015-10-19 232 case 8: *(__u64 *)res = *(volatile __u64 *)p; break; \
> d976441f Andrey Ryabinin 2015-10-19 233 default: \
> d976441f Andrey Ryabinin 2015-10-19 234 barrier(); \
> d976441f Andrey Ryabinin 2015-10-19 235 __builtin_memcpy((void *)res, (const void *)p, size); \
> d976441f Andrey Ryabinin 2015-10-19 236 barrier(); \
> d976441f Andrey Ryabinin 2015-10-19 237 } \
> d976441f Andrey Ryabinin 2015-10-19 238 })
> d976441f Andrey Ryabinin 2015-10-19 239
>
> :::::: The code at line 231 was first introduced by commit
> :::::: d976441f44bc5d48635d081d277aa76556ffbf8b compiler, atomics, kasan: Provide READ_ONCE_NOCHECK()
>
> :::::: TO: Andrey Ryabinin <aryabinin@virtuozzo.com>
> :::::: CC: Ingo Molnar <mingo@kernel.org>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH v2 net-next 4/6] net: dsa: mv88e6xxx: Fix cleanup on error for g1 interrupt setup
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
On error, remask the interrupts, release all maps, and remove the
domain. This cannot be done using the mv88e6xxx_g1_irq_free() because
some of these actions are not idempotent.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8fcef7e0d3ba..614b2f68d401 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -431,8 +431,8 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
{
- int err, irq;
- u16 reg;
+ int err, irq, virq;
+ u16 reg, mask;
chip->g1_irq.nirqs = chip->info->g1_irqs;
chip->g1_irq.domain = irq_domain_add_simple(
@@ -447,32 +447,41 @@ static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
chip->g1_irq.chip = mv88e6xxx_g1_irq_chip;
chip->g1_irq.masked = ~0;
- err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, ®);
+ err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &mask);
if (err)
- goto out;
+ goto out_mapping;
- reg &= ~GENMASK(chip->g1_irq.nirqs, 0);
+ mask &= ~GENMASK(chip->g1_irq.nirqs, 0);
- err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, reg);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask);
if (err)
- goto out;
+ goto out_disable;
/* Reading the interrupt status clears (most of) them */
err = mv88e6xxx_g1_read(chip, GLOBAL_STATUS, ®);
if (err)
- goto out;
+ goto out_disable;
err = request_threaded_irq(chip->irq, NULL,
mv88e6xxx_g1_irq_thread_fn,
IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
dev_name(chip->dev), chip);
if (err)
- goto out;
+ goto out_disable;
return 0;
-out:
- mv88e6xxx_g1_irq_free(chip);
+out_disable:
+ mask |= GENMASK(chip->g1_irq.nirqs, 0);
+ mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask);
+
+out_mapping:
+ for (irq = 0; irq < 16; irq++) {
+ virq = irq_find_mapping(chip->g1_irq.domain, irq);
+ irq_dispose_mapping(virq);
+ }
+
+ irq_domain_remove(chip->g1_irq.domain);
return err;
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 net-next 5/6] net: dsa: mv88e6xxx: Fix releasing for the global2 interrupts
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
It is not possible to use devm_request_threaded_irq() because we have
two stacked interrupt controllers in one device. The lower interrupt
controller cannot be removed until the upper is fully removed. This
happens too late with the devm API, resulting in error messages about
removing a domain while there is still an active interrupt. Swap to
using request_threaded_irq() and manage the release of the interrupt
manually.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Fix device_irq typo spotted by David Miller.
---
drivers/net/dsa/mv88e6xxx/global2.c | 28 ++++++++++++++++++----------
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 1a0b13521d13..536a27c9735f 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -507,6 +507,9 @@ void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip)
{
int irq, virq;
+ free_irq(chip->device_irq, chip);
+ irq_dispose_mapping(chip->device_irq);
+
for (irq = 0; irq < 16; irq++) {
virq = irq_find_mapping(chip->g2_irq.domain, irq);
irq_dispose_mapping(virq);
@@ -517,8 +520,7 @@ void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip)
int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
{
- int device_irq;
- int err, irq;
+ int err, irq, virq;
if (!chip->dev->of_node)
return -EINVAL;
@@ -534,22 +536,28 @@ int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
chip->g2_irq.chip = mv88e6xxx_g2_irq_chip;
chip->g2_irq.masked = ~0;
- device_irq = irq_find_mapping(chip->g1_irq.domain,
- GLOBAL_STATUS_IRQ_DEVICE);
- if (device_irq < 0) {
- err = device_irq;
+ chip->device_irq = irq_find_mapping(chip->g1_irq.domain,
+ GLOBAL_STATUS_IRQ_DEVICE);
+ if (chip->device_irq < 0) {
+ err = chip->device_irq;
goto out;
}
- err = devm_request_threaded_irq(chip->dev, device_irq, NULL,
- mv88e6xxx_g2_irq_thread_fn,
- IRQF_ONESHOT, "mv88e6xxx-g1", chip);
+ err = request_threaded_irq(chip->device_irq, NULL,
+ mv88e6xxx_g2_irq_thread_fn,
+ IRQF_ONESHOT, "mv88e6xxx-g1", chip);
if (err)
goto out;
return 0;
+
out:
- mv88e6xxx_g2_irq_free(chip);
+ for (irq = 0; irq < 16; irq++) {
+ virq = irq_find_mapping(chip->g2_irq.domain, irq);
+ irq_dispose_mapping(virq);
+ }
+
+ irq_domain_remove(chip->g2_irq.domain);
return err;
}
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 929613021eff..a3869504f881 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -714,6 +714,7 @@ struct mv88e6xxx_chip {
struct mv88e6xxx_irq g1_irq;
struct mv88e6xxx_irq g2_irq;
int irq;
+ int device_irq;
};
struct mv88e6xxx_bus_ops {
--
2.10.2
^ permalink raw reply related
* [PATCH v2 net-next 1/6] net: dsa: mv88e6xxx: Fix typos when removing g1 interrupts
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
Simple typos, s/g2/g1/
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d6d9d66b81ce..6aa81d2d8f63 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -415,11 +415,11 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
int irq, virq;
for (irq = 0; irq < 16; irq++) {
- virq = irq_find_mapping(chip->g2_irq.domain, irq);
+ virq = irq_find_mapping(chip->g1_irq.domain, irq);
irq_dispose_mapping(virq);
}
- irq_domain_remove(chip->g2_irq.domain);
+ irq_domain_remove(chip->g1_irq.domain);
}
static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
--
2.10.2
^ permalink raw reply related
* [PATCH v2 net-next 3/6] net: dsa: mv88e6xxx: Mask g1 interrupts and free interrupt
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
Fix the g1 interrupt free code such that is masks any further
interrupts, and then releases the interrupt.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b843052d32bd..8fcef7e0d3ba 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -413,6 +413,13 @@ static const struct irq_domain_ops mv88e6xxx_g1_irq_domain_ops = {
static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
{
int irq, virq;
+ u16 mask;
+
+ mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &mask);
+ mask |= GENMASK(chip->g1_irq.nirqs, 0);
+ mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, mask);
+
+ free_irq(chip->irq, chip);
for (irq = 0; irq < 16; irq++) {
virq = irq_find_mapping(chip->g1_irq.domain, irq);
--
2.10.2
^ permalink raw reply related
* [PATCH v2 net-next 6/6] net: dsa: mv88e6xxx: Hold the mutex while freeing g1 interrupts
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
Freeing interrupts requires switch register access to mask the
interrupts. Hence we must hold the register mutex.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 614b2f68d401..e30d0eaf2b5f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3916,8 +3916,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0)
mv88e6xxx_g2_irq_free(chip);
out_g1_irq:
- if (chip->irq > 0)
+ if (chip->irq > 0) {
+ mutex_lock(&chip->reg_lock);
mv88e6xxx_g1_irq_free(chip);
+ mutex_unlock(&chip->reg_lock);
+ }
out:
return err;
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 net-next 0/6] Fixes for the MV88e6xxx interrupt code
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
The interrupt code was never tested with a board who's probing
resulted in an -EPROBE_DEFFERED. So the clean up paths never got
tested. I now do have -EPROBE_DEFFERED, and things break badly during
cleanup. These are the fixes.
This is fixing code in net-next.
v2:
Fix typo pointed out by David Miller
Andrew Lunn (6):
net: dsa: mv88e6xxx: Fix typos when removing g1 interrupts
net: dsa: mv88e6xxx: Fix unconditional irq freeing
net: dsa: mv88e6xxx: Mask g1 interrupts and free interrupt
net: dsa: mv88e6xxx: Fix cleanup on error for g1 interrupt setup
net: dsa: mv88e6xxx: Fix releasing for the global2 interrupts
net: dsa: mv88e6xxx: Hold the mutex while freeing g1 interrupts
drivers/net/dsa/mv88e6xxx/chip.c | 58 ++++++++++++++++++++++++-----------
drivers/net/dsa/mv88e6xxx/global2.c | 28 +++++++++++------
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
3 files changed, 59 insertions(+), 28 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH v2 net-next 2/6] net: dsa: mv88e6xxx: Fix unconditional irq freeing
From: Andrew Lunn @ 2016-11-20 19:14 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn
In-Reply-To: <1479669260-14638-1-git-send-email-andrew@lunn.ch>
Trying to remove an IRQ domain that was not created results in an
Opps. Add the necessary checks that the irqs were created before
freeing them.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6aa81d2d8f63..b843052d32bd 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3897,10 +3897,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
out_mdio:
mv88e6xxx_mdio_unregister(chip);
out_g2_irq:
- if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
+ if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0)
mv88e6xxx_g2_irq_free(chip);
out_g1_irq:
- mv88e6xxx_g1_irq_free(chip);
+ if (chip->irq > 0)
+ mv88e6xxx_g1_irq_free(chip);
out:
return err;
}
@@ -3914,9 +3915,11 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
mv88e6xxx_unregister_switch(chip);
mv88e6xxx_mdio_unregister(chip);
- if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
- mv88e6xxx_g2_irq_free(chip);
- mv88e6xxx_g1_irq_free(chip);
+ if (chip->irq > 0) {
+ if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
+ mv88e6xxx_g2_irq_free(chip);
+ mv88e6xxx_g1_irq_free(chip);
+ }
}
static const struct of_device_id mv88e6xxx_of_match[] = {
--
2.10.2
^ permalink raw reply related
* Re: [RFC PATCH v2 1/2] macb: Add 1588 support in Cadence GEM.
From: Richard Cochran @ 2016-11-20 19:18 UTC (permalink / raw)
To: Andrei Pistirica
Cc: tbultel, boris.brezillon, netdev, alexandre.belloni,
nicolas.ferre, linux-kernel, harinikatakamlinux, michals, anirudh,
punnaia, harini.katakam, davem, linux-arm-kernel
In-Reply-To: <1479478912-14067-1-git-send-email-andrei.pistirica@microchip.com>
On Fri, Nov 18, 2016 at 03:21:51PM +0100, Andrei Pistirica wrote:
> - Frequency adjustment is not directly supported by this IP.
This statement still makes no sense. Doesn't the following text...
> addend is the initial value ns increment and similarly addendesub.
> The ppb (parts per billion) provided is used as
> ns_incr = addend +/- (ppb/rate).
> Similarly the remainder of the above is used to populate subns increment.
describe how frequency adjustment is in fact supported?
> +config MACB_USE_HWSTAMP
> + bool "Use IEEE 1588 hwstamp"
> + depends on MACB
> + default y
> + select PTP_1588_CLOCK
This "select" pattern is going to be replaced with "imply" soon.
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1269181.html
You should use the new "imply" key word and reference that series in
your change log.
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 3f385ab..2ee9af8 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -10,6 +10,10 @@
> #ifndef _MACB_H
> #define _MACB_H
>
> +#include <linux/net_tstamp.h>
> +#include <linux/ptp_clock.h>
> +#include <linux/ptp_clock_kernel.h>
Don't need net_tstamp.h here. Move it into the .c file please.
> @@ -840,8 +902,26 @@ struct macb {
>
> unsigned int rx_frm_len_mask;
> unsigned int jumbo_max_len;
> +
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> + unsigned int hwts_tx_en;
> + unsigned int hwts_rx_en;
These two can be bool'eans.
> + spinlock_t tsu_clk_lock;
> + unsigned int tsu_rate;
> +
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info ptp_caps;
> + unsigned int ns_incr;
> + unsigned int subns_incr;
These two are 32 bit register values, right? Then use the u32 type.
> +#endif
> };
> +static inline void macb_tsu_set_time(struct macb *bp,
> + const struct timespec64 *ts)
> +{
> + u32 ns, sech, secl;
> + s64 word_mask = 0xffffffff;
> +
> + sech = (u32)ts->tv_sec;
> + secl = (u32)ts->tv_sec;
> + ns = ts->tv_nsec;
> + if (ts->tv_sec > word_mask)
> + sech = (ts->tv_sec >> 32);
> +
> + spin_lock(&bp->tsu_clk_lock);
> +
> + /* TSH doesn't latch the time and no atomicity! */
> + gem_writel(bp, TSH, sech);
> + gem_writel(bp, TSL, secl);
If TN overflows here then the clock will be off by one whole second!
Why not clear TN first?
> + gem_writel(bp, TN, ns);
> +
> + spin_unlock(&bp->tsu_clk_lock);
> +}
> +
> +static int macb_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> +{
> + struct macb *bp = container_of(ptp, struct macb, ptp_caps);
> + u32 addend, addend_frac, rem;
> + u64 drift_tmr, diff, diff_frac = 0;
> + int neg_adj = 0;
> +
> + if (ppb < 0) {
> + neg_adj = 1;
> + ppb = -ppb;
> + }
> +
> + /* drift/period */
> + drift_tmr = (bp->ns_incr * ppb) +
> + ((bp->subns_incr * ppb) >> 16);
What? Why the 16 bit shift? Last time your said it was 24 bits.
> + /* drift/cycle */
> + diff = div_u64_rem(drift_tmr, 1000000000ULL, &rem);
> +
> + /* drift fraction/cycle, if necessary */
> + if (rem) {
> + u64 fraction = rem;
> + fraction = fraction << 16;
> +
> + diff_frac = div_u64(fraction, 1000000000ULL);
If you use a combined tuning word like I explained last time, then you
will not need a second division.
Also, please use the new adjfine() PHC method, as adjfreq() is now deprecated.
> + }
> +
> + /* adjustmets */
> + addend = neg_adj ? (bp->ns_incr - diff) : (bp->ns_incr + diff);
> + addend_frac = neg_adj ? (bp->subns_incr - diff_frac) :
> + (bp->subns_incr + diff_frac);
> +
> + spin_lock(&bp->tsu_clk_lock);
> +
> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, addend_frac));
> + gem_writel(bp, TI, GEM_BF(NSINCR, addend));
> +
> + spin_unlock(&bp->tsu_clk_lock);
> + return 0;
> +}
> +void macb_ptp_init(struct net_device *ndev)
> +{
> + struct macb *bp = netdev_priv(ndev);
> + struct timespec64 now;
> + u32 rem = 0;
> +
> + if (!(bp->caps | MACB_CAPS_GEM_HAS_PTP)){
> + netdev_vdbg(bp->dev, "Platform does not support PTP!\n");
> + return;
> + }
> +
> + spin_lock_init(&bp->tsu_clk_lock);
> +
> + bp->ptp_caps = macb_ptp_caps;
> + bp->tsu_rate = clk_get_rate(bp->pclk);
> +
> + getnstimeofday64(&now);
> + macb_tsu_set_time(bp, (const struct timespec64 *)&now);
> +
> + bp->ns_incr = div_u64_rem(NSEC_PER_SEC, bp->tsu_rate, &rem);
> + if (rem) {
> + u64 adj = rem;
> + /* Multiply by 2^16 as subns register is 16 bits */
Last time you said:
> + /* Multiple by 2^24 as subns field is 24 bits */
> + adj <<= 16;
> + bp->subns_incr = div_u64(adj, bp->tsu_rate);
> + } else {
> + bp->subns_incr = 0;
> + }
> +
> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, bp->subns_incr));
> + gem_writel(bp, TI, GEM_BF(NSINCR, bp->ns_incr));
> + gem_writel(bp, TA, 0);
> +
> + bp->ptp_clock = ptp_clock_register(&bp->ptp_caps, NULL);
You call regsiter, but you never call unregister!
> + if (IS_ERR(&bp->ptp_clock)) {
> + bp->ptp_clock = NULL;
> + pr_err("ptp clock register failed\n");
> + return;
> + }
> +
> + dev_info(&bp->pdev->dev, "%s ptp clock registered.\n", GMAC_TIMER_NAME);
> +}
> +
> --
> 1.9.1
>
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH v2 1/2] macb: Add 1588 support in Cadence GEM.
From: Richard Cochran @ 2016-11-20 19:37 UTC (permalink / raw)
To: Andrei Pistirica
Cc: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam, punnaia, michals, anirudh,
boris.brezillon, alexandre.belloni, tbultel
In-Reply-To: <1479478912-14067-1-git-send-email-andrei.pistirica@microchip.com>
On Fri, Nov 18, 2016 at 03:21:51PM +0100, Andrei Pistirica wrote:
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> +void macb_ptp_init(struct net_device *ndev);
> +#else
> +void macb_ptp_init(struct net_device *ndev) { }
static inline ^^^
> +#endif
> +void macb_ptp_init(struct net_device *ndev)
> +{
> + struct macb *bp = netdev_priv(ndev);
> + struct timespec64 now;
> + u32 rem = 0;
> +
> + if (!(bp->caps | MACB_CAPS_GEM_HAS_PTP)){
> + netdev_vdbg(bp->dev, "Platform does not support PTP!\n");
> + return;
> + }
You would have needed '&' and not '|' here.
Also, using a flag limits the code to your platform. This works for
you, but it is short sighted. The other MACB PTP blocks have
different register layouts, and this patch does not lay the ground
work for the others.
The driver needs to be designed to support the other platforms.
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH v2 2/2] macb: Enable 1588 support in SAMA5D2 platform.
From: Richard Cochran @ 2016-11-20 19:54 UTC (permalink / raw)
To: Andrei Pistirica
Cc: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam, punnaia, michals, anirudh,
boris.brezillon, alexandre.belloni, tbultel
In-Reply-To: <1479478912-14067-2-git-send-email-andrei.pistirica@microchip.com>
On Fri, Nov 18, 2016 at 03:21:52PM +0100, Andrei Pistirica wrote:
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index d975882..eb66b76 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -697,6 +697,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
>
> /* First, update TX stats if needed */
> if (skb) {
> + macb_ptp_do_txstamp(bp, skb);
This is in the hot path, and so you should have an inline wrapper that
tests (bp->hwts_tx_en) and THEN calls into macb_ptp.c
In case PTP isn't configured, then the inline wrapper should be empty.
> netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
> macb_tx_ring_wrap(tail), skb->data);
> bp->stats.tx_packets++;
> @@ -853,6 +855,8 @@ static int gem_rx(struct macb *bp, int budget)
> GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> + macb_ptp_do_rxstamp(bp, skb);
Same here.
> bp->stats.rx_packets++;
> bp->stats.rx_bytes += skb->len;
>
> @@ -1946,6 +1950,8 @@ static int macb_open(struct net_device *dev)
>
> netif_tx_start_all_queues(dev);
>
> + macb_ptp_init(dev);
This leaks PHC instances starting the second time that the interface goes up!
> return 0;
> }
>
> @@ -2204,7 +2210,7 @@ static const struct ethtool_ops gem_ethtool_ops = {
> .get_regs_len = macb_get_regs_len,
> .get_regs = macb_get_regs,
> .get_link = ethtool_op_get_link,
> - .get_ts_info = ethtool_op_get_ts_info,
> + .get_ts_info = macb_get_ts_info,
You enable the time stamping logic unconditionally here ...
> .get_ethtool_stats = gem_get_ethtool_stats,
> .get_strings = gem_get_ethtool_strings,
> .get_sset_count = gem_get_sset_count,
> @@ -2221,7 +2227,14 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> if (!phydev)
> return -ENODEV;
>
> - return phy_mii_ioctl(phydev, rq, cmd);
> + switch (cmd) {
> + case SIOCSHWTSTAMP:
> + return macb_hwtst_set(dev, rq, cmd);
> + case SIOCGHWTSTAMP:
> + return macb_hwtst_get(dev, rq);
and here.
Is that logic always available on every MACB device? If so, is the
implementation also identical?
Thanks,
Richard
^ permalink raw reply
* [PATCH net-next 1/1] driver: macvlan: Remove duplicated IFF_UP condition check in macvlan_forward_source
From: fgao @ 2016-11-21 0:26 UTC (permalink / raw)
To: davem, kaber, netdev, gfree.wind
From: Gao Feng <gfree.wind@gmail.com>
The function macvlan_forward_source_one has already checked the flag
IFF_UP, so needn't check it outside in macvlan_forward_source too.
Signed-off-by: Gao Feng <gfree.wind@gmail.com>
---
v2: Remove the IFF_UP check in macvlan_forward_source instead of macvlan_forward_source_one
v1: Initial patch
drivers/net/macvlan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 13b7e0b..7ddfd2c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -400,8 +400,7 @@ static void macvlan_forward_source(struct sk_buff *skb,
hlist_for_each_entry_rcu(entry, h, hlist) {
if (ether_addr_equal_64bits(entry->addr, addr))
- if (entry->vlan->dev->flags & IFF_UP)
- macvlan_forward_source_one(skb, entry->vlan);
+ macvlan_forward_source_one(skb, entry->vlan);
}
}
--
1.9.1
^ permalink raw reply related
* [PATCH net 1/1] net: batman-adv: Treat NET_XMIT_CN as transmit successfully
From: fgao @ 2016-11-21 0:39 UTC (permalink / raw)
To: mareklindner, sw, a, davem, b.a.t.m.a.n, netdev, gfree.wind; +Cc: Gao Feng
From: Gao Feng <fgao@ikuai8.com>
The tc could return NET_XMIT_CN as one congestion notification, but
it does not mean the packe is lost. Other modules like ipvlan,
macvlan, and others treat NET_XMIT_CN as success too.
So batman-adv should add the NET_XMIT_CN check.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/batman-adv/distributed-arp-table.c | 2 +-
net/batman-adv/fragmentation.c | 2 +-
net/batman-adv/routing.c | 2 +-
net/batman-adv/tp_meter.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index e257efd..4bf0622 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -660,7 +660,7 @@ static bool batadv_dat_send_data(struct batadv_priv *bat_priv,
}
send_status = batadv_send_unicast_skb(tmp_skb, neigh_node);
- if (send_status == NET_XMIT_SUCCESS) {
+ if (send_status == NET_XMIT_SUCCESS || send_status == NET_XMIT_CN) {
/* count the sent packet */
switch (packet_subtype) {
case BATADV_P_DAT_DHT_GET:
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index 0934730..4714b8f 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -495,7 +495,7 @@ int batadv_frag_send_packet(struct sk_buff *skb,
batadv_add_counter(bat_priv, BATADV_CNT_FRAG_TX_BYTES,
skb_fragment->len + ETH_HLEN);
ret = batadv_send_unicast_skb(skb_fragment, neigh_node);
- if (ret != NET_XMIT_SUCCESS) {
+ if (ret != NET_XMIT_SUCCESS && ret != NET_XMIT_CN) {
/* return -1 so that the caller can free the original
* skb
*/
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 7e8dc64..8edd324 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -706,7 +706,7 @@ static int batadv_route_unicast_packet(struct sk_buff *skb,
goto out;
/* translate transmit result into receive result */
- if (res == NET_XMIT_SUCCESS) {
+ if (res == NET_XMIT_SUCCESS || ret == NET_XMIT_CN) {
/* skb was transmitted and consumed */
batadv_inc_counter(bat_priv, BATADV_CNT_FORWARD);
batadv_add_counter(bat_priv, BATADV_CNT_FORWARD_BYTES,
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 8af1611..461dbad 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -618,7 +618,7 @@ static int batadv_tp_send_msg(struct batadv_tp_vars *tp_vars, const u8 *src,
if (r == -1)
kfree_skb(skb);
- if (r == NET_XMIT_SUCCESS)
+ if (r == NET_XMIT_SUCCESS || r == NET_XMIT_CN)
return 0;
return BATADV_TP_REASON_CANT_SEND;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 1/1] driver: macvlan: Remove duplicated IFF_UP condition check in macvlan_forward_source
From: Feng Gao @ 2016-11-21 0:52 UTC (permalink / raw)
To: David S. Miller, Patrick McHardy, Linux Kernel Network Developers,
Feng Gao
In-Reply-To: <1479687998-456-1-git-send-email-fgao@ikuai8.com>
On Mon, Nov 21, 2016 at 8:26 AM, <fgao@ikuai8.com> wrote:
> From: Gao Feng <gfree.wind@gmail.com>
>
> The function macvlan_forward_source_one has already checked the flag
> IFF_UP, so needn't check it outside in macvlan_forward_source too.
>
> Signed-off-by: Gao Feng <gfree.wind@gmail.com>
> ---
> v2: Remove the IFF_UP check in macvlan_forward_source instead of macvlan_forward_source_one
> v1: Initial patch
>
> drivers/net/macvlan.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 13b7e0b..7ddfd2c 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -400,8 +400,7 @@ static void macvlan_forward_source(struct sk_buff *skb,
>
> hlist_for_each_entry_rcu(entry, h, hlist) {
> if (ether_addr_equal_64bits(entry->addr, addr))
> - if (entry->vlan->dev->flags & IFF_UP)
> - macvlan_forward_source_one(skb, entry->vlan);
> + macvlan_forward_source_one(skb, entry->vlan);
> }
> }
>
> --
> 1.9.1
>
>
Sorry, I forget add the "v2" in the title.
Regards
Feng
^ permalink raw reply
* [PATCH net 1/1] net: l2tp: Treat NET_XMIT_CN as success in l2tp_eth_dev_xmit
From: fgao @ 2016-11-21 0:56 UTC (permalink / raw)
To: edumazet, davem, javier, netdev; +Cc: gfree.wind
From: Gao Feng <gfree.wind@gmail.com>
The tc could return NET_XMIT_CN as one congestion notification, but
it does not mean the packe is lost. Other modules like ipvlan,
macvlan, and others treat NET_XMIT_CN as success too.
So l2tp_eth_dev_xmit should add the NET_XMIT_CN check.
Signed-off-by: Gao Feng <gfree.wind@gmail.com>
---
net/l2tp/l2tp_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 965f7e3..3dc97b4 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -97,7 +97,7 @@ static int l2tp_eth_dev_xmit(struct sk_buff *skb, struct net_device *dev)
unsigned int len = skb->len;
int ret = l2tp_xmit_skb(session, skb, session->hdr_len);
- if (likely(ret == NET_XMIT_SUCCESS)) {
+ if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
atomic_long_add(len, &priv->tx_bytes);
atomic_long_inc(&priv->tx_packets);
} else {
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox