| Message ID | 1375118644.10515.18.camel@edumazet-glaptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Eric Dumazet <eric.dumazet@gmail.com> writes: > From: Eric Dumazet <edumazet@google.com> > > On Mon, 2013-07-29 at 08:30 -0700, Eric Dumazet wrote: >> On Mon, 2013-07-29 at 13:09 +0100, Luis Henriques wrote: >> >> > >> > I confirm that I can't reproduce the issue using this patch. >> > >> >> Thanks, I'll send a polished patch, as this one had an error if >> build_skb() returns NULL (in case sk_buff allocation fails) > > Please try the following patch : It should use 2K frags instead of 4K > for normal 1500 mtu This patch seems to work fine as well -- I'm unable to reproduce the issue. Cheers,
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 29 Jul 2013 10:24:04 -0700 > [PATCH] atl1c: use custom skb allocator > > We had reports ( https://bugzilla.kernel.org/show_bug.cgi?id=54021 ) > that using high order pages for skb allocations is problematic for atl1c > > We do not know exactly what the problem is, but we suspect that crossing > 4K pages is not well supported by this hardware. > > Use a custom allocator, using page allocator and 2K fragments for > optimal stack behavior. We might make this allocator generic > in future kernels. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Theoretically we'll still potentially hit this with > PAGE_SIZE MTUs.... But whatever, this is a step in the positive direction so applied and queued up for -stable, thanks Eric! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2013/07/30 19:11, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 29 Jul 2013 10:24:04 -0700 > > > [PATCH] atl1c: use custom skb allocator > > BTW, this didn't end up in git with the intended patch subject. The extra text prepended in the email also made it to the log. 7b70176 atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-07-31 at 13:48 -0400, Benjamin Poirier wrote: > On 2013/07/30 19:11, David Miller wrote: > > From: Eric Dumazet <eric.dumazet@gmail.com> > > Date: Mon, 29 Jul 2013 10:24:04 -0700 > > > > > [PATCH] atl1c: use custom skb allocator > > > > > BTW, this didn't end up in git with the intended patch subject. The > extra text prepended in the email also made it to the log. > > 7b70176 atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring True, but its not a problem, as long as the bug is fixed ;) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Benjamin Poirier <benjamin.poirier@gmail.com> Date: Wed, 31 Jul 2013 13:48:54 -0400 > On 2013/07/30 19:11, David Miller wrote: >> From: Eric Dumazet <eric.dumazet@gmail.com> >> Date: Mon, 29 Jul 2013 10:24:04 -0700 >> >> > [PATCH] atl1c: use custom skb allocator >> > > > BTW, this didn't end up in git with the intended patch subject. The > extra text prepended in the email also made it to the log. > > 7b70176 atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring I know, I realized this when I sent the pull request to Linus. Eric, your patch submission format is extremely error prone for me sometime. When you submit patches in email thread, and do things like you did here, it causes problems sometimes. Really just send it in the usual way, with the real Subject line matching the commit message header line you wish to use etc. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-07-31 at 12:01 -0700, David Miller wrote: > I know, I realized this when I sent the pull request to Linus. > > Eric, your patch submission format is extremely error prone for me > sometime. When you submit patches in email thread, and do things like > you did here, it causes problems sometimes. > > Really just send it in the usual way, with the real Subject line > matching the commit message header line you wish to use etc. > Sorry for that, I'll do this next times ! Thanks ! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c.h b/drivers/net/ethernet/atheros/atl1c/atl1c.h index b2bf324..0f05565 100644 --- a/drivers/net/ethernet/atheros/atl1c/atl1c.h +++ b/drivers/net/ethernet/atheros/atl1c/atl1c.h @@ -520,6 +520,9 @@ struct atl1c_adapter { struct net_device *netdev; struct pci_dev *pdev; struct napi_struct napi; + struct page *rx_page; + unsigned int rx_page_offset; + unsigned int rx_frag_size; struct atl1c_hw hw; struct atl1c_hw_stats hw_stats; struct mii_if_info mii; /* MII interface info */ diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c index 786a874..a36a760 100644 --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c @@ -481,10 +481,15 @@ static int atl1c_set_mac_addr(struct net_device *netdev, void *p) static void atl1c_set_rxbufsize(struct atl1c_adapter *adapter, struct net_device *dev) { + unsigned int head_size; int mtu = dev->mtu; adapter->rx_buffer_len = mtu > AT_RX_BUF_SIZE ? roundup(mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN, 8) : AT_RX_BUF_SIZE; + + head_size = SKB_DATA_ALIGN(adapter->rx_buffer_len + NET_SKB_PAD) + + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); + adapter->rx_frag_size = roundup_pow_of_two(head_size); } static netdev_features_t atl1c_fix_features(struct net_device *netdev, @@ -952,6 +957,10 @@ static void atl1c_free_ring_resources(struct atl1c_adapter *adapter) kfree(adapter->tpd_ring[0].buffer_info); adapter->tpd_ring[0].buffer_info = NULL; } + if (adapter->rx_page) { + put_page(adapter->rx_page); + adapter->rx_page = NULL; + } } /** @@ -1639,6 +1648,35 @@ static inline void atl1c_rx_checksum(struct atl1c_adapter *adapter, skb_checksum_none_assert(skb); } +static struct sk_buff *atl1c_alloc_skb(struct atl1c_adapter *adapter) +{ + struct sk_buff *skb; + struct page *page; + + if (adapter->rx_frag_size > PAGE_SIZE) + return netdev_alloc_skb(adapter->netdev, + adapter->rx_buffer_len); + + page = adapter->rx_page; + if (!page) { + adapter->rx_page = page = alloc_page(GFP_ATOMIC); + if (unlikely(!page)) + return NULL; + adapter->rx_page_offset = 0; + } + + skb = build_skb(page_address(page) + adapter->rx_page_offset, + adapter->rx_frag_size); + if (likely(skb)) { + adapter->rx_page_offset += adapter->rx_frag_size; + if (adapter->rx_page_offset >= PAGE_SIZE) + adapter->rx_page = NULL; + else + get_page(page); + } + return skb; +} + static int atl1c_alloc_rx_buffer(struct atl1c_adapter *adapter) { struct atl1c_rfd_ring *rfd_ring = &adapter->rfd_ring; @@ -1660,7 +1698,7 @@ static int atl1c_alloc_rx_buffer(struct atl1c_adapter *adapter) while (next_info->flags & ATL1C_BUFFER_FREE) { rfd_desc = ATL1C_RFD_DESC(rfd_ring, rfd_next_to_use); - skb = netdev_alloc_skb(adapter->netdev, adapter->rx_buffer_len); + skb = atl1c_alloc_skb(adapter); if (unlikely(!skb)) { if (netif_msg_rx_err(adapter)) dev_warn(&pdev->dev, "alloc rx buffer failed\n");