[PATCH] iavf: use kvzalloc instead of kzalloc for rx/tx_bi buffer

when changes the rx/tx ring to 4096, kzalloc may fail due to
a temporary shortage on slab entries.

kvmalloc is used to allocate this memory as there is no need
to have this memory area physical continuously.

Signed-off-by: Li RongQing <redacted>
---
 drivers/net/ethernet/intel/iavf/iavf_txrx.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index 256fa07d54d5..f5a3e195ec54 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c

@@ -92,7 +92,7 @@ void iavf_clean_tx_ring(struct iavf_ring *tx_ring)
 void iavf_free_tx_resources(struct iavf_ring *tx_ring)
 {
 	iavf_clean_tx_ring(tx_ring);
-	kfree(tx_ring->tx_bi);
+	kvfree(tx_ring->tx_bi);
 	tx_ring->tx_bi = NULL;
 
 	if (tx_ring->desc) {

@@ -622,7 +622,7 @@ int iavf_setup_tx_descriptors(struct iavf_ring *tx_ring)
 	/* warn if we are about to overwrite the pointer */
 	WARN_ON(tx_ring->tx_bi);
 	bi_size = sizeof(struct iavf_tx_buffer) * tx_ring->count;
-	tx_ring->tx_bi = kzalloc(bi_size, GFP_KERNEL);
+	tx_ring->tx_bi = kvzalloc(bi_size, GFP_KERNEL);
 	if (!tx_ring->tx_bi)
 		goto err;
 

@@ -643,7 +643,7 @@ int iavf_setup_tx_descriptors(struct iavf_ring *tx_ring)
 	return 0;
 
 err:
-	kfree(tx_ring->tx_bi);
+	kvfree(tx_ring->tx_bi);
 	tx_ring->tx_bi = NULL;
 	return -ENOMEM;
 }

@@ -714,7 +714,7 @@ void iavf_clean_rx_ring(struct iavf_ring *rx_ring)
 void iavf_free_rx_resources(struct iavf_ring *rx_ring)
 {
 	iavf_clean_rx_ring(rx_ring);
-	kfree(rx_ring->rx_bi);
+	kvfree(rx_ring->rx_bi);
 	rx_ring->rx_bi = NULL;
 
 	if (rx_ring->desc) {

@@ -738,7 +738,7 @@ int iavf_setup_rx_descriptors(struct iavf_ring *rx_ring)
 	/* warn if we are about to overwrite the pointer */
 	WARN_ON(rx_ring->rx_bi);
 	bi_size = sizeof(struct iavf_rx_buffer) * rx_ring->count;
-	rx_ring->rx_bi = kzalloc(bi_size, GFP_KERNEL);
+	rx_ring->rx_bi = kvzalloc(bi_size, GFP_KERNEL);
 	if (!rx_ring->rx_bi)
 		goto err;
 

@@ -762,7 +762,7 @@ int iavf_setup_rx_descriptors(struct iavf_ring *rx_ring)
 
 	return 0;
 err:
-	kfree(rx_ring->rx_bi);
+	kvfree(rx_ring->rx_bi);
 	rx_ring->rx_bi = NULL;
 	return -ENOMEM;
 }

-- 
2.16.2

On 8/27/20 12:53 AM, Li RongQing wrote:

when changes the rx/tx ring to 4096, kzalloc may fail due to
a temporary shortage on slab entries.

kvmalloc is used to allocate this memory as there is no need
to have this memory area physical continuously.

Signed-off-by: Li RongQing <redacted>
---

Well, fallback to vmalloc() overhead because order-1 pages are not readily available
when the NIC is setup (usually one time per boot)
is adding TLB cost at run time, for billions of packets to come, maybe for months.

Surely trying a bit harder to get order-1 pages is desirable.

 __GFP_RETRY_MAYFAIL is supposed to help here.

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Thursday, August 27, 2020 4:26 PM
To: Li,Rongqing <redacted>; netdev@vger.kernel.org;
intel-wired-lan@lists.osuosl.org
Subject: Re: [PATCH] iavf: use kvzalloc instead of kzalloc for rx/tx_bi buffer



On 8/27/20 12:53 AM, Li RongQing wrote:

quoted

when changes the rx/tx ring to 4096, kzalloc may fail due to a
temporary shortage on slab entries.

kvmalloc is used to allocate this memory as there is no need to have
this memory area physical continuously.

Signed-off-by: Li RongQing <redacted>
---

Well, fallback to vmalloc() overhead because order-1 pages are not readily
available when the NIC is setup (usually one time per boot) is adding TLB cost
at run time, for billions of packets to come, maybe for months.

Surely trying a bit harder to get order-1 pages is desirable.

 __GFP_RETRY_MAYFAIL is supposed to help here.

Could we add __GFP_RETRY_MAYFAIL to kvmalloc, to ensure the allocation success ?

I see that lots of drivers are using vmalloc for this buffer, should we change it kmalloc?  

grep "buffer_info =" drivers/net/ethernet/intel/ -rI|grep alloc

drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:      tx_ring->tx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:      rx_ring->rx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:  tx_ring->tx_buffer_info = vmalloc_node(size, ring_node);
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:          tx_ring->tx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:  rx_ring->rx_buffer_info = vmalloc_node(size, ring_node);
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:          rx_ring->rx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/ixgb/ixgb_main.c:    txdr->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/ixgb/ixgb_main.c:    rxdr->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/e1000e/ethtool.c:    tx_ring->buffer_info = kcalloc(tx_ring->count,
drivers/net/ethernet/intel/e1000e/ethtool.c:    rx_ring->buffer_info = kcalloc(rx_ring->count,
drivers/net/ethernet/intel/e1000e/netdev.c:     tx_ring->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/e1000e/netdev.c:     rx_ring->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/igb/igb_main.c:      tx_ring->tx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/igb/igb_main.c:      rx_ring->rx_buffer_info = vmalloc(size);
drivers/net/ethernet/intel/e1000/e1000_ethtool.c:       txdr->buffer_info = kcalloc(txdr->count, sizeof(struct e1000_tx_buffer),
drivers/net/ethernet/intel/e1000/e1000_ethtool.c:       rxdr->buffer_info = kcalloc(rxdr->count, sizeof(struct e1000_rx_buffer),
drivers/net/ethernet/intel/e1000/e1000_main.c:  txdr->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/e1000/e1000_main.c:  rxdr->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/igc/igc_main.c:      tx_ring->tx_buffer_info = vzalloc(size);
drivers/net/ethernet/intel/igc/igc_main.c:      rx_ring->rx_buffer_info = vzalloc(size);
drivers/net/ethernet/intel/igbvf/netdev.c:      tx_ring->buffer_info = vzalloc(size);
drivers/net/ethernet/intel/igbvf/netdev.c:      rx_ring->buffer_info = vzalloc(size);


-Li

On 8/27/20 1:53 AM, Li,Rongqing wrote:

quoted

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Thursday, August 27, 2020 4:26 PM
To: Li,Rongqing <redacted>; netdev@vger.kernel.org;
intel-wired-lan@lists.osuosl.org
Subject: Re: [PATCH] iavf: use kvzalloc instead of kzalloc for rx/tx_bi buffer



On 8/27/20 12:53 AM, Li RongQing wrote:

quoted

when changes the rx/tx ring to 4096, kzalloc may fail due to a
temporary shortage on slab entries.

kvmalloc is used to allocate this memory as there is no need to have
this memory area physical continuously.

Signed-off-by: Li RongQing <redacted>
---

Well, fallback to vmalloc() overhead because order-1 pages are not readily
available when the NIC is setup (usually one time per boot) is adding TLB cost
at run time, for billions of packets to come, maybe for months.

Surely trying a bit harder to get order-1 pages is desirable.

 __GFP_RETRY_MAYFAIL is supposed to help here.

Could we add __GFP_RETRY_MAYFAIL to kvmalloc, to ensure the allocation success ?

__GFP_RETRY_MAYFAIL does not _ensure_ the allocation success.

The idea here is that for large allocations (bigger than PAGE_SIZE),
kvmalloc_node() will not force __GFP_NORETRY, meaning that page allocator
will not bailout immediately in case of memory pressure.

This gives a chance for page reclaims to happen, and eventually the high order page
allocation will succeed under normal circumstances.

It is a trade-off, and only worth it for long living allocations.

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Thursday, August 27, 2020 5:55 PM
To: Li,Rongqing <redacted>; Eric Dumazet
[off-list ref]; netdev@vger.kernel.org;
intel-wired-lan@lists.osuosl.org
Subject: Re: [PATCH] iavf: use kvzalloc instead of kzalloc for rx/tx_bi buffer



On 8/27/20 1:53 AM, Li,Rongqing wrote:

quoted

quoted

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Thursday, August 27, 2020 4:26 PM
To: Li,Rongqing <redacted>; netdev@vger.kernel.org;
intel-wired-lan@lists.osuosl.org
Subject: Re: [PATCH] iavf: use kvzalloc instead of kzalloc for
rx/tx_bi buffer



On 8/27/20 12:53 AM, Li RongQing wrote:

quoted

when changes the rx/tx ring to 4096, kzalloc may fail due to a
temporary shortage on slab entries.

kvmalloc is used to allocate this memory as there is no need to have
this memory area physical continuously.

Signed-off-by: Li RongQing <redacted>
---

Well, fallback to vmalloc() overhead because order-1 pages are not
readily available when the NIC is setup (usually one time per boot)
is adding TLB cost at run time, for billions of packets to come, maybe for

months.

quoted

quoted

Surely trying a bit harder to get order-1 pages is desirable.

 __GFP_RETRY_MAYFAIL is supposed to help here.

Could we add __GFP_RETRY_MAYFAIL to kvmalloc, to ensure the allocation

success ?

__GFP_RETRY_MAYFAIL does not _ensure_ the allocation success.

The idea here is that for large allocations (bigger than PAGE_SIZE),
kvmalloc_node() will not force __GFP_NORETRY, meaning that page allocator
will not bailout immediately in case of memory pressure.

This gives a chance for page reclaims to happen, and eventually the high order
page allocation will succeed under normal circumstances.

It is a trade-off, and only worth it for long living allocations.

Thanks, Eric; 
I will change it as below, that kmalloc will be used in most time, and ensure allocation success if fail to reclaim memory under memory pressure.

@@ -622,7 +622,7 @@ int iavf_setup_tx_descriptors(struct iavf_ring *tx_ring)
        /* warn if we are about to overwrite the pointer */
        WARN_ON(tx_ring->tx_bi);
        bi_size = sizeof(struct iavf_tx_buffer) * tx_ring->count;
-       tx_ring->tx_bi = kzalloc(bi_size, GFP_KERNEL);
+       tx_ring->tx_bi = kvzalloc(bi_size, GFP_KERNEL|__GFP_RETRY_MAYFAIL);
        if (!tx_ring->tx_bi)
                goto err;
 

@@ -643,7 +643,7 @@ int iavf_setup_tx_descriptors(struct iavf_ring *tx_ring)


-Li