xen-netback: allocate xenvif arrays using vzalloc.

Quoted 6 lines · Joby Poriyath

This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.

This looks overkill.

Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...

This should be done generically in alloc_netdev_mqs()

Take a look at commit 60877a32bce00041
("net: allow large number of tx queues")

Wei Liu

2013-10-29 15:50 UTC #22 lore raw

On Tue, Oct 29, 2013 at 03:27:13PM +0000, Joby Poriyath wrote:
[...]

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 315

Quoted 8 lines

drivers/net/xen-netback/interface.c +8 -0

+
+static int allocate_xenvif_arrays(struct xenvif *vif)
+{
+	vif->mmap_pages = vif->pending_tx_info = NULL;
+	vif->tx_copy_ops = vif->grant_copy_op = vif->meta = NULL;
+
+	vif->mmap_pages = vzalloc(MAX_PENDING_REQS * sizeof(struct page *));
+	if (! vif->mmap_pages)

No space after "!".

Quoted 9 lines

drivers/net/xen-netback/interface.c +9 -0

+		goto fail;
+
+	vif->pending_tx_info = vzalloc(MAX_PENDING_REQS * 
+					sizeof(struct pending_tx_info));
+	if (! vif->pending_tx_info)
+		goto fail;
+
+	vif->tx_copy_ops = vzalloc(2 * MAX_PENDING_REQS *
+					sizeof(struct gnttab_copy));

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 334

Quoted 10 lines

drivers/net/xen-netback/interface.c +10 -0

+	if (! vif->tx_copy_ops)
+		goto fail;
+
+	vif->grant_copy_op = vzalloc(2 * XEN_NETIF_RX_RING_SIZE *
+					sizeof(struct gnttab_copy));
+	if (! vif->grant_copy_op)
+		goto fail;
+
+	vif->meta = vzalloc(2 * XEN_NETIF_RX_RING_SIZE *
+					sizeof(struct xenvif_rx_meta));

Indentation.

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 342

Quoted 8 lines

drivers/net/xen-netback/interface.c +8 -0

+	if (! vif->meta)
+		goto fail;
+
+	return 0;
+
+fail:
+	deallocate_xenvif_arrays(vif);
+	return 1;

return -ENOMEM;

Quoted 32 lines

drivers/net/xen-netback/interface.c +9 -0

+}
+
 struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 			    unsigned int handle)
 {

@@ -313,6 +367,12 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	vif->ip_csum = 1;
 	vif->dev = dev;
 
+	if (allocate_xenvif_arrays(vif)) {
+		netdev_warn(dev, "Could not create device: out of memory\n");
+		free_netdev(dev);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	vif->credit_bytes = vif->remaining_credit = ~0UL;
 	vif->credit_usec  = 0UL;
 	init_timer(&vif->credit_timeout);

@@ -484,6 +544,7 @@ void xenvif_free(struct xenvif *vif)
 
 	unregister_netdev(vif->dev);
 
+	deallocate_xenvif_arrays(vif);
 	free_netdev(vif->dev);
 
 	module_put(THIS_MODULE);

drivers/net/xen-netback/netback.c +0 -0

index 828fdab..34c0c05 100644
@@ -602,12 +602,12 @@ void xenvif_rx_action(struct xenvif *vif)
 			break;

drivers/net/xen-netback/netback.c void xenvif_rx_action(struct xenvif *vif) · line 610

Quoted 10 lines

drivers/net/xen-netback/netback.c +2 -2

 	}
 
-	BUG_ON(npo.meta_prod > ARRAY_SIZE(vif->meta));
+	BUG_ON(npo.meta_prod > 2*XEN_NETIF_RX_RING_SIZE);
 
 	if (!npo.copy_prod)
 		return;
 
-	BUG_ON(npo.copy_prod > ARRAY_SIZE(vif->grant_copy_op));
+	BUG_ON(npo.copy_prod > 2*XEN_NETIF_RX_RING_SIZE);

It's better to

  #define XEN_NETBK_RX_ARRAY_SIZE (2*XEN_NETIF_RX_RING_SIZE)

And use it consistently in code (allocation / comparison). Otherwise it's easy to change one site and forget about others.

drivers/net/xen-netback/netback.c static unsigned xenvif_tx_build_gops(struct xenvif *vif) · line 1574

Quoted 9 lines

drivers/net/xen-netback/netback.c +1 -1

 	gnttab_batch_copy(vif->grant_copy_op, npo.copy_prod);
 
 	while ((skb = __skb_dequeue(&rxq)) != NULL) {

@@ -1571,7 +1571,7 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif)
 
 		vif->tx.req_cons = idx;
 
-		if ((gop-vif->tx_copy_ops) >= ARRAY_SIZE(vif->tx_copy_ops))
+		if ((gop-vif->tx_copy_ops) >= 2*MAX_PENDING_REQS)

Same here.

  #define XEN_NETBK_TX_ARRAY_SIZE

Wei.

Wei Liu

2013-10-29 15:50 UTC #23 lore raw

On Tue, Oct 29, 2013 at 03:27:13PM +0000, Joby Poriyath wrote:
[...]

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 315

Quoted 8 lines

drivers/net/xen-netback/interface.c +8 -0

+
+static int allocate_xenvif_arrays(struct xenvif *vif)
+{
+	vif->mmap_pages = vif->pending_tx_info = NULL;
+	vif->tx_copy_ops = vif->grant_copy_op = vif->meta = NULL;
+
+	vif->mmap_pages = vzalloc(MAX_PENDING_REQS * sizeof(struct page *));
+	if (! vif->mmap_pages)

No space after "!".

Quoted 9 lines

drivers/net/xen-netback/interface.c +9 -0

+		goto fail;
+
+	vif->pending_tx_info = vzalloc(MAX_PENDING_REQS * 
+					sizeof(struct pending_tx_info));
+	if (! vif->pending_tx_info)
+		goto fail;
+
+	vif->tx_copy_ops = vzalloc(2 * MAX_PENDING_REQS *
+					sizeof(struct gnttab_copy));

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 334

Quoted 10 lines

drivers/net/xen-netback/interface.c +10 -0

+	if (! vif->tx_copy_ops)
+		goto fail;
+
+	vif->grant_copy_op = vzalloc(2 * XEN_NETIF_RX_RING_SIZE *
+					sizeof(struct gnttab_copy));
+	if (! vif->grant_copy_op)
+		goto fail;
+
+	vif->meta = vzalloc(2 * XEN_NETIF_RX_RING_SIZE *
+					sizeof(struct xenvif_rx_meta));

Indentation.

drivers/net/xen-netback/interface.c static const struct net_device_ops xenvif_netdev_ops = { · line 342

Quoted 8 lines

drivers/net/xen-netback/interface.c +8 -0

+	if (! vif->meta)
+		goto fail;
+
+	return 0;
+
+fail:
+	deallocate_xenvif_arrays(vif);
+	return 1;

return -ENOMEM;

Quoted 32 lines

drivers/net/xen-netback/interface.c +9 -0

+}
+
 struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 			    unsigned int handle)
 {

@@ -313,6 +367,12 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	vif->ip_csum = 1;
 	vif->dev = dev;
 
+	if (allocate_xenvif_arrays(vif)) {
+		netdev_warn(dev, "Could not create device: out of memory\n");
+		free_netdev(dev);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	vif->credit_bytes = vif->remaining_credit = ~0UL;
 	vif->credit_usec  = 0UL;
 	init_timer(&vif->credit_timeout);

@@ -484,6 +544,7 @@ void xenvif_free(struct xenvif *vif)
 
 	unregister_netdev(vif->dev);
 
+	deallocate_xenvif_arrays(vif);
 	free_netdev(vif->dev);
 
 	module_put(THIS_MODULE);

drivers/net/xen-netback/netback.c +0 -0

index 828fdab..34c0c05 100644
@@ -602,12 +602,12 @@ void xenvif_rx_action(struct xenvif *vif)
 			break;

drivers/net/xen-netback/netback.c void xenvif_rx_action(struct xenvif *vif) · line 610

Quoted 10 lines

drivers/net/xen-netback/netback.c +2 -2

 	}
 
-	BUG_ON(npo.meta_prod > ARRAY_SIZE(vif->meta));
+	BUG_ON(npo.meta_prod > 2*XEN_NETIF_RX_RING_SIZE);
 
 	if (!npo.copy_prod)
 		return;
 
-	BUG_ON(npo.copy_prod > ARRAY_SIZE(vif->grant_copy_op));
+	BUG_ON(npo.copy_prod > 2*XEN_NETIF_RX_RING_SIZE);

It's better to

  #define XEN_NETBK_RX_ARRAY_SIZE (2*XEN_NETIF_RX_RING_SIZE)

And use it consistently in code (allocation / comparison). Otherwise it's easy to change one site and forget about others.

drivers/net/xen-netback/netback.c static unsigned xenvif_tx_build_gops(struct xenvif *vif) · line 1574

Quoted 9 lines

drivers/net/xen-netback/netback.c +1 -1

 	gnttab_batch_copy(vif->grant_copy_op, npo.copy_prod);
 
 	while ((skb = __skb_dequeue(&rxq)) != NULL) {

@@ -1571,7 +1571,7 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif)
 
 		vif->tx.req_cons = idx;
 
-		if ((gop-vif->tx_copy_ops) >= ARRAY_SIZE(vif->tx_copy_ops))
+		if ((gop-vif->tx_copy_ops) >= 2*MAX_PENDING_REQS)

Same here.

  #define XEN_NETBK_TX_ARRAY_SIZE

Wei.

Joby Poriyath

author

2013-10-29 18:46 UTC #3 lore raw

Quoted 8 lines · Eric Dumazet

Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.

Quoted 6 lines

This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...
This should be done generically in alloc_netdev_mqs()

Sorry Eric, I didn't quite understand how this can be generically done.

The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.

Quoted 4 lines

Take a look at commit 60877a32bce00041
("net: allow large number of tx queues")

I could try allocating using kmalloc and if that fails, then fall back to vmalloc.

Thanks,
Joby

Joby Poriyath

author

2013-10-29 18:47 UTC #4 lore raw

Quoted 8 lines · Eric Dumazet

Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.

Quoted 6 lines

This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...
This should be done generically in alloc_netdev_mqs()

Sorry Eric, I didn't quite understand how this can be generically done.

The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.

Quoted 4 lines

Take a look at commit 60877a32bce00041
("net: allow large number of tx queues")

I could try allocating using kmalloc and if that fails, then fall back to vmalloc.

Thanks,
Joby

Eric Dumazet

2013-10-29 23:24 UTC #5 lore raw

Quoted 14 lines · Joby Poriyath

Quoted 13 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...

Quoted 6 lines

Quoted 1 line
This should be done generically in alloc_netdev_mqs()
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.

They are created by alloc_netdev_mqs()

Quoted 8 lines

Quoted 5 lines
Take a look at commit 60877a32bce00041
("net: allow large number of tx queues")
I could try allocating using kmalloc and if that fails, then fall back to vmalloc.

My point is the patch should be done in net/core, not in your driver.

Eric Dumazet

2013-10-29 23:24 UTC #6 lore raw

Quoted 14 lines · Joby Poriyath

Quoted 13 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...

Quoted 6 lines

Quoted 1 line
This should be done generically in alloc_netdev_mqs()
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.

They are created by alloc_netdev_mqs()

Quoted 8 lines

Quoted 5 lines
Take a look at commit 60877a32bce00041
("net: allow large number of tx queues")
I could try allocating using kmalloc and if that fails, then fall back to vmalloc.

My point is the patch should be done in net/core, not in your driver.

Eric Dumazet

2013-10-29 23:32 UTC #7 lore raw

Quoted 15 lines · Eric Dumazet

Quoted 14 lines · Joby Poriyath
Quoted 13 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
This should be done generically in alloc_netdev_mqs()
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.
They are created by alloc_netdev_mqs()

Something like the following should be fine.

net/core/dev.c +13 -3

index 0054c8c..874a57a 100644
@@ -6239,7 +6239,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6302,7 +6304,10 @@ free_pcpu:
 #endif
 
 free_p:
-	kfree(p);
+	if (is_vmalloc_addr(p))
+		vfree(p);
+	else
+		kfree(p);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6344,12 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		char *addr = (char *)dev - dev->padded;
+
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
 		return;
 	}

net/core/net-sysfs.c +6 -1

index d954b56..406c54b 100644
@@ -1259,11 +1259,16 @@ exit:
 static void netdev_release(struct device *d)
 {
 	struct net_device *dev = to_net_dev(d);
+	char *addr;
 
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	addr = (char *)dev - dev->padded;
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
 }
 
 static const void *net_namespace(struct device *d)

Eric Dumazet

2013-10-29 23:32 UTC #8 lore raw

Quoted 15 lines · Eric Dumazet

Quoted 14 lines · Joby Poriyath
Quoted 13 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
This should be done generically in alloc_netdev_mqs()
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.
They are created by alloc_netdev_mqs()

Something like the following should be fine.

net/core/dev.c +13 -3

index 0054c8c..874a57a 100644
@@ -6239,7 +6239,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6302,7 +6304,10 @@ free_pcpu:
 #endif
 
 free_p:
-	kfree(p);
+	if (is_vmalloc_addr(p))
+		vfree(p);
+	else
+		kfree(p);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6344,12 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		char *addr = (char *)dev - dev->padded;
+
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
 		return;
 	}

