diff mbox series

[net-next] net: bcmgenet: suppress warnings on failed Rx SKB allocations

Message ID 1587682931-38636-1-git-send-email-opendmb@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series [net-next] net: bcmgenet: suppress warnings on failed Rx SKB allocations | expand

Commit Message

Doug Berger April 23, 2020, 11:02 p.m. UTC
The driver is designed to drop Rx packets and reclaim the buffers
when an allocation fails, and the network interface needs to safely
handle this packet loss. Therefore, an allocation failure of Rx
SKBs is relatively benign.

However, the output of the warning message occurs with a high
scheduling priority that can cause excessive jitter/latency for
other high priority processing.

This commit suppresses the warning messages to prevent scheduling
problems while retaining the failure count in the statistics of
the network interface.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Florian Fainelli April 23, 2020, 11:10 p.m. UTC | #1
On 4/23/2020 4:02 PM, Doug Berger wrote:
> The driver is designed to drop Rx packets and reclaim the buffers
> when an allocation fails, and the network interface needs to safely
> handle this packet loss. Therefore, an allocation failure of Rx
> SKBs is relatively benign.
> 
> However, the output of the warning message occurs with a high
> scheduling priority that can cause excessive jitter/latency for
> other high priority processing.
> 
> This commit suppresses the warning messages to prevent scheduling
> problems while retaining the failure count in the statistics of
> the network interface.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

It seems to me this should be the default behavior for all network
device drivers, but I am fine with this being a driver decision if
people think differently.

> ---
>  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 20aba79becce..bfeff5585f4b 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1617,7 +1617,8 @@ static struct sk_buff *bcmgenet_rx_refill(struct bcmgenet_priv *priv,
>  	dma_addr_t mapping;
>  
>  	/* Allocate a new Rx skb */
> -	skb = netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT);
> +	skb = __netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT,
> +				 GFP_ATOMIC | __GFP_NOWARN);
>  	if (!skb) {
>  		priv->mib.alloc_rx_buff_failed++;
>  		netif_err(priv, rx_err, priv->dev,
>
David Miller April 24, 2020, 11:49 p.m. UTC | #2
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 23 Apr 2020 16:10:07 -0700

> It seems to me this should be the default behavior for all network
> device drivers, but I am fine with this being a driver decision if
> people think differently.

Yeah I think the behavior should be consistent and default across
drivers too.
David Miller April 24, 2020, 11:50 p.m. UTC | #3
From: Doug Berger <opendmb@gmail.com>
Date: Thu, 23 Apr 2020 16:02:11 -0700

> The driver is designed to drop Rx packets and reclaim the buffers
> when an allocation fails, and the network interface needs to safely
> handle this packet loss. Therefore, an allocation failure of Rx
> SKBs is relatively benign.
> 
> However, the output of the warning message occurs with a high
> scheduling priority that can cause excessive jitter/latency for
> other high priority processing.
> 
> This commit suppresses the warning messages to prevent scheduling
> problems while retaining the failure count in the statistics of
> the network interface.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>

Applied.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 20aba79becce..bfeff5585f4b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1617,7 +1617,8 @@  static struct sk_buff *bcmgenet_rx_refill(struct bcmgenet_priv *priv,
 	dma_addr_t mapping;
 
 	/* Allocate a new Rx skb */
-	skb = netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT);
+	skb = __netdev_alloc_skb(priv->dev, priv->rx_buf_len + SKB_ALIGNMENT,
+				 GFP_ATOMIC | __GFP_NOWARN);
 	if (!skb) {
 		priv->mib.alloc_rx_buff_failed++;
 		netif_err(priv, rx_err, priv->dev,