[PATCH net-next 0/5] be2net: fixes

[PATCH net-next 0/5] be2net: fixes

[Sathya Perla]

Pls apply.

Sathya Perla (5):
  be2net: Fix race in posting rx buffers.
  be2net: get rid of memory mapped pci-cfg space address
  be2net: fix erx->rx_drops_no_frags wrap around
  be2net: increase FW update completion timeout
  be2net: remove unused variable

 drivers/net/ethernet/emulex/benet/be.h      |    2 -
 drivers/net/ethernet/emulex/benet/be_cmds.c |    2 +-
 drivers/net/ethernet/emulex/benet/be_main.c |   50 ++++++++++++++------------
 3 files changed, 28 insertions(+), 26 deletions(-)

-- 
1.7.4

[PATCH net-next 1/5] be2net: Fix race in posting rx buffers.

[Sathya Perla]

There is a possibility of be_post_rx_frags() being called simultaneously from
both be_worker() (when rx_post_starved) and be_poll_rx() (when rxq->used is 0).
This can be avoided by posting rx buffers only when some completions have been
reaped.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index ef62594..09eb699 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1862,7 +1862,7 @@ loop_continue:
 	}
 
 	/* Refill the queue */
-	if (atomic_read(&rxo->q.used) < RX_FRAGS_REFILL_WM)
+	if (work_done && atomic_read(&rxo->q.used) < RX_FRAGS_REFILL_WM)
 		be_post_rx_frags(rxo, GFP_ATOMIC);
 
 	/* All consumed */
-- 
1.7.4

[PATCH net-next 2/5] be2net: get rid of memory mapped pci-cfg space address

[Sathya Perla]