net/core/net-sysfs.c +6 -1

index d954b56..406c54b 100644
@@ -1259,11 +1259,16 @@ exit:
 static void netdev_release(struct device *d)
 {
 	struct net_device *dev = to_net_dev(d);
+	char *addr;
 
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	addr = (char *)dev - dev->padded;
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
 }
 
 static const void *net_namespace(struct device *d)

Joby Poriyath

author

2013-10-30 10:39 UTC #9 lore raw

Quoted 18 lines · Eric Dumazet

Quoted 17 lines · Eric Dumazet
Quoted 16 lines · Joby Poriyath
Quoted 14 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...
This should be done generically in alloc_netdev_mqs()

Quoted 11 lines

Quoted 6 lines
Quoted 4 lines
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.
They are created by alloc_netdev_mqs()
Something like the following should be fine.

Thanks for the patch.

Quoted 5 lines

net/core/dev.c +0 -0
index 0054c8c..874a57a 100644

net/core/dev.c struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, · line 6247

Quoted 10 lines

net/core/dev.c +3 -1

@@ -6239,7 +6239,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

The net_device allocation rule {linux/Documentation/networking/netdevices.txt} states that net_device struct must be allocated using kmalloc.

Is this safe to do?

Quoted 40 lines

net/core/dev.c +10 -2

