diff mbox

[net-next,v2,11/12] net: ethernet: aquantia: Fixed memory allocation if AQ_CFG_RX_FRAME_MAX > 1 page.

Message ID c45b44987d24c7aae83c796a3731295317ae83f7.1487365317.git.pavel.belous@aquantia.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Pavel Belous Feb. 17, 2017, 9:07 p.m. UTC 
From: Pavel Belous <pavel.belous@aquantia.com>

We should allocate the number of pages based on the config parameter
AQ_CFG_RX_FRAME_MAX.

Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c  | 4 ++--
 drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Andrew Lunn Feb. 17, 2017, 10:43 p.m. UTC | #1 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> index 4c40644..0877625 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
> @@ -278,6 +278,8 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
>  	struct aq_ring_buff_s *buff = NULL;
>  	int err = 0;
>  	int i = 0;
> +	unsigned int pages_order = fls(AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
> +		(AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0)) - 1;

Reverse Christmas tree?

	Andrew
Lino Sanfilippo Feb. 18, 2017, 11:50 a.m. UTC | #2 
Hi,

On 17.02.2017 22:07, Pavel Belous wrote:
> From: Pavel Belous <pavel.belous@aquantia.com>
> 
> We should allocate the number of pages based on the config parameter
> AQ_CFG_RX_FRAME_MAX.
> 
> Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>

>  	do {
>  		if (spin_trylock(&ring->header.lock)) {
> -			frags = aq_nic_map_skb(self, skb, &buffers[0]);
> +			frags = aq_nic_map_skb(self, skb, buffers);
>  
> -			aq_ring_tx_append_buffs(ring, &buffers[0], frags);
> +			aq_ring_tx_append_buffs(ring, buffers, frags);
>  

This change has nothing to do with what the commit message claims that the
patch is about. Please dont mix fixes and totally unrelated cleanups in one
patch.

Regards,
Lino
Pavel Belous Feb. 18, 2017, 4:33 p.m. UTC | #3 
On 02/18/2017 01:43 AM, Andrew Lunn wrote:
>> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
>> index 4c40644..0877625 100644
>> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
>> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
>> @@ -278,6 +278,8 @@ int aq_ring_rx_fill(struct aq_ring_s *self)
>>  	struct aq_ring_buff_s *buff = NULL;
>>  	int err = 0;
>>  	int i = 0;
>> +	unsigned int pages_order = fls(AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
>> +		(AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0)) - 1;
>
> Reverse Christmas tree?
>
> 	Andrew
>

Thank you.
I will fix it in v3.

Regards,
Pavel
Pavel Belous Feb. 18, 2017, 4:44 p.m. UTC | #4 
On 02/18/2017 02:50 PM, Lino Sanfilippo wrote:
> Hi,
>
> On 17.02.2017 22:07, Pavel Belous wrote:
>> From: Pavel Belous <pavel.belous@aquantia.com>
>>
>> We should allocate the number of pages based on the config parameter
>> AQ_CFG_RX_FRAME_MAX.
>>
>> Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
>
>>  	do {
>>  		if (spin_trylock(&ring->header.lock)) {
>> -			frags = aq_nic_map_skb(self, skb, &buffers[0]);
>> +			frags = aq_nic_map_skb(self, skb, buffers);
>>
>> -			aq_ring_tx_append_buffs(ring, &buffers[0], frags);
>> +			aq_ring_tx_append_buffs(ring, buffers, frags);
>>
>
> This change has nothing to do with what the commit message claims that the
> patch is about. Please dont mix fixes and totally unrelated cleanups in one
> patch.
>
> Regards,
> Lino
>

Sorry, its just small fix for readability.
I will remove it or put in separate patch in v3.

Regards,
Pavel
diff mbox

Patch

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index daed4c1..25dc9b4 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -596,9 +596,9 @@  __acquires(&ring->lock)
 
 	do {
 		if (spin_trylock(&ring->header.lock)) {
-			frags = aq_nic_map_skb(self, skb, &buffers[0]);
+			frags = aq_nic_map_skb(self, skb, buffers);
 
-			aq_ring_tx_append_buffs(ring, &buffers[0], frags);
+			aq_ring_tx_append_buffs(ring, buffers, frags);
 
 			err = self->aq_hw_ops.hw_ring_tx_xmit(self->aq_hw,
 							      ring, frags);
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 4c40644..0877625 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -278,6 +278,8 @@  int aq_ring_rx_fill(struct aq_ring_s *self)
 	struct aq_ring_buff_s *buff = NULL;
 	int err = 0;
 	int i = 0;
+	unsigned int pages_order = fls(AQ_CFG_RX_FRAME_MAX / PAGE_SIZE +
+		(AQ_CFG_RX_FRAME_MAX % PAGE_SIZE ? 1 : 0)) - 1;
 
 	for (i = aq_ring_avail_dx(self); i--;
 		self->sw_tail = aq_ring_next_dx(self, self->sw_tail)) {
@@ -287,7 +289,7 @@  int aq_ring_rx_fill(struct aq_ring_s *self)
 		buff->len = AQ_CFG_RX_FRAME_MAX;
 
 		buff->page = alloc_pages(GFP_ATOMIC | __GFP_COLD |
-					 __GFP_COMP, 0);
+					 __GFP_COMP, pages_order);
 		if (!buff->page) {
 			err = -ENOMEM;
 			goto err_exit;