All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] trivial clean up for e1000e driver
@ 2013-05-20  8:15 Wei Yang
  2013-05-20  8:15 ` [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size Wei Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20  8:15 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev, Wei Yang

Here is several trivial clean up for e1000e driver.

Wei Yang (4):
  e1000e: Remove duplicate assignment of default rx/tx ring size
  e1000e: Use marco instead of digit for defining
    e1000_rx_desc_packet_split
  e1000e: Calculate the desc_len based on adapter type
  e1000e: Not initialize the e1000_ps_page array when packet-split is
    not used

 drivers/net/ethernet/intel/e1000e/e1000.h  |    3 --
 drivers/net/ethernet/intel/e1000e/hw.h     |    5 ++-
 drivers/net/ethernet/intel/e1000e/netdev.c |   47 +++++++++++++++-------------
 3 files changed, 29 insertions(+), 26 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size
  2013-05-20  8:15 [PATCH 0/4] trivial clean up for e1000e driver Wei Yang
@ 2013-05-20  8:15 ` Wei Yang
  2013-05-20  9:37   ` Jeff Kirsher
  2013-05-20  8:15 ` [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split Wei Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2013-05-20  8:15 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev, Wei Yang

tx_ring/rx_ring size is assigned in function e1000_alloc_queues(), which is
called by e1000_sw_init() in the early stage of e1000_probe().

This patch just remove the duplicate assignment of this default ring size
value.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Reviewed-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Reviewed-by: Da Yu Qiu <qiudayu@cn.ibm.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 7e615e2..5cb8321 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6707,10 +6707,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	adapter->hw.fc.current_mode = e1000_fc_default;
 	adapter->hw.phy.autoneg_advertised = 0x2f;
 
-	/* ring size defaults */
-	adapter->rx_ring->count = E1000_DEFAULT_RXD;
-	adapter->tx_ring->count = E1000_DEFAULT_TXD;
-
 	/* Initial Wake on LAN setting - If APM wake is enabled in
 	 * the EEPROM, enable the ACPI Magic Packet filter
 	 */
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split
  2013-05-20  8:15 [PATCH 0/4] trivial clean up for e1000e driver Wei Yang
  2013-05-20  8:15 ` [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size Wei Yang
@ 2013-05-20  8:15 ` Wei Yang
  2013-05-20  9:37   ` Jeff Kirsher
  2013-05-20  8:15 ` [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type Wei Yang
  2013-05-20  8:15 ` [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used Wei Yang
  3 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2013-05-20  8:15 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev, Wei Yang

In structure e1000_rx_desc_packet_split, the size of wb.upper.length is
defined by a digit. This may introduce some problem when the lenght is
changed.

This patch use the marco PS_PAGE_BUFFERS for the definition. And move the
definition to hw.h.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h |    3 ---
 drivers/net/ethernet/intel/e1000e/hw.h    |    5 ++++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index fcc7581..3ba560d 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -90,9 +90,6 @@ struct e1000_info;
 
 #define E1000_MNG_VLAN_NONE		(-1)
 
-/* Number of packet split data buffers (not including the header buffer) */
-#define PS_PAGE_BUFFERS			(MAX_PS_BUFFERS - 1)
-
 #define DEFAULT_JUMBO			9234
 
 /* Time to wait before putting the device into D3 if there's no link (in ms). */
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index 1e6b889..edc618ad 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -227,6 +227,9 @@ union e1000_rx_desc_extended {
 };
 
 #define MAX_PS_BUFFERS 4
+
+/* Number of packet split data buffers (not including the header buffer) */
+#define PS_PAGE_BUFFERS			(MAX_PS_BUFFERS - 1)
 /* Receive Descriptor - Packet Split */
 union e1000_rx_desc_packet_split {
 	struct {
@@ -251,7 +254,7 @@ union e1000_rx_desc_packet_split {
 		} middle;
 		struct {
 			__le16 header_status;
-			__le16 length[3];	/* length of buffers 1-3 */
+			__le16 length[PS_PAGE_BUFFERS];	/* length of buffers 1-3 */
 		} upper;
 		__le64 reserved;
 	} wb; /* writeback */
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type
  2013-05-20  8:15 [PATCH 0/4] trivial clean up for e1000e driver Wei Yang
  2013-05-20  8:15 ` [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size Wei Yang
  2013-05-20  8:15 ` [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split Wei Yang
@ 2013-05-20  8:15 ` Wei Yang
  2013-05-20  9:37   ` Jeff Kirsher
  2013-08-22 10:57   ` Jeff Kirsher
  2013-05-20  8:15 ` [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used Wei Yang
  3 siblings, 2 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20  8:15 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev, Wei Yang

desc_len represents the size of descriptor in rx_ring. There are two kinds of
rx descriptors, e1000_rx_desc_packet_split(32 byte) and
e1000_rx_desc_extended(16 byte). Different adapter will use different rx
descriptors.

When allocating the dma space for this descriptor in current implementation,
the code ignore the descriptor type and take it as e1000_rx_desc_packet_split
in any case. This behavior will not effect the function, but will require
double size of dma space.

This patch will calculate the desc_len based on the adapter type.

Tested on T420, which use e1000_rx_desc_extended and works fine.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 5cb8321..a2e8a53 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2364,7 +2364,12 @@ int e1000e_setup_rx_resources(struct e1000_ring *rx_ring)
 			goto err_pages;
 	}
 
-	desc_len = sizeof(union e1000_rx_desc_packet_split);
+	if (adapter->rx_ps_pages) {
+		/* this is a 32 byte descriptor */
+		desc_len = sizeof(union e1000_rx_desc_packet_split);
+	} else {
+		desc_len = sizeof(union e1000_rx_desc_extended);
+	}
 
 	/* Round up to nearest 4K */
 	rx_ring->size = rx_ring->count * desc_len;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used
  2013-05-20  8:15 [PATCH 0/4] trivial clean up for e1000e driver Wei Yang
                   ` (2 preceding siblings ...)
  2013-05-20  8:15 ` [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type Wei Yang
@ 2013-05-20  8:15 ` Wei Yang
  2013-05-20  9:38   ` Jeff Kirsher
  3 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2013-05-20  8:15 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev, Wei Yang

When packet split is not used, those fields are still initialized and memory
is allocated for them.

This patch check whether packet split is used and do the initialization base
on the status.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   38 ++++++++++++++-------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index a2e8a53..c75d9cf 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1690,15 +1690,17 @@ static void e1000_clean_rx_ring(struct e1000_ring *rx_ring)
 			buffer_info->skb = NULL;
 		}
 
-		for (j = 0; j < PS_PAGE_BUFFERS; j++) {
-			ps_page = &buffer_info->ps_pages[j];
-			if (!ps_page->page)
-				break;
-			dma_unmap_page(&pdev->dev, ps_page->dma, PAGE_SIZE,
-				       DMA_FROM_DEVICE);
-			ps_page->dma = 0;
-			put_page(ps_page->page);
-			ps_page->page = NULL;
+		if (adapter->rx_ps_pages) {
+			for (j = 0; j < PS_PAGE_BUFFERS; j++) {
+				ps_page = &buffer_info->ps_pages[j];
+				if (!ps_page->page)
+					break;
+				dma_unmap_page(&pdev->dev, ps_page->dma, PAGE_SIZE,
+					       DMA_FROM_DEVICE);
+				ps_page->dma = 0;
+				put_page(ps_page->page);
+				ps_page->page = NULL;
+			}
 		}
 	}
 
@@ -2355,16 +2357,16 @@ int e1000e_setup_rx_resources(struct e1000_ring *rx_ring)
 	if (!rx_ring->buffer_info)
 		goto err;
 
-	for (i = 0; i < rx_ring->count; i++) {
-		buffer_info = &rx_ring->buffer_info[i];
-		buffer_info->ps_pages = kcalloc(PS_PAGE_BUFFERS,
-						sizeof(struct e1000_ps_page),
-						GFP_KERNEL);
-		if (!buffer_info->ps_pages)
-			goto err_pages;
-	}
-
 	if (adapter->rx_ps_pages) {
+		for (i = 0; i < rx_ring->count; i++) {
+			buffer_info = &rx_ring->buffer_info[i];
+			buffer_info->ps_pages = kcalloc(PS_PAGE_BUFFERS,
+							sizeof(struct e1000_ps_page),
+							GFP_KERNEL);
+			if (!buffer_info->ps_pages)
+				goto err_pages;
+		}
+
 		/* this is a 32 byte descriptor */
 		desc_len = sizeof(union e1000_rx_desc_packet_split);
 	} else {
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size
  2013-05-20  8:15 ` [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size Wei Yang
@ 2013-05-20  9:37   ` Jeff Kirsher
  2013-05-20 16:24     ` Wei Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2013-05-20  9:37 UTC (permalink / raw)
  To: Wei Yang; +Cc: jeffrey.t.kirsher, e1000-devel, netdev

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On 05/20/2013 01:15 AM, Wei Yang wrote:
> tx_ring/rx_ring size is assigned in function e1000_alloc_queues(), which is
> called by e1000_sw_init() in the early stage of e1000_probe().
>
> This patch just remove the duplicate assignment of this default ring size
> value.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> Reviewed-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> Reviewed-by: Da Yu Qiu <qiudayu@cn.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
Thanks, I have added the patch to my queue for e1000e.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split
  2013-05-20  8:15 ` [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split Wei Yang
@ 2013-05-20  9:37   ` Jeff Kirsher
  2013-05-20 16:24     ` Wei Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2013-05-20  9:37 UTC (permalink / raw)
  To: Wei Yang; +Cc: jeffrey.t.kirsher, e1000-devel, netdev

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On 05/20/2013 01:15 AM, Wei Yang wrote:
> In structure e1000_rx_desc_packet_split, the size of wb.upper.length is
> defined by a digit. This may introduce some problem when the lenght is
> changed.
>
> This patch use the marco PS_PAGE_BUFFERS for the definition. And move the
> definition to hw.h.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h |    3 ---
>  drivers/net/ethernet/intel/e1000e/hw.h    |    5 ++++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
Thanks, I have added the patch to my queue for e1000e.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type
  2013-05-20  8:15 ` [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type Wei Yang
@ 2013-05-20  9:37   ` Jeff Kirsher
  2013-05-20 16:25     ` Wei Yang
  2013-08-22 10:57   ` Jeff Kirsher
  1 sibling, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2013-05-20  9:37 UTC (permalink / raw)
  To: Wei Yang; +Cc: jeffrey.t.kirsher, e1000-devel, netdev

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

On 05/20/2013 01:15 AM, Wei Yang wrote:
> desc_len represents the size of descriptor in rx_ring. There are two kinds of
> rx descriptors, e1000_rx_desc_packet_split(32 byte) and
> e1000_rx_desc_extended(16 byte). Different adapter will use different rx
> descriptors.
>
> When allocating the dma space for this descriptor in current implementation,
> the code ignore the descriptor type and take it as e1000_rx_desc_packet_split
> in any case. This behavior will not effect the function, but will require
> double size of dma space.
>
> This patch will calculate the desc_len based on the adapter type.
>
> Tested on T420, which use e1000_rx_desc_extended and works fine.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
Thanks, I have added the patch to my queue for e1000e.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used
  2013-05-20  8:15 ` [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used Wei Yang
@ 2013-05-20  9:38   ` Jeff Kirsher
  2013-05-20 16:25     ` Wei Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2013-05-20  9:38 UTC (permalink / raw)
  To: Wei Yang; +Cc: jeffrey.t.kirsher, e1000-devel, netdev

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On 05/20/2013 01:15 AM, Wei Yang wrote:
> When packet split is not used, those fields are still initialized and memory
> is allocated for them.
>
> This patch check whether packet split is used and do the initialization base
> on the status.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |   38 ++++++++++++++-------------
>  1 files changed, 20 insertions(+), 18 deletions(-)
Thanks, I have added the patch to my queue for e1000e.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size
  2013-05-20  9:37   ` Jeff Kirsher
@ 2013-05-20 16:24     ` Wei Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20 16:24 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev

On Mon, May 20, 2013 at 02:37:16AM -0700, Jeff Kirsher wrote:
>On 05/20/2013 01:15 AM, Wei Yang wrote:
>> tx_ring/rx_ring size is assigned in function e1000_alloc_queues(), which is
>> called by e1000_sw_init() in the early stage of e1000_probe().
>>
>> This patch just remove the duplicate assignment of this default ring size
>> value.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> Reviewed-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> Reviewed-by: Da Yu Qiu <qiudayu@cn.ibm.com>
>> ---
>>  drivers/net/ethernet/intel/e1000e/netdev.c |    4 ----
>>  1 files changed, 0 insertions(+), 4 deletions(-)
>Thanks, I have added the patch to my queue for e1000e.

Thanks~

>



-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split
  2013-05-20  9:37   ` Jeff Kirsher
@ 2013-05-20 16:24     ` Wei Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20 16:24 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev

On Mon, May 20, 2013 at 02:37:29AM -0700, Jeff Kirsher wrote:
>On 05/20/2013 01:15 AM, Wei Yang wrote:
>> In structure e1000_rx_desc_packet_split, the size of wb.upper.length is
>> defined by a digit. This may introduce some problem when the lenght is
>> changed.
>>
>> This patch use the marco PS_PAGE_BUFFERS for the definition. And move the
>> definition to hw.h.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/net/ethernet/intel/e1000e/e1000.h |    3 ---
>>  drivers/net/ethernet/intel/e1000e/hw.h    |    5 ++++-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>Thanks, I have added the patch to my queue for e1000e.
>

Thanks~


-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type
  2013-05-20  9:37   ` Jeff Kirsher
@ 2013-05-20 16:25     ` Wei Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20 16:25 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev

On Mon, May 20, 2013 at 02:37:48AM -0700, Jeff Kirsher wrote:
>On 05/20/2013 01:15 AM, Wei Yang wrote:
>> desc_len represents the size of descriptor in rx_ring. There are two kinds of
>> rx descriptors, e1000_rx_desc_packet_split(32 byte) and
>> e1000_rx_desc_extended(16 byte). Different adapter will use different rx
>> descriptors.
>>
>> When allocating the dma space for this descriptor in current implementation,
>> the code ignore the descriptor type and take it as e1000_rx_desc_packet_split
>> in any case. This behavior will not effect the function, but will require
>> double size of dma space.
>>
>> This patch will calculate the desc_len based on the adapter type.
>>
>> Tested on T420, which use e1000_rx_desc_extended and works fine.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/net/ethernet/intel/e1000e/netdev.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>Thanks, I have added the patch to my queue for e1000e.
>

Thanks~



-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used
  2013-05-20  9:38   ` Jeff Kirsher
@ 2013-05-20 16:25     ` Wei Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2013-05-20 16:25 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: e1000-devel, netdev

On Mon, May 20, 2013 at 02:38:07AM -0700, Jeff Kirsher wrote:
>On 05/20/2013 01:15 AM, Wei Yang wrote:
>> When packet split is not used, those fields are still initialized and memory
>> is allocated for them.
>>
>> This patch check whether packet split is used and do the initialization base
>> on the status.
>>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/net/ethernet/intel/e1000e/netdev.c |   38 ++++++++++++++-------------
>>  1 files changed, 20 insertions(+), 18 deletions(-)
>Thanks, I have added the patch to my queue for e1000e.
>

Thanks~


-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type
  2013-05-20  8:15 ` [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type Wei Yang
  2013-05-20  9:37   ` Jeff Kirsher
@ 2013-08-22 10:57   ` Jeff Kirsher
  2013-08-22 14:26     ` Wei Yang
  1 sibling, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2013-08-22 10:57 UTC (permalink / raw)
  To: Wei Yang; +Cc: e1000-devel, netdev


[-- Attachment #1.1: Type: text/plain, Size: 5536 bytes --]

Validation ran into issues with this patch and because we could not apply
patch 4 in this series without patch 3, I have had to drop patch 3 & 4 from
my queue.

Here is what validation had to say about this patch...

Aaron Brown wrote:

This patch introduces a call trace when the interface is up and then
the mtu is set over 1500, or if the mtu is set above 1500 when the
interface is down,
the call trace appears when the interface is brought up.  The trace is
captured to demsg and /var/log/messages:
---------------------------------------------------
17:14:52 u1307 kernel: Call Trace:
17:14:52 u1307 kernel: [<ffffffff813ffaeb>] dump_stack+0x19/0x1e
17:14:52 u1307 kernel: [<ffffffff8104ae80>] __might_sleep+0xe2/0xe4
17:14:52 u1307 kernel: [<ffffffff81400e63>] down_read+0x1f/0x31
17:14:52 u1307 kernel: [<ffffffff8102c1b7>] exit_mm+0x3a/0x164
17:14:52 u1307 kernel: [<ffffffff8102da09>] do_exit+0x1ef/0x2f7
17:14:52 u1307 kernel: [<ffffffff81403bce>] oops_end+0x8f/0x94
17:14:52 u1307 kernel: [<ffffffff81020ca5>] no_context+0x1a4/0x1b3
17:14:52 u1307 kernel: [<ffffffff81020e6c>] __bad_area_nosemaphore+0x1b8/0x1d8
17:14:52 u1307 kernel: [<ffffffff81020e9a>] bad_area_nosemaphore+0xe/0x10
17:14:52 u1307 kernel: [<ffffffff81405a69>] __do_page_fault+0x44f/0x48e
17:14:52 u1307 kernel: [<ffffffff81004494>] ? print_context_stack+0xa2/0xbe
17:14:52 u1307 kernel: [<ffffffff81003488>] ? dump_trace+0x282/0x2aa
17:14:52 u1307 kernel: [<ffffffff81402bf0>] ?
_raw_spin_unlock_irqrestore+0x1d/0x3a
17:14:52 u1307 kernel: [<ffffffff81405ab1>] do_page_fault+0x9/0xb
17:14:52 u1307 kernel: [<ffffffff814031e2>] page_fault+0x22/0x30
17:14:52 u1307 kernel: [<ffffffffa00238ef>] ?
e1000_alloc_rx_buffers_ps+0x1d9/0x421 [e1000e]
17:14:52 u1307 kernel: [<ffffffffa00238a3>] ?
e1000_alloc_rx_buffers_ps+0x18d/0x421 [e1000e]
17:14:52 u1307 kernel: [<ffffffffa0022bd6>] e1000_configure+0xe7/0xf0 [e1000e]
17:14:52 u1307 kernel: [<ffffffffa0022bee>] e1000e_up+0xf/0xe7 [e1000e]
17:14:52 u1307 kernel: [<ffffffffa00282da>]
e1000_change_mtu+0x142/0x162 [e1000e]
17:14:52 u1307 kernel: [<ffffffff8135a98b>] dev_set_mtu+0x3f/0x5e
17:14:52 u1307 kernel: [<ffffffff81369280>] dev_ifsioc+0xec/0x329
17:14:52 u1307 kernel: [<ffffffff81369a24>] dev_ioctl+0x309/0x3bd
17:14:52 u1307 kernel: [<ffffffff81348035>] sock_ioctl+0x21e/0x22b
17:14:52 u1307 kernel: [<ffffffff810ce1b4>] do_vfs_ioctl+0x28e/0x2aa
17:14:52 u1307 kernel: [<ffffffff810ce217>] SyS_ioctl+0x47/0x69
17:14:52 u1307 kernel: [<ffffffff81407b12>] system_call_fastpath+0x16/0x1b
17:14:52 u1307 kernel: ------------[ cut here ]------------
17:14:52 u1307 kernel: WARNING: at kernel/softirq.c:160
local_bh_enable_ip+0x3c/0x9a()
17:14:52 u1307 kernel: Modules linked in: bridge stp llc nfsd lockd
exportfs sunrpc e1000e ptp pps_core
17:14:52 u1307 kernel: CPU: 6 PID: 5289 Comm: ifconfig Tainted: G
D      3.10.0-rc1_net-next_e1000e_2850dce_regress-11638-gb471f26 #5
17:14:52 u1307 kernel: Hardware name: Supermicro
X9SCL/X9SCM/X9SCL/X9SCM, BIOS 2.0b 09/17/2012
17:14:52 u1307 kernel: ffffffff816e7023 ffff880222b2b808
ffffffff813ffaeb ffff880222b2b848
17:14:52 u1307 kernel: ffffffff810287e6 ffffffff81a25188
0000000000000000 ffff880221f91bc0
17:14:52 u1307 kernel: ffff880221f91dd8 ffff880221f91f70
0000000000000000 ffff880222b2b858
-------------------------------------------------------------------------------

The system remains up but unstable, attempts to access the network
interface causes a hang, attempts to login to a new session on the
console sometimes hang.
This is happening with all adapters I've tried so far, 82579, 82578,
82574, ich10, ich9...



On Mon, May 20, 2013 at 1:15 AM, Wei Yang <weiyang@linux.vnet.ibm.com>wrote:

> desc_len represents the size of descriptor in rx_ring. There are two kinds
> of
> rx descriptors, e1000_rx_desc_packet_split(32 byte) and
> e1000_rx_desc_extended(16 byte). Different adapter will use different rx
> descriptors.
>
> When allocating the dma space for this descriptor in current
> implementation,
> the code ignore the descriptor type and take it as
> e1000_rx_desc_packet_split
> in any case. This behavior will not effect the function, but will require
> double size of dma space.
>
> This patch will calculate the desc_len based on the adapter type.
>
> Tested on T420, which use e1000_rx_desc_extended and works fine.
>
> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 5cb8321..a2e8a53 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -2364,7 +2364,12 @@ int e1000e_setup_rx_resources(struct e1000_ring
> *rx_ring)
>                         goto err_pages;
>         }
>
> -       desc_len = sizeof(union e1000_rx_desc_packet_split);
> +       if (adapter->rx_ps_pages) {
> +               /* this is a 32 byte descriptor */
> +               desc_len = sizeof(union e1000_rx_desc_packet_split);
> +       } else {
> +               desc_len = sizeof(union e1000_rx_desc_extended);
> +       }
>
>         /* Round up to nearest 4K */
>         rx_ring->size = rx_ring->count * desc_len;
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Cheers,
Jeff

[-- Attachment #2: Type: text/plain, Size: 379 bytes --]

------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and 
AppDynamics. Performance Central is your source for news, insights, 
analysis and resources for efficient Application Performance Management. 
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type
  2013-08-22 10:57   ` Jeff Kirsher
@ 2013-08-22 14:26     ` Wei Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2013-08-22 14:26 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: e1000-devel, netdev

Jeff,

Thanks for letting me know this.

Setting MTU is not tested, maybe this patch introduce some problem in this
case. Sorry for breaking your driver :-(

On Thu, Aug 22, 2013 at 03:57:24AM -0700, Jeff Kirsher wrote:
>Validation ran into issues with this patch and because we could not apply patch
>4 in this series without patch 3, I have had to drop patch 3 &amp; 4 from my
>queue.
>
>Here is what validation had to say about this patch...
>
>Aaron Brown wrote:
>This patch introduces a call trace when the interface is up and then the mtu is
>set over 1500, or if the mtu is set above 1500 when the interface is down,
>the call trace appears when the interface is brought up.  The trace is captured
>to demsg and /var/log/messages:
>---------------------------------------------------
>17:14:52 u1307 kernel: Call Trace:
>17:14:52 u1307 kernel: [<ffffffff813ffaeb>] dump_stack+0x19/0x1e
>17:14:52 u1307 kernel: [<ffffffff8104ae80>] __might_sleep+0xe2/0xe4
>17:14:52 u1307 kernel: [<ffffffff81400e63>] down_read+0x1f/0x31
>17:14:52 u1307 kernel: [<ffffffff8102c1b7>] exit_mm+0x3a/0x164
>17:14:52 u1307 kernel: [<ffffffff8102da09>] do_exit+0x1ef/0x2f7
>17:14:52 u1307 kernel: [<ffffffff81403bce>] oops_end+0x8f/0x94
>17:14:52 u1307 kernel: [<ffffffff81020ca5>] no_context+0x1a4/0x1b3
>17:14:52 u1307 kernel: [<ffffffff81020e6c>] __bad_area_nosemaphore+0x1b8/0x1d8
>17:14:52 u1307 kernel: [<ffffffff81020e9a>] bad_area_nosemaphore+0xe/0x10
>17:14:52 u1307 kernel: [<ffffffff81405a69>] __do_page_fault+0x44f/0x48e
>17:14:52 u1307 kernel: [<ffffffff81004494>] ? print_context_stack+0xa2/0xbe
>17:14:52 u1307 kernel: [<ffffffff81003488>] ? dump_trace+0x282/0x2aa
>17:14:52 u1307 kernel: [<ffffffff81402bf0>] ? _raw_spin_unlock_irqrestore+0x1d/
>0x3a
>17:14:52 u1307 kernel: [<ffffffff81405ab1>] do_page_fault+0x9/0xb
>17:14:52 u1307 kernel: [<ffffffff814031e2>] page_fault+0x22/0x30
>17:14:52 u1307 kernel: [<ffffffffa00238ef>] ? e1000_alloc_rx_buffers_ps+0x1d9/
>0x421 [e1000e]
>17:14:52 u1307 kernel: [<ffffffffa00238a3>] ? e1000_alloc_rx_buffers_ps+0x18d/
>0x421 [e1000e]
>17:14:52 u1307 kernel: [<ffffffffa0022bd6>] e1000_configure+0xe7/0xf0 [e1000e]
>17:14:52 u1307 kernel: [<ffffffffa0022bee>] e1000e_up+0xf/0xe7 [e1000e]
>17:14:52 u1307 kernel: [<ffffffffa00282da>] e1000_change_mtu+0x142/0x162
>[e1000e]
>17:14:52 u1307 kernel: [<ffffffff8135a98b>] dev_set_mtu+0x3f/0x5e
>17:14:52 u1307 kernel: [<ffffffff81369280>] dev_ifsioc+0xec/0x329
>17:14:52 u1307 kernel: [<ffffffff81369a24>] dev_ioctl+0x309/0x3bd
>17:14:52 u1307 kernel: [<ffffffff81348035>] sock_ioctl+0x21e/0x22b
>17:14:52 u1307 kernel: [<ffffffff810ce1b4>] do_vfs_ioctl+0x28e/0x2aa
>17:14:52 u1307 kernel: [<ffffffff810ce217>] SyS_ioctl+0x47/0x69
>17:14:52 u1307 kernel: [<ffffffff81407b12>] system_call_fastpath+0x16/0x1b
>17:14:52 u1307 kernel: ------------[ cut here ]------------
>17:14:52 u1307 kernel: WARNING: at kernel/softirq.c:160
>local_bh_enable_ip+0x3c/0x9a()
>17:14:52 u1307 kernel: Modules linked in: bridge stp llc nfsd lockd exportfs
>sunrpc e1000e ptp pps_core
>17:14:52 u1307 kernel: CPU: 6 PID: 5289 Comm: ifconfig Tainted: G      D
>3.10.0-rc1_net-next_e1000e_2850dce_regress-11638-gb471f26 #5
>17:14:52 u1307 kernel: Hardware name: Supermicro X9SCL/X9SCM/X9SCL/X9SCM, BIOS
>2.0b 09/17/2012
>17:14:52 u1307 kernel: ffffffff816e7023 ffff880222b2b808 ffffffff813ffaeb
>ffff880222b2b848
>17:14:52 u1307 kernel: ffffffff810287e6 ffffffff81a25188 0000000000000000
>ffff880221f91bc0
>17:14:52 u1307 kernel: ffff880221f91dd8 ffff880221f91f70 0000000000000000
>ffff880222b2b858
>-------------------------------------------------------------------------------
>
>The system remains up but unstable, attempts to access the network interface
>causes a hang, attempts to login to a new session on the console sometimes
>hang.
>This is happening with all adapters I've tried so far, 82579, 82578, 82574,
>ich10, ich9...
>
>
>On Mon, May 20, 2013 at 1:15 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>     desc_len represents the size of descriptor in rx_ring. There are two
>     kinds of
>     rx descriptors, e1000_rx_desc_packet_split(32 byte) and
>     e1000_rx_desc_extended(16 byte). Different adapter will use different
>     rx
>     descriptors.
>
>     When allocating the dma space for this descriptor in current
>     implementation,
>     the code ignore the descriptor type and take it as
>     e1000_rx_desc_packet_split
>     in any case. This behavior will not effect the function, but will
>     require
>     double size of dma space.
>
>     This patch will calculate the desc_len based on the adapter type.
>
>     Tested on T420, which use e1000_rx_desc_extended and works fine.
>
>     Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>     ---
>      drivers/net/ethernet/intel/e1000e/netdev.c |    7 ++++++-
>      1 files changed, 6 insertions(+), 1 deletions(-)
>
>     diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/
>     net/ethernet/intel/e1000e/netdev.c
>     index 5cb8321..a2e8a53 100644
>     --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>     +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>     @@ -2364,7 +2364,12 @@ int e1000e_setup_rx_resources(struct
>     e1000_ring *rx_ring)
>                             goto err_pages;
>             }
>
>     -       desc_len = sizeof(union e1000_rx_desc_packet_split);
>     +       if (adapter->rx_ps_pages) {
>     +               /* this is a 32 byte descriptor */
>     +               desc_len = sizeof(union
>     e1000_rx_desc_packet_split);
>     +       } else {
>     +               desc_len = sizeof(union
>     e1000_rx_desc_extended);
>     +       }
>
>             /* Round up to nearest 4K */
>             rx_ring->size = rx_ring->count * desc_len;
>     --
>     1.7.5.4
>
>     --
>     To unsubscribe from this list: send the line &quot;unsubscribe
>     netdev&quot; in
>     the body of a message to majordomo@vger.kernel.org
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
>--
>Cheers,
>Jeff

-- 
Richard Yang
Help you, Help me

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-08-22 14:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-20  8:15 [PATCH 0/4] trivial clean up for e1000e driver Wei Yang
2013-05-20  8:15 ` [PATCH 1/4] e1000e: Remove duplicate assignment of default rx/tx ring size Wei Yang
2013-05-20  9:37   ` Jeff Kirsher
2013-05-20 16:24     ` Wei Yang
2013-05-20  8:15 ` [PATCH 2/4] e1000e: Use marco instead of digit for defining e1000_rx_desc_packet_split Wei Yang
2013-05-20  9:37   ` Jeff Kirsher
2013-05-20 16:24     ` Wei Yang
2013-05-20  8:15 ` [PATCH 3/4] e1000e: Calculate the desc_len based on adapter type Wei Yang
2013-05-20  9:37   ` Jeff Kirsher
2013-05-20 16:25     ` Wei Yang
2013-08-22 10:57   ` Jeff Kirsher
2013-08-22 14:26     ` Wei Yang
2013-05-20  8:15 ` [PATCH 4/4] e1000e: Not initialize the e1000_ps_page array when packet-split is not used Wei Yang
2013-05-20  9:38   ` Jeff Kirsher
2013-05-20 16:25     ` Wei Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.