@@ -6302,7 +6304,10 @@ free_pcpu:
 #endif
 
 free_p:
-	kfree(p);
+	if (is_vmalloc_addr(p))
+		vfree(p);
+	else
+		kfree(p);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6344,12 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		char *addr = (char *)dev - dev->padded;
+
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
 		return;
 	}

net/core/net-sysfs.c +1 -0

index d954b56..406c54b 100644
@@ -1259,11 +1259,16 @@ exit:
 static void netdev_release(struct device *d)
 {
 	struct net_device *dev = to_net_dev(d);
+	char *addr;
 
 	BUG_ON(dev->reg_state != NETREG_RELEASED);

net/core/net-sysfs.c exit: · line 1274

Quoted 10 lines

net/core/net-sysfs.c +5 -1

 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	addr = (char *)dev - dev->padded;
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
 }
 
 static const void *net_namespace(struct device *d)

Thanks,
Joby

Joby Poriyath

author

2013-10-30 10:40 UTC #10 lore raw

Quoted 18 lines · Eric Dumazet

Quoted 17 lines · Eric Dumazet
Quoted 16 lines · Joby Poriyath
Quoted 14 lines · Eric Dumazet
Quoted 6 lines · Joby Poriyath
This will reduce memory pressure when allocating struct xenvif.
The size of xenvif struct has increased from 168 to 36632 bytes (on x86-32). See commit b3f980bd827e6e81a050c518d60ed7811a83061d. This resulted in occasional netdev allocation failure in dom0 with 752MiB RAM, due to fragmented memory.
This looks overkill.
Replacing a single allocation of ~36 KB into 5 vmalloc() looks like you did not really tried other things...
This should be done generically in alloc_netdev_mqs()