Get rid of adapter->pcicfg and its use. Use pci_config_read/write_dword()
instead.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h      |    1 -
 drivers/net/ethernet/emulex/benet/be_main.c |   27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 12b5b51..868d7f4 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -298,7 +298,6 @@ struct be_adapter {
 
 	u8 __iomem *csr;
 	u8 __iomem *db;		/* Door Bell */
-	u8 __iomem *pcicfg;	/* PCI config space */
 
 	struct mutex mbox_lock; /* For serializing mbox cmds to BE card */
 	struct be_dma_mem mbox_mem;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 09eb699..2375c0c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -141,13 +141,15 @@ static int be_queue_alloc(struct be_adapter *adapter, struct be_queue_info *q,
 
 static void be_intr_set(struct be_adapter *adapter, bool enable)
 {
-	u8 __iomem *addr = adapter->pcicfg + PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET;
-	u32 reg = ioread32(addr);
-	u32 enabled = reg & MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
+	u32 reg, enabled;
 
 	if (adapter->eeh_err)
 		return;
 
+	pci_read_config_dword(adapter->pdev, PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET,
+				&reg);
+	enabled = reg & MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
+
 	if (!enabled && enable)
 		reg |= MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
 	else if (enabled && !enable)
@@ -155,7 +157,8 @@ static void be_intr_set(struct be_adapter *adapter, bool enable)
 	else
 		return;
 
-	iowrite32(reg, addr);
+	pci_write_config_dword(adapter->pdev,
+			PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET, reg);
 }
 
 static void be_rxq_notify(struct be_adapter *adapter, u16 qid, u16 posted)
@@ -2951,14 +2954,12 @@ static void be_unmap_pci_bars(struct be_adapter *adapter)
 		iounmap(adapter->csr);
 	if (adapter->db)
 		iounmap(adapter->db);
-	if (adapter->pcicfg && be_physfn(adapter))
-		iounmap(adapter->pcicfg);
 }
 
 static int be_map_pci_bars(struct be_adapter *adapter)
 {
 	u8 __iomem *addr;
-	int pcicfg_reg, db_reg;
+	int db_reg;
 
 	if (lancer_chip(adapter)) {
 		addr = ioremap_nocache(pci_resource_start(adapter->pdev, 0),
@@ -2978,10 +2979,8 @@ static int be_map_pci_bars(struct be_adapter *adapter)
 	}
 
 	if (adapter->generation == BE_GEN2) {
-		pcicfg_reg = 1;
 		db_reg = 4;
 	} else {
-		pcicfg_reg = 0;
 		if (be_physfn(adapter))
 			db_reg = 4;
 		else
@@ -2993,16 +2992,6 @@ static int be_map_pci_bars(struct be_adapter *adapter)
 		goto pci_map_err;
 	adapter->db = addr;
 
-	if (be_physfn(adapter)) {
-		addr = ioremap_nocache(
-				pci_resource_start(adapter->pdev, pcicfg_reg),
-				pci_resource_len(adapter->pdev, pcicfg_reg));
-		if (addr == NULL)
-			goto pci_map_err;
-		adapter->pcicfg = addr;
-	} else
-		adapter->pcicfg = adapter->db + SRIOV_VF_PCICFG_OFFSET;
-
 	return 0;
 pci_map_err:
 	be_unmap_pci_bars(adapter);
-- 
1.7.4

[PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Sathya Perla]

The rx_drops_no_frags HW counter for RSS rings is 16bits in HW and can
wraparound often. Maintain a 32-bit accumulator in the driver to prevent
frequent wraparound.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2375c0c..4e588d8 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -378,6 +378,17 @@ static void populate_lancer_stats(struct be_adapter *adapter)
 				pport_stats->rx_drops_too_many_frags_lo;
 }
 
+static void accumulate_16bit_val(u32 *acc, u16 val)
+{
+#define lo(x)			(x & 0xFFFF)
+#define hi(x)			(x & 0xFFFF0000)
+	bool wrapped = val < lo(*acc);
+
+	*acc = hi(*acc) + val;
+	if (wrapped)
+		*acc += 65536;
+}
+
 void be_parse_stats(struct be_adapter *adapter)
 {
 	struct be_erx_stats_v1 *erx = be_erx_stats_from_cmd(adapter);
@@ -394,9 +405,13 @@ void be_parse_stats(struct be_adapter *adapter)
 	}
 
 	/* as erx_v1 is longer than v0, ok to use v1 defn for v0 access */
-	for_all_rx_queues(adapter, rxo, i)
-		rx_stats(rxo)->rx_drops_no_frags =
-			erx->rx_drops_no_fragments[rxo->q.id];
+	for_all_rx_queues(adapter, rxo, i) {
+		/* below erx HW counter can actually wrap around after
+		 * 65535. Driver accumulates a 32-bit value
+		 */
+		accumulate_16bit_val(&rx_stats(rxo)->rx_drops_no_frags,
+				(u16)erx->rx_drops_no_fragments[rxo->q.id]);
+	}
 }
 
 static struct rtnl_link_stats64 *be_get_stats64(struct net_device *netdev,
-- 
1.7.4

[PATCH net-next 4/5] be2net: increase FW update completion timeout

[Sathya Perla]

Flashing some of the PHYs can take longer thus increasing the total flash
update time to a max of 40s.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index bec039d..bebeee6 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1939,7 +1939,7 @@ int be_cmd_write_flashrom(struct be_adapter *adapter, struct be_dma_mem *cmd,
 	spin_unlock_bh(&adapter->mcc_lock);
 
 	if (!wait_for_completion_timeout(&adapter->flash_compl,
-			msecs_to_jiffies(12000)))
+			msecs_to_jiffies(40000)))
 		status = -1;
 	else
 		status = adapter->flash_status;
-- 
1.7.4

[PATCH net-next 5/5] be2net: remove unused variable

[Sathya Perla]

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 868d7f4..c5f0516 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -347,7 +347,6 @@ struct be_adapter {
 	u32 beacon_state;	/* for set_phys_id */
 
 	bool eeh_err;
-	bool link_up;
 	u32 port_num;
 	bool promiscuous;
 	bool wol;
-- 
1.7.4

Re: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Eric Dumazet]

Le lundi 22 août 2011 à 16:43 +0530, Sathya Perla a écrit :
> The rx_drops_no_frags HW counter for RSS rings is 16bits in HW and can
> wraparound often. Maintain a 32-bit accumulator in the driver to prevent
> frequent wraparound.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
>  drivers/net/ethernet/emulex/benet/be_main.c |   21 ++++++++++++++++++---
>  1 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 2375c0c..4e588d8 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/f
> @@ -378,6 +378,17 @@ static void populate_lancer_stats(struct be_adapter *adapter)
>  				pport_stats->rx_drops_too_many_frags_lo;
>  }
>  
> +static void accumulate_16bit_val(u32 *acc, u16 val)
> +{
> +#define lo(x)			(x & 0xFFFF)
> +#define hi(x)			(x & 0xFFFF0000)
> +	bool wrapped = val < lo(*acc);
> +
> +	*acc = hi(*acc) + val;
> +	if (wrapped)
> +		*acc += 65536;
> +}
> +
>  

This adds a race for lockless SNMP readers (in be_get_stats64())

They can see the 32bit value going backward.

You have to be very careful to read the *acc once, and write it once.

Re: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Eric Dumazet]

