sky2: Allocate initial skbs in sky2_alloc_buffers
Subsystems (2)
PATCH 2/3 sky2: Allocate initial skbs in sky2_alloc_buffers
Allocating everything in one place means there's a single point of failure in sky2_up, and sky2_rx_start can no longer fail.
This also fixes a memory leak in the case that sky2_rx_start fails in the middle of allocating skbs, since any allocated skbs will not be free'd in sky2_up's failure path.
Signed-off-by Mike McCormack <[email protected]>
--- drivers/net/sky2.c | 42 +++++++++++++++++++++++------------------- 1 files changed, 23 insertions(+), 19 deletions(-)
drivers/net/sky2.c +23 -19
@@ -1358,7 +1358,7 @@ static inline void sky2_rx_update(struct sky2_port *sky2, unsigned rxq) }/* -* Allocate and setup receiver buffer pool. +* Setup receiver buffer pool. * Normal case this ends up creating one list element for skb * in the receive ring. Worst case if using large MTU and each * allocation falls on a different 64 bit region, that results
@@ -1392,22 +1392,10 @@ static int sky2_rx_start(struct sky2_port *sky2)if (!(hw->flags &SKY2_HW_NEW_LE ))rx_set_checksum (sky2); - sky2->rx_data_size =sky2_get_rx_data_size (sky2); -/* Fill Rx ring */ +/* submit Rx ring */ for (i =0 ; i < sky2->rx_pending; i++) { re = sky2->rx_ring + i; - - re->skb =sky2_rx_alloc (sky2); -if (!re->skb) - goto nomem; - -if (sky2_rx_map_skb (hw->pdev, re, sky2->rx_data_size)) { -dev_kfree_skb (re->skb); - re->skb =NULL ; - goto nomem; - } -sky2_rx_submit (sky2, re); }
@@ -1453,14 +1441,12 @@ static int sky2_rx_start(struct sky2_port *sky2)return 0 ; -nomem: -sky2_rx_clean (sky2); -return -ENOMEM ; }static int sky2_alloc_buffers (struct sky2_port *sky2) {struct sky2_hw *hw = sky2->hw; +unsigned i;/* must be power of 2 */ sky2->tx_le = pci_alloc_consistent(hw->pdev,
@@ -1486,6 +1472,24 @@ static int sky2_alloc_buffers(struct sky2_port *sky2)if (!sky2->rx_ring) goto nomem; + sky2->rx_data_size =sky2_get_rx_data_size (sky2); + +/* Fill Rx ring */ +for (i =0 ; i < sky2->rx_pending; i++) { +struct rx_ring_info *re = sky2->rx_ring + i; + + re->skb =sky2_rx_alloc (sky2); +if (!re->skb) + goto nomem; + +if (sky2_rx_map_skb (hw->pdev, re, sky2->rx_data_size)) { +dev_kfree_skb (re->skb); + re->skb =NULL ; + goto nomem; + } + } + +return 0 ; nomem:return -ENOMEM ;
@@ -1495,6 +1499,8 @@ static void sky2_free_buffers(struct sky2_port *sky2) {struct sky2_hw *hw = sky2->hw; +sky2_rx_clean (sky2); +if (sky2->rx_le) {pci_free_consistent (hw->pdev,RX_LE_BYTES , sky2->rx_le, sky2->rx_le_map);
@@ -1953,8 +1959,6 @@ static int sky2_down(struct net_device *dev)/* Free any pending frames stuck in HW queue */ sky2_tx_complete (sky2, sky2->tx_prod); -sky2_rx_clean (sky2); -sky2_free_buffers (sky2);return 0 ;
Signature · 1 line
1.5.6.5
Quoted 6 lines · On Thu, 28 Jan 2010 00:05:44 +0900 Mike McCormack <[email protected]> wrote:
Allocating everything in one place means there's a single point of failure in
sky2_up, andsky2_rx_startcan no longer fail.This also fixes a memory leak in the case that
sky2_rx_startfails in the middle of allocating skbs, since any allocated skbs will not be free'd in sky2_up's failure path.
I may incorporate the cleanup, but there is no leak. if rx_start fails, it already called rx_clean.