Quoted 11 lines

Quoted 6 lines
Quoted 4 lines
Sorry Eric, I didn't quite understand how this can be generically done.
The netback interfaces are tied to the Xen guests (VMs) and these are created when guests are started and deleted when guest are halted.
They are created by alloc_netdev_mqs()
Something like the following should be fine.

Thanks for the patch.

Quoted 5 lines

net/core/dev.c +0 -0
index 0054c8c..874a57a 100644

net/core/dev.c struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, · line 6247

Quoted 10 lines

net/core/dev.c +3 -1

@@ -6239,7 +6239,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

The net_device allocation rule {linux/Documentation/networking/netdevices.txt} states that net_device struct must be allocated using kmalloc.

Is this safe to do?

Quoted 40 lines

net/core/dev.c +10 -2

@@ -6302,7 +6304,10 @@ free_pcpu:
 #endif
 
 free_p:
-	kfree(p);
+	if (is_vmalloc_addr(p))
+		vfree(p);
+	else
+		kfree(p);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6344,12 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		char *addr = (char *)dev - dev->padded;
+
+		if (is_vmalloc_addr(addr))
+			vfree(addr);
+		else
+			kfree(addr);
 		return;
 	}

net/core/net-sysfs.c +1 -0

index d954b56..406c54b 100644
@@ -1259,11 +1259,16 @@ exit:
 static void netdev_release(struct device *d)
 {
 	struct net_device *dev = to_net_dev(d);
+	char *addr;
 
 	BUG_ON(dev->reg_state != NETREG_RELEASED);

net/core/net-sysfs.c exit: · line 1274

Quoted 10 lines

net/core/net-sysfs.c +5 -1

 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	addr = (char *)dev - dev->padded;
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
 }
 
 static const void *net_namespace(struct device *d)

