From: Eric Dumazet <eric.dumazet@gmail.com>
To: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Cc: David Miller <davem@davemloft.net>,
Chris Rankin <rankincj@yahoo.com>,
"e1000-devel@lists.sourceforge.net"
<e1000-devel@lists.sourceforge.net>,
"Dave, Tushar N" <tushar.n.dave@intel.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel.com>
Subject: RE: [E1000-devel] [e100] Page allocation failure warning(?) in 2.6.36.3
Date: Wed, 12 Jan 2011 19:14:12 +0100 [thread overview]
Message-ID: <1294856052.3981.125.camel@edumazet-laptop> (raw)
In-Reply-To: <alpine.WNT.2.00.1101120958080.5816@JBRANDEB-DESK2.amr.corp.intel.com>
Le mercredi 12 janvier 2011 à 10:05 -0800, Brandeburg, Jesse a écrit :
> 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.
>
If an allocation failure is really handled, in the sense NIC doesnt
freeze but only lose one incoming frame, then probably yes.
I think the warning message can be useful when driver is known to let
things in a non working state ;)
As you said, this can be done later, here is a respin without this bit.
Thanks !
[PATCH v2] 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.
Reported-by: Chris Rankin <rankincj@yahoo.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/e100.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index b0aa9e6..c9a2126 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,7 @@ 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)))
break; /* Better luck next time (see watchdog) */
}
@@ -2102,13 +2114,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;
}
next prev parent reply other threads:[~2011-01-12 18:14 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-08 12:53 [e100] Page allocation failure warning(?) in 2.6.36.3 Chris Rankin
2011-01-10 23:11 ` [E1000-devel] " Dave, Tushar N
2011-01-10 23:41 ` Chris Rankin
2011-01-11 0:37 ` Dave, Tushar N
2011-01-11 8:52 ` [E1000-devel] " Chris Rankin
2011-01-11 20:59 ` Chris Rankin
2011-01-12 17:35 ` Eric Dumazet
2011-01-12 17:42 ` [E1000-devel] " Chris Rankin
2011-01-12 17:48 ` Eric Dumazet
2011-01-12 18:05 ` Brandeburg, Jesse
2011-01-12 18:14 ` Eric Dumazet [this message]
2011-01-12 23:27 ` Chris Rankin
2011-01-13 4:55 ` Eric Dumazet
2011-01-13 9:00 ` Chris Rankin
2011-01-13 9:05 ` Eric Dumazet
2011-01-13 9:24 ` Chris Rankin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1294856052.3981.125.camel@edumazet-laptop \
--to=eric.dumazet@gmail.com \
--cc=davem@davemloft.net \
--cc=e1000-devel@lists.sourceforge.net \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=netdev@vger.kernel.org \
--cc=rankincj@yahoo.com \
--cc=tushar.n.dave@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox