From patchwork Fri Jul 14 23:12:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Berger X-Patchwork-Id: 788827 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3x8T4b4yxQz9s65 for ; Sat, 15 Jul 2017 09:13:03 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="qa9s8mmY"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751221AbdGNXMs (ORCPT ); Fri, 14 Jul 2017 19:12:48 -0400 Received: from mail-qk0-f196.google.com ([209.85.220.196]:34587 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751154AbdGNXMn (ORCPT ); Fri, 14 Jul 2017 19:12:43 -0400 Received: by mail-qk0-f196.google.com with SMTP id q66so13958854qki.1; Fri, 14 Jul 2017 16:12:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=VqtdetjM9F5u2H3c5UmSjZOhNB+hhaar4VxZTckL+fQ=; b=qa9s8mmYrG+nWLuqir7OGgYfn6qfk9+OWEy8hpZekr89qAR8vHthdnjUf6++zIjl2Y 2L0sdt0tf22hGSbb3OMbZDtU0nDbCHGJTCyMsVac/5NNY0NYa6ybP5FWCVQgaGwUU9LT vFrEhs7LTaeob5pUL+PxpLfoTYs4rQkyKp2LEItQiKG2NNdDGzy7jUrLQOGnt0T4lyY2 GkB5mEQ8g2VKoNsMYVl0VyLXMuFEOZe26viAWC4IhEBtzlOEPhYSU/J1U/MLglrQPmnN Yra9iqwIaqtt3yncSXA40J2E5Lt114cNfu3viHd7e2FNA4VkBUDdNL4/oA7lwPYVqTfh /J8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=VqtdetjM9F5u2H3c5UmSjZOhNB+hhaar4VxZTckL+fQ=; b=HmviWC/me1UPwk8Ba9QrxwrU8BROpp0wU1jHNcidYgsBjxNhAlNNspeM6QuPArXMoi 5i3RWI81KfuakjUcCtBs6YgqHwvQ9N2tLnemQYNuSIvPHkK9fbng6SHa4NYBgUN3LEGD 6enGf/MWitkpHqGIz0+HAcBHrG7sik2JW+ZKBEDBuMbPKCWOAlB8IPwpfZDb0MaTghI1 thReZXFteLctwDXjTkLQif2Z8TJJvGYmxjlG/dbElxKpJCuEUQg4SMMX2tIxvshpLCuC xsJxwSltw+Nvz6Z0OQ81C+7zEWLI65gtrBIJAFgANaaplohFDDsh13l+oR8tpvcFhUkC LLKA== X-Gm-Message-State: AIVw110VcG5um8Ikj/5fDPcZtiLcW3V4mydXMRHGPu5JAojS5akI7rGY HqdRVJQq8P6Jc2lzE04= X-Received: by 10.55.104.81 with SMTP id d78mr14068078qkc.24.1500073962816; Fri, 14 Jul 2017 16:12:42 -0700 (PDT) Received: from stb-bld-02.irv.broadcom.com ([192.19.255.250]) by smtp.gmail.com with ESMTPSA id z5sm7436938qkz.53.2017.07.14.16.12.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 14 Jul 2017 16:12:41 -0700 (PDT) From: Doug Berger To: netdev@vger.kernel.org Cc: Florian Fainelli , linux-kernel@vger.kernel.org, Doug Berger Subject: [PATCH net 2/2] net: bcmgenet: Free skb after last Tx frag Date: Fri, 14 Jul 2017 16:12:10 -0700 Message-Id: <20170714231210.12967-3-opendmb@gmail.com> X-Mailer: git-send-email 2.13.0 In-Reply-To: <20170714231210.12967-1-opendmb@gmail.com> References: <20170714231210.12967-1-opendmb@gmail.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Since the skb is attached to the first control block of a fragmented skb it is possible that the skb could be freed when reclaiming that control block before all fragments of the skb have been consumed by the hardware and unmapped. This commit introduces first_cb and last_cb pointers to the skb control block used by the driver to keep track of which transmit control blocks within a transmit ring are the first and last ones associated with the skb. It then splits the bcmgenet_free_cb() function into transmit (bcmgenet_free_tx_cb) and receive (bcmgenet_free_rx_cb) versions that can handle the unmapping of dma mapped memory and cleaning up the corresponding control block structure so that the skb is only freed after the last associated transmit control block is reclaimed. Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file") Signed-off-by: Doug Berger --- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 142 ++++++++++++++----------- drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 + 2 files changed, 84 insertions(+), 60 deletions(-) diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c index 20021525f795..7b0b399aaedd 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c @@ -1219,14 +1219,6 @@ static struct enet_cb *bcmgenet_put_txcb(struct bcmgenet_priv *priv, return tx_cb_ptr; } -/* Simple helper to free a control block's resources */ -static void bcmgenet_free_cb(struct enet_cb *cb) -{ - dev_kfree_skb_any(cb->skb); - cb->skb = NULL; - dma_unmap_addr_set(cb, dma_addr, 0); -} - static inline void bcmgenet_rx_ring16_int_disable(struct bcmgenet_rx_ring *ring) { bcmgenet_intrl2_0_writel(ring->priv, UMAC_IRQ_RXDMA_DONE, @@ -1277,18 +1269,72 @@ static inline void bcmgenet_tx_ring_int_disable(struct bcmgenet_tx_ring *ring) INTRL2_CPU_MASK_SET); } +/* Simple helper to free a transmit control block's resources + * Returns an skb when the last transmit control block associated with the + * skb is freed. The skb should be freed by the caller if necessary. + */ +static struct sk_buff *bcmgenet_free_tx_cb(struct device *dev, + struct enet_cb *cb) +{ + struct sk_buff *skb; + + skb = cb->skb; + + if (skb) { + cb->skb = NULL; + if (cb == GENET_CB(skb)->first_cb) + dma_unmap_single(dev, dma_unmap_addr(cb, dma_addr), + dma_unmap_len(cb, dma_len), + DMA_TO_DEVICE); + else + dma_unmap_page(dev, dma_unmap_addr(cb, dma_addr), + dma_unmap_len(cb, dma_len), + DMA_TO_DEVICE); + dma_unmap_addr_set(cb, dma_addr, 0); + + if (cb == GENET_CB(skb)->last_cb) + return skb; + + } else if (dma_unmap_addr(cb, dma_addr)) { + dma_unmap_page(dev, + dma_unmap_addr(cb, dma_addr), + dma_unmap_len(cb, dma_len), + DMA_TO_DEVICE); + dma_unmap_addr_set(cb, dma_addr, 0); + } + + return 0; +} + +/* Simple helper to free a receive control block's resources */ +static struct sk_buff *bcmgenet_free_rx_cb(struct device *dev, + struct enet_cb *cb) +{ + struct sk_buff *skb; + + skb = cb->skb; + cb->skb = NULL; + + if (dma_unmap_addr(cb, dma_addr)) { + dma_unmap_single(dev, dma_unmap_addr(cb, dma_addr), + dma_unmap_len(cb, dma_len), DMA_FROM_DEVICE); + dma_unmap_addr_set(cb, dma_addr, 0); + } + + return skb; +} + /* Unlocked version of the reclaim routine */ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev, struct bcmgenet_tx_ring *ring) { struct bcmgenet_priv *priv = netdev_priv(dev); - struct device *kdev = &priv->pdev->dev; - struct enet_cb *tx_cb_ptr; - unsigned int pkts_compl = 0; + unsigned int txbds_processed = 0; unsigned int bytes_compl = 0; - unsigned int c_index; + unsigned int pkts_compl = 0; unsigned int txbds_ready; - unsigned int txbds_processed = 0; + unsigned int c_index; + struct sk_buff *skb; /* Clear status before servicing to reduce spurious interrupts */ if (ring->index == DESC_INDEX) @@ -1309,21 +1355,12 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev, /* Reclaim transmitted buffers */ while (txbds_processed < txbds_ready) { - tx_cb_ptr = &priv->tx_cbs[ring->clean_ptr]; - if (tx_cb_ptr->skb) { + skb = bcmgenet_free_tx_cb(&priv->pdev->dev, + &priv->tx_cbs[ring->clean_ptr]); + if (skb) { pkts_compl++; - bytes_compl += GENET_CB(tx_cb_ptr->skb)->bytes_sent; - dma_unmap_single(kdev, - dma_unmap_addr(tx_cb_ptr, dma_addr), - dma_unmap_len(tx_cb_ptr, dma_len), - DMA_TO_DEVICE); - bcmgenet_free_cb(tx_cb_ptr); - } else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) { - dma_unmap_page(kdev, - dma_unmap_addr(tx_cb_ptr, dma_addr), - dma_unmap_len(tx_cb_ptr, dma_len), - DMA_TO_DEVICE); - dma_unmap_addr_set(tx_cb_ptr, dma_addr, 0); + bytes_compl += GENET_CB(skb)->bytes_sent; + dev_kfree_skb_any(skb); } txbds_processed++; @@ -1533,13 +1570,12 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev) if (!i) { /* Transmit single SKB or head of fragment list */ - tx_cb_ptr->skb = skb; + GENET_CB(skb)->first_cb = tx_cb_ptr; size = skb_headlen(skb); mapping = dma_map_single(kdev, skb->data, size, DMA_TO_DEVICE); } else { /* xmit fragment */ - tx_cb_ptr->skb = NULL; frag = &skb_shinfo(skb)->frags[i - 1]; size = skb_frag_size(frag); mapping = skb_frag_dma_map(kdev, frag, 0, size, @@ -1556,6 +1592,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev) dma_unmap_addr_set(tx_cb_ptr, dma_addr, mapping); dma_unmap_len_set(tx_cb_ptr, dma_len, size); + tx_cb_ptr->skb = skb; + len_stat = (size << DMA_BUFLENGTH_SHIFT) | (priv->hw_params->qtag_mask << DMA_TX_QTAG_SHIFT); @@ -1570,6 +1608,7 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev) dmadesc_set(priv, tx_cb_ptr->bd_addr, mapping, len_stat); } + GENET_CB(skb)->last_cb = tx_cb_ptr; skb_tx_timestamp(skb); /* Decrement total BD count and advance our write pointer */ @@ -1598,18 +1637,7 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev) /* Unmap successfully mapped control blocks */ while (i-- > 0) { tx_cb_ptr = bcmgenet_put_txcb(priv, ring); - if (tx_cb_ptr->skb) - dma_unmap_single(kdev, - dma_unmap_addr(tx_cb_ptr, dma_addr), - dma_unmap_len(tx_cb_ptr, dma_len), - DMA_TO_DEVICE); - else - dma_unmap_page(kdev, - dma_unmap_addr(tx_cb_ptr, dma_addr), - dma_unmap_len(tx_cb_ptr, dma_len), - DMA_TO_DEVICE); - dma_unmap_addr_set(tx_cb_ptr, dma_addr, 0); - tx_cb_ptr->skb = NULL; + bcmgenet_free_tx_cb(kdev, tx_cb_ptr); } dev_kfree_skb(skb); @@ -1645,14 +1673,12 @@ static struct sk_buff *bcmgenet_rx_refill(struct bcmgenet_priv *priv, } /* Grab the current Rx skb from the ring and DMA-unmap it */ - rx_skb = cb->skb; - if (likely(rx_skb)) - dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr), - priv->rx_buf_len, DMA_FROM_DEVICE); + rx_skb = bcmgenet_free_rx_cb(kdev, cb); /* Put the new Rx skb on the ring */ cb->skb = skb; dma_unmap_addr_set(cb, dma_addr, mapping); + dma_unmap_len_set(cb, dma_len, priv->rx_buf_len); dmadesc_set_addr(priv, cb->bd_addr, mapping); /* Return the current Rx skb to caller */ @@ -1859,22 +1885,16 @@ static int bcmgenet_alloc_rx_buffers(struct bcmgenet_priv *priv, static void bcmgenet_free_rx_buffers(struct bcmgenet_priv *priv) { - struct device *kdev = &priv->pdev->dev; + struct sk_buff *skb; struct enet_cb *cb; int i; for (i = 0; i < priv->num_rx_bds; i++) { cb = &priv->rx_cbs[i]; - if (dma_unmap_addr(cb, dma_addr)) { - dma_unmap_single(kdev, - dma_unmap_addr(cb, dma_addr), - priv->rx_buf_len, DMA_FROM_DEVICE); - dma_unmap_addr_set(cb, dma_addr, 0); - } - - if (cb->skb) - bcmgenet_free_cb(cb); + skb = bcmgenet_free_rx_cb(&priv->pdev->dev, cb); + if (skb) + dev_kfree_skb_any(skb); } } @@ -2458,8 +2478,10 @@ static int bcmgenet_dma_teardown(struct bcmgenet_priv *priv) static void bcmgenet_fini_dma(struct bcmgenet_priv *priv) { - int i; struct netdev_queue *txq; + struct sk_buff *skb; + struct enet_cb *cb; + int i; bcmgenet_fini_rx_napi(priv); bcmgenet_fini_tx_napi(priv); @@ -2468,10 +2490,10 @@ static void bcmgenet_fini_dma(struct bcmgenet_priv *priv) bcmgenet_dma_teardown(priv); for (i = 0; i < priv->num_tx_bds; i++) { - if (priv->tx_cbs[i].skb != NULL) { - dev_kfree_skb(priv->tx_cbs[i].skb); - priv->tx_cbs[i].skb = NULL; - } + cb = priv->tx_cbs + i; + skb = bcmgenet_free_tx_cb(&priv->pdev->dev, cb); + if (skb) + dev_kfree_skb(skb); } for (i = 0; i < priv->hw_params->tx_queues; i++) { diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h index efd07020b89f..b9344de669f8 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h @@ -544,6 +544,8 @@ struct bcmgenet_hw_params { }; struct bcmgenet_skb_cb { + struct enet_cb *first_cb; /* First control block of SKB */ + struct enet_cb *last_cb; /* Last control block of SKB */ unsigned int bytes_sent; /* bytes on the wire (no TSB) */ };