Thanks,
Joby

Eric Dumazet

2013-10-30 13:54 UTC #11 lore raw

Quoted 4 lines · Joby Poriyath

The net_device allocation rule {linux/Documentation/networking/netdevices.txt} states that net_device struct must be allocated using kmalloc.
Is this safe to do?

As long as the freeing path is aware of the possibility the memory is either allocated with vmalloc() or kmalloc(), we are safe.

Eric Dumazet

2013-10-30 15:01 UTC #12 lore raw

PATCH 1/1 net: extend net_device allocation to vmalloc()

From: Eric Dumazet <[email protected]>

Joby Poriyath provided a xen-netback patch to reduce the size of xenvif structure as some netdev allocation could fail under memory pressure/fragmentation.

This patch is handling the problem at the core level, allowing any netdev structures to use vmalloc() if kmalloc() failed.

As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT to kzalloc() flags to do this fallback only when really needed.

---
 Documentation/networking/netdevices.txt |    7 ++++---
 include/linux/netdevice.h               |    1 +
 net/core/dev.c                          |   22 +++++++++++++++++-----
 net/core/net-sysfs.c                    |    2 +-
 4 files changed, 23 insertions(+), 9 deletions(-)

Documentation/networking/netdevices.txt +4 -3

index c7ecc70..df7cd19 100644
@@ -10,9 +10,10 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with kmalloc() or vmalloc(). If device has registered
+successfully, it will be freed on last use by free_netdev.
+This is required to handle the pathologic case cleanly
+(example: rmmod mydriver </sys/class/net/myeth/mtu )
 
 There are routines in net_init.c to handle the common cases of
 alloc_etherdev, alloc_netdev.  These reserve extra space for driver

include/linux/netdevice.h +1 -0

index cb1d918..e6353ca 100644
@@ -1800,6 +1800,7 @@ static inline void unregister_netdevice(struct net_device *dev)
 
 int netdev_refcnt_read(const struct net_device *dev);
 void free_netdev(struct net_device *dev);
+void netdev_freemem(struct net_device *dev);
 void synchronize_net(void);
 int init_dummy_netdev(struct net_device *dev);

net/core/dev.c +17 -5

index 0054c8c..0e61365 100644
@@ -6196,6 +6196,16 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
 
