netdev - RE: [E1000-devel] [e100] Page allocation failure warning(?) in 2.6.36.3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1294856052.3981.125.camel@edumazet-laptop>
Date:	Wed, 12 Jan 2011 19:14:12 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>
Cc:	David Miller <davem@...emloft.net>,
	Chris Rankin <rankincj@...oo.com>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Dave, Tushar N" <tushar.n.dave@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Subject: RE: [E1000-devel] [e100] Page allocation failure warning(?) in
 2.6.36.3

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@...oo.com>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@...el.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;
 		}


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html