Le lundi 22 août 2011 à 13:43 +0200, Eric Dumazet a écrit :

> This adds a race for lockless SNMP readers (in be_get_stats64())
> 
> They can see the 32bit value going backward.
> 
> You have to be very careful to read the *acc once, and write it once.

The "write once" is the only requirement.

Something like :

u32 newval = hi(*acc) + val;

if (wrapped)
	newval += 65536;
ACCESS_ONCE(*acc) = newval;

RE: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Sathya.Perla]

>-----Original Message-----
>From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
>Sent: Monday, August 22, 2011 5:18 PM
>
>Le lundi 22 août 2011 à 13:43 +0200, Eric Dumazet a écrit :
>
>> This adds a race for lockless SNMP readers (in be_get_stats64())
>>
>> They can see the 32bit value going backward.
>>
>> You have to be very careful to read the *acc once, and write it once.
>
>The "write once" is the only requirement.
>
>Something like :
>
>u32 newval = hi(*acc) + val;
>
>if (wrapped)
>	newval += 65536;
>ACCESS_ONCE(*acc) = newval;
>

Eric,

I'm wondering why you'd need ACCESS_ONCE() here? Wouldn't using the temp variable (newval) as you suggested,
ensure that the reader doesn't see a half-baked value?

RE: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Eric Dumazet]

Le lundi 22 août 2011 à 05:15 -0700, Sathya.Perla@Emulex.Com a écrit :
> >-----Original Message-----
> >From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> >Sent: Monday, August 22, 2011 5:18 PM
> >
> >Le lundi 22 août 2011 à 13:43 +0200, Eric Dumazet a écrit :
> >
> >> This adds a race for lockless SNMP readers (in be_get_stats64())
> >>
> >> They can see the 32bit value going backward.
> >>
> >> You have to be very careful to read the *acc once, and write it once.
> >
> >The "write once" is the only requirement.
> >
> >Something like :
> >
> >u32 newval = hi(*acc) + val;
> >
> >if (wrapped)
> >	newval += 65536;
> >ACCESS_ONCE(*acc) = newval;
> >
> 
> Eric,
> 
> I'm wondering why you'd need ACCESS_ONCE() here? Wouldn't using the temp variable (newval) as you suggested,
> ensure that the reader doesn't see a half-baked value?
> 

If you write :

u32 newval = hi(*acc) + val;
if (wrapped)
	newval += 65536;
*acc = newval;


C compiler (gcc) is free to eliminate the temp variable and to generate
same code than :

*acc = hi(*acc) + val;
if (wrapped)
	*acc += 65536;

ACCESS_ONCE() here is the clean way (and self explanatory/documented) to
keep compiler to write to *acc twice.

RE: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

[Sathya.Perla]

OK, thanks Eric. I'll re-spin this patch series ...

-Sathya
________________________________________
From: Eric Dumazet [eric.dumazet@gmail.com]
Sent: Monday, August 22, 2011 6:14 PM
To: Perla, Sathya
Cc: netdev@vger.kernel.org
Subject: RE: [PATCH net-next 3/5] be2net: fix erx->rx_drops_no_frags wrap around

Le lundi 22 août 2011 à 05:15 -0700, Sathya.Perla@Emulex.Com a écrit :
> >-----Original Message-----
> >From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> >Sent: Monday, August 22, 2011 5:18 PM
> >
> >Le lundi 22 août 2011 à 13:43 +0200, Eric Dumazet a écrit :
> >
> >> This adds a race for lockless SNMP readers (in be_get_stats64())
> >>
> >> They can see the 32bit value going backward.
> >>
> >> You have to be very careful to read the *acc once, and write it once.
> >
> >The "write once" is the only requirement.
> >
> >Something like :
> >
> >u32 newval = hi(*acc) + val;
> >
> >if (wrapped)
> >     newval += 65536;
> >ACCESS_ONCE(*acc) = newval;
> >
>
> Eric,
>
> I'm wondering why you'd need ACCESS_ONCE() here? Wouldn't using the temp variable (newval) as you suggested,
> ensure that the reader doesn't see a half-baked value?
>