+void netdev_freemem(struct net_device *dev)
+{
+	char *addr = (char *)dev - dev->padded;
+
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+
 /**
  *	alloc_netdev_mqs - allocate network device
  *	@sizeof_priv:	size of private data to allocate space for

@@ -6239,7 +6249,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6248,7 +6260,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
-		goto free_p;
+		goto free_dev;
 
 	if (dev_addr_init(dev))
 		goto free_pcpu;

@@ -6301,8 +6313,8 @@ free_pcpu:
 	kfree(dev->_rx);
 #endif
 
-free_p:
-	kfree(p);
+free_dev:
+	netdev_freemem(dev);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6351,7 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		netdev_freemem(dev);
 		return;
 	}

net/core/net-sysfs.c +1 -1

index d954b56..d03f2c9 100644
@@ -1263,7 +1263,7 @@ static void netdev_release(struct device *d)
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	netdev_freemem(dev);
 }
 
 static const void *net_namespace(struct device *d)

Eric Dumazet

2013-10-30 15:01 UTC #13 lore raw

PATCH 1/1 net: extend net_device allocation to vmalloc()

From: Eric Dumazet <[email protected]>

Joby Poriyath provided a xen-netback patch to reduce the size of xenvif structure as some netdev allocation could fail under memory pressure/fragmentation.

This patch is handling the problem at the core level, allowing any netdev structures to use vmalloc() if kmalloc() failed.

As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT to kzalloc() flags to do this fallback only when really needed.

---
 Documentation/networking/netdevices.txt |    7 ++++---
 include/linux/netdevice.h               |    1 +
 net/core/dev.c                          |   22 +++++++++++++++++-----
 net/core/net-sysfs.c                    |    2 +-
 4 files changed, 23 insertions(+), 9 deletions(-)

Documentation/networking/netdevices.txt +4 -3

index c7ecc70..df7cd19 100644
@@ -10,9 +10,10 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with kmalloc() or vmalloc(). If device has registered
+successfully, it will be freed on last use by free_netdev.
+This is required to handle the pathologic case cleanly
+(example: rmmod mydriver </sys/class/net/myeth/mtu )
 
 There are routines in net_init.c to handle the common cases of
 alloc_etherdev, alloc_netdev.  These reserve extra space for driver

include/linux/netdevice.h +1 -0

index cb1d918..e6353ca 100644
@@ -1800,6 +1800,7 @@ static inline void unregister_netdevice(struct net_device *dev)
 
 int netdev_refcnt_read(const struct net_device *dev);
 void free_netdev(struct net_device *dev);
+void netdev_freemem(struct net_device *dev);
 void synchronize_net(void);
 int init_dummy_netdev(struct net_device *dev);

net/core/dev.c +17 -5

index 0054c8c..0e61365 100644
@@ -6196,6 +6196,16 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
 
+void netdev_freemem(struct net_device *dev)
+{
+	char *addr = (char *)dev - dev->padded;
+
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+
 /**
  *	alloc_netdev_mqs - allocate network device
  *	@sizeof_priv:	size of private data to allocate space for

@@ -6239,7 +6249,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6248,7 +6260,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
-		goto free_p;
+		goto free_dev;
 
 	if (dev_addr_init(dev))
 		goto free_pcpu;

@@ -6301,8 +6313,8 @@ free_pcpu:
 	kfree(dev->_rx);
 #endif
 
-free_p:
-	kfree(p);
+free_dev:
+	netdev_freemem(dev);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6351,7 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		netdev_freemem(dev);
 		return;
 	}

net/core/net-sysfs.c +1 -1

index d954b56..d03f2c9 100644
@@ -1263,7 +1263,7 @@ static void netdev_release(struct device *d)
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	netdev_freemem(dev);
 }
 
 static const void *net_namespace(struct device *d)

Ben Hutchings

2013-10-30 19:16 UTC #14 lore raw

Re: [PATCH net-next] net: extend net_device allocation to vmalloc()

On Wed, 2013-10-30 at 08:01 -0700, Eric Dumazet wrote:
[...]

Documentation/networking/netdevices.txt network devices. · line 13

Quoted 10 lines

Documentation/networking/netdevices.txt +1 -3

@@ -10,9 +10,10 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with kmalloc() or vmalloc(). If device has registered

[...]

Well, drivers must never try to allocate and initialise struct net_device from scratch. They always call alloc_netdev_mqs() or one of its many wrappers. So 'must be allocated with kmalloc' was already wrong and this sentence should be removed completely.

Ben.

Signature · 3 lines

Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Ben Hutchings

2013-10-30 19:16 UTC #15 lore raw

Re: [PATCH net-next] net: extend net_device allocation to vmalloc()

In reply to Ben Hutchings

On Wed, 2013-10-30 at 08:01 -0700, Eric Dumazet wrote:
[...]

Documentation/networking/netdevices.txt network devices. · line 13

Quoted 10 lines

Documentation/networking/netdevices.txt +1 -3

@@ -10,9 +10,10 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with kmalloc() or vmalloc(). If device has registered

[...]

Ben.

Signature · 3 lines

Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Eric Dumazet

2013-10-30 20:09 UTC #16 lore raw

Re: [PATCH net-next] net: extend net_device allocation to vmalloc()

Quoted 4 lines · Ben Hutchings

, drivers must never try to allocate and initialise struct net_device from scratch. They always call alloc_netdev_mqs() or one of its many wrappers. So 'must be allocated with kmalloc' was already wrong and this sentence should be removed completely.

Right, I'll send a v2.

Eric Dumazet

2013-10-30 20:09 UTC #17 lore raw

Re: [PATCH net-next] net: extend net_device allocation to vmalloc()

Quoted 4 lines · Ben Hutchings

, drivers must never try to allocate and initialise struct net_device from scratch. They always call alloc_netdev_mqs() or one of its many wrappers. So 'must be allocated with kmalloc' was already wrong and this sentence should be removed completely.

Right, I'll send a v2.

Eric Dumazet

2013-10-30 20:10 UTC #18 lore raw

PATCH v2 1/1 net: extend net_device allocation to vmalloc()

From: Eric Dumazet <[email protected]>

Joby Poriyath provided a xen-netback patch to reduce the size of xenvif structure as some netdev allocation could fail under memory pressure/fragmentation.

This patch is handling the problem at the core level, allowing any netdev structures to use vmalloc() if kmalloc() failed.

As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT to kzalloc() flags to do this fallback only when really needed.

Cc Ben Hutchings <[email protected]>

---
v2: change the Documentation to point to alloc_netdev_mqs()
instead of kmalloc()/vzalloc(), from Ben

 Documentation/networking/netdevices.txt |   10 +++++-----
 include/linux/netdevice.h               |    1 +
 net/core/dev.c                          |   22 +++++++++++++++++-----
 net/core/net-sysfs.c                    |    2 +-
 4 files changed, 24 insertions(+), 11 deletions(-)

Documentation/networking/netdevices.txt +5 -5

index c7ecc70..7d8d79f 100644
@@ -10,12 +10,12 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with alloc_netdev_mqs() and friends.
+If device has registered successfully, it will be freed on last use
+by free_netdev(). This is required to handle the pathologic case cleanly
+(example: rmmod mydriver </sys/class/net/myeth/mtu )
 
-There are routines in net_init.c to handle the common cases of
-alloc_etherdev, alloc_netdev.  These reserve extra space for driver
+alloc_netdev_mqs()/alloc_netdev() reserve extra space for driver
 private data which gets freed when the network device is freed. If
 separately allocated data is attached to the network device
 (netdev_priv(dev)) then it is up to the module exit handler to free that.

include/linux/netdevice.h +1 -0

index cb1d918..e6353ca 100644
@@ -1800,6 +1800,7 @@ static inline void unregister_netdevice(struct net_device *dev)
 
 int netdev_refcnt_read(const struct net_device *dev);
 void free_netdev(struct net_device *dev);
+void netdev_freemem(struct net_device *dev);
 void synchronize_net(void);
 int init_dummy_netdev(struct net_device *dev);

net/core/dev.c +17 -5

index 0054c8c..0e61365 100644
@@ -6196,6 +6196,16 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
 
+void netdev_freemem(struct net_device *dev)
+{
+	char *addr = (char *)dev - dev->padded;
+
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+
 /**
  *	alloc_netdev_mqs - allocate network device
  *	@sizeof_priv:	size of private data to allocate space for

@@ -6239,7 +6249,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6248,7 +6260,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
-		goto free_p;
+		goto free_dev;
 
 	if (dev_addr_init(dev))
 		goto free_pcpu;

@@ -6301,8 +6313,8 @@ free_pcpu:
 	kfree(dev->_rx);
 #endif
 
-free_p:
-	kfree(p);
+free_dev:
+	netdev_freemem(dev);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6351,7 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		netdev_freemem(dev);
 		return;
 	}

net/core/net-sysfs.c +1 -1

index d954b56..d03f2c9 100644
@@ -1263,7 +1263,7 @@ static void netdev_release(struct device *d)
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	netdev_freemem(dev);
 }
 
 static const void *net_namespace(struct device *d)

Eric Dumazet

2013-10-30 20:10 UTC #19 lore raw

PATCH v2 1/1 net: extend net_device allocation to vmalloc()

From: Eric Dumazet <[email protected]>

Joby Poriyath provided a xen-netback patch to reduce the size of xenvif structure as some netdev allocation could fail under memory pressure/fragmentation.

This patch is handling the problem at the core level, allowing any netdev structures to use vmalloc() if kmalloc() failed.

As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT to kzalloc() flags to do this fallback only when really needed.

Cc Ben Hutchings <[email protected]>

---
v2: change the Documentation to point to alloc_netdev_mqs()
instead of kmalloc()/vzalloc(), from Ben

 Documentation/networking/netdevices.txt |   10 +++++-----
 include/linux/netdevice.h               |    1 +
 net/core/dev.c                          |   22 +++++++++++++++++-----
 net/core/net-sysfs.c                    |    2 +-
 4 files changed, 24 insertions(+), 11 deletions(-)

Documentation/networking/netdevices.txt +5 -5

index c7ecc70..7d8d79f 100644
@@ -10,12 +10,12 @@ network devices.
 struct net_device allocation rules
 ==================================
 Network device structures need to persist even after module is unloaded and
-must be allocated with kmalloc.  If device has registered successfully,
-it will be freed on last use by free_netdev.  This is required to handle the
-pathologic case cleanly (example: rmmod mydriver </sys/class/net/myeth/mtu )
+must be allocated with alloc_netdev_mqs() and friends.
+If device has registered successfully, it will be freed on last use
+by free_netdev(). This is required to handle the pathologic case cleanly
+(example: rmmod mydriver </sys/class/net/myeth/mtu )
 
-There are routines in net_init.c to handle the common cases of
-alloc_etherdev, alloc_netdev.  These reserve extra space for driver
+alloc_netdev_mqs()/alloc_netdev() reserve extra space for driver
 private data which gets freed when the network device is freed. If
 separately allocated data is attached to the network device
 (netdev_priv(dev)) then it is up to the module exit handler to free that.

include/linux/netdevice.h +1 -0

index cb1d918..e6353ca 100644
@@ -1800,6 +1800,7 @@ static inline void unregister_netdevice(struct net_device *dev)
 
 int netdev_refcnt_read(const struct net_device *dev);
 void free_netdev(struct net_device *dev);
+void netdev_freemem(struct net_device *dev);
 void synchronize_net(void);
 int init_dummy_netdev(struct net_device *dev);

net/core/dev.c +17 -5

index 0054c8c..0e61365 100644
@@ -6196,6 +6196,16 @@ void netdev_set_default_ethtool_ops(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_set_default_ethtool_ops);
 
+void netdev_freemem(struct net_device *dev)
+{
+	char *addr = (char *)dev - dev->padded;
+
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+
 /**
  *	alloc_netdev_mqs - allocate network device
  *	@sizeof_priv:	size of private data to allocate space for

@@ -6239,7 +6249,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	/* ensure 32-byte alignment of whole construct */
 	alloc_size += NETDEV_ALIGN - 1;
 
-	p = kzalloc(alloc_size, GFP_KERNEL);
+	p = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+	if (!p)
+		p = vzalloc(alloc_size);
 	if (!p)
 		return NULL;

@@ -6248,7 +6260,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->pcpu_refcnt = alloc_percpu(int);
 	if (!dev->pcpu_refcnt)
-		goto free_p;
+		goto free_dev;
 
 	if (dev_addr_init(dev))
 		goto free_pcpu;

@@ -6301,8 +6313,8 @@ free_pcpu:
 	kfree(dev->_rx);
 #endif
 
-free_p:
-	kfree(p);
+free_dev:
+	netdev_freemem(dev);
 	return NULL;
 }
 EXPORT_SYMBOL(alloc_netdev_mqs);

