* [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized
@ 2009-12-12 7:51 Jeff Kirsher
2009-12-12 7:51 ` [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters Jeff Kirsher
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jeff Kirsher @ 2009-12-12 7:51 UTC (permalink / raw
To: davem; +Cc: netdev, gospo, Peter P Waskiewicz Jr, Jeff Kirsher
From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
tc is still throwing a warning that is could be used
uninitialized. This fixes it.
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 35ea8c9..d561fdf 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -258,7 +258,7 @@ static inline bool ixgbe_tx_is_paused(struct ixgbe_adapter *adapter,
#ifdef CONFIG_IXGBE_DCB
if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
- int tc;
+ int tc = 0;
int reg_idx = tx_ring->reg_idx;
int dcb_i = adapter->ring_feature[RING_F_DCB].indices;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters
2009-12-12 7:51 [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized Jeff Kirsher
@ 2009-12-12 7:51 ` Jeff Kirsher
2009-12-14 3:18 ` David Miller
2009-12-12 7:52 ` [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation Jeff Kirsher
2009-12-14 3:20 ` [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized David Miller
2 siblings, 1 reply; 10+ messages in thread
From: Jeff Kirsher @ 2009-12-12 7:51 UTC (permalink / raw
To: davem
Cc: netdev, gospo, Mallikarjuna R Chilakala, Peter P Waskiewicz Jr,
Jeff Kirsher
From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Fix tx_restart_queue & non_eop_desc stats counters, counters were not
getting incremented as expected with the traffic
Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index d561fdf..65eec2d 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -4511,6 +4511,7 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
struct ixgbe_hw *hw = &adapter->hw;
u64 total_mpc = 0;
u32 i, missed_rx = 0, mpc, bprc, lxon, lxoff, xon_off_tot;
+ u64 non_eop_descs = 0, restart_queue = 0;
if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
u64 rsc_count = 0;
@@ -4528,10 +4529,12 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
/* gather some stats to the adapter struct that are per queue */
for (i = 0; i < adapter->num_tx_queues; i++)
- adapter->restart_queue += adapter->tx_ring[i].restart_queue;
+ restart_queue += adapter->tx_ring[i]->restart_queue;
+ adapter->restart_queue = restart_queue;
for (i = 0; i < adapter->num_rx_queues; i++)
- adapter->non_eop_descs += adapter->tx_ring[i].non_eop_descs;
+ non_eop_descs += adapter->rx_ring[i]->non_eop_descs;
+ adapter->non_eop_descs = non_eop_descs;
adapter->stats.crcerrs += IXGBE_READ_REG(hw, IXGBE_CRCERRS);
for (i = 0; i < 8; i++) {
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation
2009-12-12 7:51 [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized Jeff Kirsher
2009-12-12 7:51 ` [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters Jeff Kirsher
@ 2009-12-12 7:52 ` Jeff Kirsher
2009-12-14 3:18 ` David Miller
2009-12-14 3:20 ` [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized David Miller
2 siblings, 1 reply; 10+ messages in thread
From: Jeff Kirsher @ 2009-12-12 7:52 UTC (permalink / raw
To: davem
Cc: netdev, gospo, Mallikarjuna R Chilakala, Peter P Waskiewicz Jr,
Jeff Kirsher
From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Fix 82598 copper link issue, where the phy prematurely indicates link
before it is ready to process packets. The new function looks for phy
link and indicates that, when it is available. If phy is not ready
within few seconds of MAC indicating link, the function will return
failure which translates to link down indication
Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_82598.c | 38 ++++++++++++++++++++++++++++++++++++++
drivers/net/ixgbe/ixgbe_type.h | 2 ++
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_82598.c b/drivers/net/ixgbe/ixgbe_82598.c
index e2d5343..1cc91f8 100644
--- a/drivers/net/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ixgbe/ixgbe_82598.c
@@ -510,6 +510,40 @@ static s32 ixgbe_start_mac_link_82598(struct ixgbe_hw *hw,
}
/**
+ * ixgbe_validate_link_ready - Function looks for phy link
+ * @hw: pointer to hardware structure
+ *
+ * Function indicates success when phy link is available. If phy is not ready
+ * within 5 seconds of MAC indicating link, the function returns error.
+ **/
+static s32 ixgbe_validate_link_ready(struct ixgbe_hw *hw)
+{
+ u32 timeout;
+ u16 an_reg;
+
+ if (hw->device_id != IXGBE_DEV_ID_82598AT2)
+ return 0;
+
+ for (timeout = 0;
+ timeout < IXGBE_VALIDATE_LINK_READY_TIMEOUT; timeout++) {
+ hw->phy.ops.read_reg(hw, MDIO_STAT1, MDIO_MMD_AN, &an_reg);
+
+ if ((an_reg & MDIO_AN_STAT1_COMPLETE) &&
+ (an_reg & MDIO_STAT1_LSTATUS))
+ break;
+
+ mdelay(100);
+ }
+
+ if (timeout == IXGBE_VALIDATE_LINK_READY_TIMEOUT) {
+ hw_dbg(hw, "Link was indicated but link is down\n");
+ return IXGBE_ERR_LINK_SETUP;
+ }
+
+ return 0;
+}
+
+/**
* ixgbe_check_mac_link_82598 - Get link/speed status
* @hw: pointer to hardware structure
* @speed: pointer to link speed
@@ -589,6 +623,10 @@ static s32 ixgbe_check_mac_link_82598(struct ixgbe_hw *hw,
else
*speed = IXGBE_LINK_SPEED_1GB_FULL;
+ if ((hw->device_id == IXGBE_DEV_ID_82598AT2) && (*link_up == true) &&
+ (ixgbe_validate_link_ready(hw) != 0))
+ *link_up = false;
+
/* if link is down, zero out the current_mode */
if (*link_up == false) {
hw->fc.current_mode = ixgbe_fc_none;
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index f3e8d52..84650c6 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -841,6 +841,8 @@
#define IXGBE_MPVC 0x04318
#define IXGBE_SGMIIC 0x04314
+#define IXGBE_VALIDATE_LINK_READY_TIMEOUT 50
+
/* Omer CORECTL */
#define IXGBE_CORECTL 0x014F00
/* BARCTRL */
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters
2009-12-12 7:51 ` [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters Jeff Kirsher
@ 2009-12-14 3:18 ` David Miller
2009-12-14 4:57 ` Peter P Waskiewicz Jr
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2009-12-14 3:18 UTC (permalink / raw
To: jeffrey.t.kirsher
Cc: netdev, gospo, mallikarjuna.chilakala, peter.p.waskiewicz.jr
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 11 Dec 2009 23:51:41 -0800
> @@ -4528,10 +4529,12 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
>
> /* gather some stats to the adapter struct that are per queue */
> for (i = 0; i < adapter->num_tx_queues; i++)
> - adapter->restart_queue += adapter->tx_ring[i].restart_queue;
> + restart_queue += adapter->tx_ring[i]->restart_queue;
> + adapter->restart_queue = restart_queue;
I don't see how these two versions of the code can behave
differently unless there is very broken locking on the
adapter->* statistic counters.
I'm not applying this without at least a better commit
log message.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation
2009-12-12 7:52 ` [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation Jeff Kirsher
@ 2009-12-14 3:18 ` David Miller
2009-12-14 18:37 ` Malli
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2009-12-14 3:18 UTC (permalink / raw
To: jeffrey.t.kirsher
Cc: netdev, gospo, mallikarjuna.chilakala, peter.p.waskiewicz.jr
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 11 Dec 2009 23:52:01 -0800
> + for (timeout = 0;
> + timeout < IXGBE_VALIDATE_LINK_READY_TIMEOUT; timeout++) {
> + hw->phy.ops.read_reg(hw, MDIO_STAT1, MDIO_MMD_AN, &an_reg);
> +
> + if ((an_reg & MDIO_AN_STAT1_COMPLETE) &&
> + (an_reg & MDIO_STAT1_LSTATUS))
> + break;
> +
> + mdelay(100);
> + }
An up to 5000 msec non-preemptable spin loop.
If this used "msleep()" that would make it OK.
But as it is, sorry, no way.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized
2009-12-12 7:51 [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized Jeff Kirsher
2009-12-12 7:51 ` [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters Jeff Kirsher
2009-12-12 7:52 ` [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation Jeff Kirsher
@ 2009-12-14 3:20 ` David Miller
2009-12-14 4:55 ` Peter P Waskiewicz Jr
2 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2009-12-14 3:20 UTC (permalink / raw
To: jeffrey.t.kirsher; +Cc: netdev, gospo, peter.p.waskiewicz.jr
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 11 Dec 2009 23:51:23 -0800
> From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
>
> tc is still throwing a warning that is could be used
> uninitialized. This fixes it.
>
> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
It's warning because it absolutely can be used uninitialized.
The two code blocks only handle two chip variants. Why not
make what's happening here explicit by using a switch
statement and having a 'default' case?
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized
2009-12-14 3:20 ` [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized David Miller
@ 2009-12-14 4:55 ` Peter P Waskiewicz Jr
0 siblings, 0 replies; 10+ messages in thread
From: Peter P Waskiewicz Jr @ 2009-12-14 4:55 UTC (permalink / raw
To: David Miller; +Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, gospo@redhat.com
On Sun, 2009-12-13 at 19:20 -0800, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 11 Dec 2009 23:51:23 -0800
>
> > From: PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> >
> > tc is still throwing a warning that is could be used
> > uninitialized. This fixes it.
> >
> > Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> It's warning because it absolutely can be used uninitialized.
>
> The two code blocks only handle two chip variants. Why not
> make what's happening here explicit by using a switch
> statement and having a 'default' case?
That I can do, and makes more sense. I'll respin the patch.
Thanks Dave,
-PJ
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters
2009-12-14 3:18 ` David Miller
@ 2009-12-14 4:57 ` Peter P Waskiewicz Jr
2009-12-14 5:07 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Peter P Waskiewicz Jr @ 2009-12-14 4:57 UTC (permalink / raw
To: David Miller
Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, gospo@redhat.com,
Chilakala, Mallikarjuna
On Sun, 2009-12-13 at 19:18 -0800, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 11 Dec 2009 23:51:41 -0800
>
> > @@ -4528,10 +4529,12 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
> >
> > /* gather some stats to the adapter struct that are per queue */
> > for (i = 0; i < adapter->num_tx_queues; i++)
> > - adapter->restart_queue += adapter->tx_ring[i].restart_queue;
> > + restart_queue += adapter->tx_ring[i]->restart_queue;
> > + adapter->restart_queue = restart_queue;
>
> I don't see how these two versions of the code can behave
> differently unless there is very broken locking on the
> adapter->* statistic counters.
The problem is the adapter->restart_queue is being double-counted. If
we accumulate from each ring's restart_queue, which are cumulative, then
we don't want to add it to the previous update_stats() values in
adapter->restart_queue.
> I'm not applying this without at least a better commit
> log message.
We can fix the commit log message to better explain what the problem is.
Thanks,
-PJ
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters
2009-12-14 4:57 ` Peter P Waskiewicz Jr
@ 2009-12-14 5:07 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2009-12-14 5:07 UTC (permalink / raw
To: peter.p.waskiewicz.jr
Cc: jeffrey.t.kirsher, netdev, gospo, mallikarjuna.chilakala
From: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Date: Sun, 13 Dec 2009 20:57:05 -0800
> On Sun, 2009-12-13 at 19:18 -0800, David Miller wrote:
>> I don't see how these two versions of the code can behave
>> differently unless there is very broken locking on the
>> adapter->* statistic counters.
>
> The problem is the adapter->restart_queue is being double-counted. If
> we accumulate from each ring's restart_queue, which are cumulative, then
> we don't want to add it to the previous update_stats() values in
> adapter->restart_queue.
Now I understand, thanks.
>> I'm not applying this without at least a better commit
>> log message.
>
> We can fix the commit log message to better explain what the problem is.
Thanks PJ.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation
2009-12-14 3:18 ` David Miller
@ 2009-12-14 18:37 ` Malli
0 siblings, 0 replies; 10+ messages in thread
From: Malli @ 2009-12-14 18:37 UTC (permalink / raw
To: David Miller
Cc: jeffrey.t.kirsher, netdev, gospo, mallikarjuna.chilakala,
peter.p.waskiewicz.jr
Thanks Dave. I'll respin the patch, didn't mean to wait in non-preemptive loop.
On Sun, Dec 13, 2009 at 7:18 PM, David Miller <davem@davemloft.net> wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 11 Dec 2009 23:52:01 -0800
>
>> + for (timeout = 0;
>> + timeout < IXGBE_VALIDATE_LINK_READY_TIMEOUT; timeout++) {
>> + hw->phy.ops.read_reg(hw, MDIO_STAT1, MDIO_MMD_AN, &an_reg);
>> +
>> + if ((an_reg & MDIO_AN_STAT1_COMPLETE) &&
>> + (an_reg & MDIO_STAT1_LSTATUS))
>> + break;
>> +
>> + mdelay(100);
>> + }
>
> An up to 5000 msec non-preemptable spin loop.
>
> If this used "msleep()" that would make it OK.
>
> But as it is, sorry, no way.
> --
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-12-14 18:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-12 7:51 [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized Jeff Kirsher
2009-12-12 7:51 ` [net-2.6 PATCH 2/3] ixgbe: Fix tx_restart_queue/non_eop_desc statistics counters Jeff Kirsher
2009-12-14 3:18 ` David Miller
2009-12-14 4:57 ` Peter P Waskiewicz Jr
2009-12-14 5:07 ` David Miller
2009-12-12 7:52 ` [net-2.6 PATCH 3/3] Fix 82598 premature copper PHY link indicatation Jeff Kirsher
2009-12-14 3:18 ` David Miller
2009-12-14 18:37 ` Malli
2009-12-14 3:20 ` [net-2.6 PATCH 1/3] ixgbe: Fix compiler warning about variable being used uninitialized David Miller
2009-12-14 4:55 ` Peter P Waskiewicz Jr
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.