On Wed, 2014-09-17 at 11:02 +0200, Bernhard Kaindl wrote:
> Remove a source of latency spikes (in my case up to 10ms) by not
> calling
> code that uses mdelay() for feeding a phy statistic (rx errors for
> idle
> symbols - not data -> idle_errors) while being called with a spinlock
> held.
>
> As idle_errors isn't read, this patch only removes unused code and
> data.
>
> Later, more complicated changes may be applied to address the spinlock
> and
> allow for some PHY diagnostics by harvesting this PHY stats register
> fully.
>
> This patch is designed to fix the issue and be safe for
> longterm/stable.
>
> For the Intel e1000e driver, the same change was applied in 2008 with
> commit 23033fad5be0 ("e1000e: remove phy read from inside spinlock").
>
> The mdelay is triggered by HW/SW semaphores, thus it depends on the
> HW.
>
> I've HW that triggers it even when idle. Others may trigger it only
> e.g.
> when Ethernet ports aquire or loose the link or on ifconfig up / down.
> We've noticed this first from delays in frame rx/tx due to the
> mdelay().
>
> Example command for checking if the issue is triggered: cyclictest
> -Smp1
> (Look for occasional "Max:" values > 4000 or use -b 4000 to stop if
> greater)
>
> It was observed with I350 ports connected to other I350 ports, but not
> if driver and EEPROM was modified to run the I350 in EEPROM-less mode.
>
> phy_stats.idle_errors and .receive_errors (isn't touched) occupy 64
> not
> used bits in the adapter struct: Their allocation may be removed as
> well.
>
> Signed-off-by: Bernhard Kaindl <[email protected]>
> Fixes: 12dcd86b75d5 ("igb: fix stats handling") (this added the
> spin_lock)
> Cc: Jeff Kirsher <[email protected]>
> Cc: Jesse Brandeburg <[email protected]>
> Cc: Carolyn Wyborny <[email protected]>
> Cc: Todd Fujinaka <[email protected]>
> Cc: [email protected]
> ---
> drivers/net/ethernet/intel/igb/e1000_hw.h | 5 -----
> drivers/net/ethernet/intel/igb/igb.h | 1 -
> drivers/net/ethernet/intel/igb/igb_main.c | 12 ------------
> 3 files changed, 18 deletions(-)Thanks Bernhard, I have added your patch to my queue.
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------------ Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