@@ -6339,7 +6351,7 @@ void free_netdev(struct net_device *dev)
 
 	/*  Compatibility with error handling in drivers */
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
-		kfree((char *)dev - dev->padded);
+		netdev_freemem(dev);
 		return;
 	}

net/core/net-sysfs.c +1 -1

index d954b56..d03f2c9 100644
@@ -1263,7 +1263,7 @@ static void netdev_release(struct device *d)
 	BUG_ON(dev->reg_state != NETREG_RELEASED);
 
 	kfree(dev->ifalias);
-	kfree((char *)dev - dev->padded);
+	netdev_freemem(dev);
 }
 
 static const void *net_namespace(struct device *d)

David Miller

maintainer

2013-11-04 04:19 UTC #20 lore raw

Re: [PATCH v2 net-next] net: extend net_device allocation to vmalloc()

From: Eric Dumazet <[email protected]>
Date: Wed, 30 Oct 2013 13:10:44 -0700

Quoted 12 lines

From: Eric Dumazet <[email protected]>
Joby Poriyath provided a xen-netback patch to reduce the size of xenvif structure as some netdev allocation could fail under memory pressure/fragmentation.
This patch is handling the problem at the core level, allowing any netdev structures to use vmalloc() if kmalloc() failed.
As vmalloc() adds overhead on a critical network path, add __GFP_REPEAT to kzalloc() flags to do this fallback only when really needed.

Quoted 6 lines

Signed-off-by Eric Dumazet <[email protected]>
Reported-by Joby Poriyath <[email protected]>
Cc Ben Hutchings <[email protected]>
---
v2: change the Documentation to point to alloc_netdev_mqs()
instead of kmalloc()/vzalloc(), from Ben

Looks good, applied, thanks Eric.

David Miller

maintainer

2013-11-04 04:19 UTC #21 lore raw

Re: [PATCH v2 net-next] net: extend net_device allocation to vmalloc()