If you write :

u32 newval = hi(*acc) + val;
if (wrapped)
        newval += 65536;
*acc = newval;


C compiler (gcc) is free to eliminate the temp variable and to generate
same code than :

*acc = hi(*acc) + val;
if (wrapped)
        *acc += 65536;

ACCESS_ONCE() here is the clean way (and self explanatory/documented) to
keep compiler to write to *acc twice.

[PATCH net-next 2/5] be2net: get rid of memory mapped pci-cfg space address

[Sathya Perla]

Get rid of adapter->pcicfg and its use. Use pci_config_read/write_dword()
instead.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h      |    1 -
 drivers/net/ethernet/emulex/benet/be_main.c |   27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 12b5b51..868d7f4 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -298,7 +298,6 @@ struct be_adapter {
 
 	u8 __iomem *csr;
 	u8 __iomem *db;		/* Door Bell */
-	u8 __iomem *pcicfg;	/* PCI config space */
 
 	struct mutex mbox_lock; /* For serializing mbox cmds to BE card */
 	struct be_dma_mem mbox_mem;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 09eb699..2375c0c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -141,13 +141,15 @@ static int be_queue_alloc(struct be_adapter *adapter, struct be_queue_info *q,
 
 static void be_intr_set(struct be_adapter *adapter, bool enable)
 {
-	u8 __iomem *addr = adapter->pcicfg + PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET;
-	u32 reg = ioread32(addr);
-	u32 enabled = reg & MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
+	u32 reg, enabled;
 
 	if (adapter->eeh_err)
 		return;
 
+	pci_read_config_dword(adapter->pdev, PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET,
+				&reg);
+	enabled = reg & MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
+
 	if (!enabled && enable)
 		reg |= MEMBAR_CTRL_INT_CTRL_HOSTINTR_MASK;
 	else if (enabled && !enable)
@@ -155,7 +157,8 @@ static void be_intr_set(struct be_adapter *adapter, bool enable)
 	else
 		return;
 
-	iowrite32(reg, addr);
+	pci_write_config_dword(adapter->pdev,
+			PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET, reg);
 }
 
 static void be_rxq_notify(struct be_adapter *adapter, u16 qid, u16 posted)
@@ -2951,14 +2954,12 @@ static void be_unmap_pci_bars(struct be_adapter *adapter)
 		iounmap(adapter->csr);
 	if (adapter->db)
 		iounmap(adapter->db);
-	if (adapter->pcicfg && be_physfn(adapter))
-		iounmap(adapter->pcicfg);
 }
 
 static int be_map_pci_bars(struct be_adapter *adapter)
 {
 	u8 __iomem *addr;
-	int pcicfg_reg, db_reg;
+	int db_reg;
 
 	if (lancer_chip(adapter)) {
 		addr = ioremap_nocache(pci_resource_start(adapter->pdev, 0),
@@ -2978,10 +2979,8 @@ static int be_map_pci_bars(struct be_adapter *adapter)
 	}
 
 	if (adapter->generation == BE_GEN2) {
-		pcicfg_reg = 1;
 		db_reg = 4;
 	} else {
-		pcicfg_reg = 0;
 		if (be_physfn(adapter))
 			db_reg = 4;
 		else
@@ -2993,16 +2992,6 @@ static int be_map_pci_bars(struct be_adapter *adapter)
 		goto pci_map_err;
 	adapter->db = addr;
 
-	if (be_physfn(adapter)) {
-		addr = ioremap_nocache(
-				pci_resource_start(adapter->pdev, pcicfg_reg),
-				pci_resource_len(adapter->pdev, pcicfg_reg));
-		if (addr == NULL)
-			goto pci_map_err;
-		adapter->pcicfg = addr;
-	} else
-		adapter->pcicfg = adapter->db + SRIOV_VF_PCICFG_OFFSET;
-
 	return 0;
 pci_map_err:
 	be_unmap_pci_bars(adapter);
-- 
1.7.4