| Message ID | 1294853710.3981.108.camel@edumazet-laptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
--- On Wed, 12/1/11, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Apparently e100 driver uses GFP_ATOMIC allocations in setup > phase, and your machine doesnt have enough memory. Thanks. You'd think 64 MB of RAM would be enough... ;-). Cheers, Chris -- 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
Le mercredi 12 janvier 2011 à 09:42 -0800, Chris Rankin a écrit : > --- On Wed, 12/1/11, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > Apparently e100 driver uses GFP_ATOMIC allocations in setup > > phase, and your machine doesnt have enough memory. > > Thanks. You'd think 64 MB of RAM would be enough... ;-). I remember using linux on a 8MB machine a loooong time ago, but not a 2.6 kernel :) -- 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
Hi Eric, thanks for doing this work! On Wed, 12 Jan 2011, Eric Dumazet wrote: > Apparently e100 driver uses GFP_ATOMIC allocations in setup phase, and > your machine doesnt have enough memory. This part looks great. > I would try following patch, allowing the use of GFP_KERNEL at init > time, to let vm games play. > > Thanks > > [PATCH] e100: use GFP_KERNEL allocations at device init stage > > In lowmem conditions, e100 driver can fail its initialization, because > of GFP_ATOMIC abuse. > > Switch to GFP_KERNEL were applicable. > > Add __GFP_NOWARN flag to GFP_ATOMIC allocations, since driver can cope > with failed allocations (if setup succeeded) > > Reported-by: Chris Rankin <rankincj@yahoo.com> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > - if (unlikely(e100_rx_alloc_skb(nic, rx))) > + if (unlikely(e100_rx_alloc_skb(nic, rx, > + GFP_ATOMIC | __GFP_NOWARN))) First, I don't think the following comment should hold up this patch. As a policy question when I asked about using __GFP_NOWARN before in other Intel ethernet drivers the consensus seemed to be that the warning messages were useful. All our drivers correctly handle runtime memory failures, but none of them are currently using __GFP_NOWARN. Can I submit patches to change our other drivers to __GFP_NOWARN? I think it will make for quite a few less reports of non-issues to the list. All our drivers that I would be patching already have ethtool counters that count failed allocations. -- 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/e100.c b/drivers/net/e100.c index b0aa9e6..e4d8a70 100644 --- a/drivers/net/e100.c +++ b/drivers/net/e100.c @@ -1880,9 +1880,21 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx) } #define RFD_BUF_LEN (sizeof(struct rfd) + VLAN_ETH_FRAME_LEN) -static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx) + +static struct sk_buff *e100_alloc_skb(struct net_device *dev, gfp_t flags) +{ + struct sk_buff *skb; + + skb = __netdev_alloc_skb(dev, RFD_BUF_LEN + NET_IP_ALIGN, flags); + if (NET_IP_ALIGN && skb) + skb_reserve(skb, NET_IP_ALIGN); + return skb; +} + +static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx, gfp_t flags) { - if (!(rx->skb = netdev_alloc_skb_ip_align(nic->netdev, RFD_BUF_LEN))) + rx->skb = e100_alloc_skb(nic->netdev, flags); + if (!rx->skb) return -ENOMEM; /* Init, and map the RFD. */ @@ -2026,7 +2038,8 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done, /* Alloc new skbs to refill list */ for (rx = nic->rx_to_use; !rx->skb; rx = nic->rx_to_use = rx->next) { - if (unlikely(e100_rx_alloc_skb(nic, rx))) + if (unlikely(e100_rx_alloc_skb(nic, rx, + GFP_ATOMIC | __GFP_NOWARN))) break; /* Better luck next time (see watchdog) */ } @@ -2102,13 +2115,13 @@ static int e100_rx_alloc_list(struct nic *nic) nic->rx_to_use = nic->rx_to_clean = NULL; nic->ru_running = RU_UNINITIALIZED; - if (!(nic->rxs = kcalloc(count, sizeof(struct rx), GFP_ATOMIC))) + if (!(nic->rxs = kcalloc(count, sizeof(struct rx), GFP_KERNEL))) return -ENOMEM; for (rx = nic->rxs, i = 0; i < count; rx++, i++) { rx->next = (i + 1 < count) ? rx + 1 : nic->rxs; rx->prev = (i == 0) ? nic->rxs + count - 1 : rx - 1; - if (e100_rx_alloc_skb(nic, rx)) { + if (e100_rx_alloc_skb(nic, rx, GFP_KERNEL)) { e100_rx_clean_list(nic); return -ENOMEM; }