Re: Spurious timeouts in mvmdio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/04/2013 12:38 AM, Leigh Brown wrote:
On 2013-12-03 23:17, Sebastian Hesselbarth wrote:
On 12/04/2013 12:20 AM, Leigh Brown wrote:
On 2013-12-03 22:45, Sebastian Hesselbarth wrote:
On 12/03/2013 09:57 PM, Leigh Brown wrote:
[...]
Nicolas' patch should fix the issue, but I prefer the following as
it is
more
correct, as it only adjusts the timeout when calling
wait_event_timeout().  As
I said above,I believe the polling code is correct.

diff --git a/drivers/net/ethernet/marvell/mvmdio.c
b/drivers/net/ethernet/marvell/mvmdio.c
index 7354960..b187c08 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -92,6 +92,14 @@ static int orion_mdio_wait_ready(struct mii_bus
*bus)
              if (time_is_before_jiffies(end))
                  ++timedout;
              } else {
+            /*
+             * wait_event_timeout does not guarantee a delay of at
+             * least one whole jiffie, so timeout must be no less
+             * than two.
+             */
+            if (timeout < 2)
+                timeout = 2;

If you always want to wait at least two jiffies, why not just increase
TIMEOUT makro to 20ms instead of messing here with it again?
As said on IRC log above, originally timeout was 100ms.

You could do that, but would you not feel bad leaving a latent bug in
the code?
I know it's unlikely that someone would set HZ to 50, but if they did,
the same
bug would appear again.

If you want to ensure timeout > 2, why not then just use:

-     unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);
+     unsigned long timeout = 1 + usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);

This will make it correct when using interrupts but it will make the
loop wait one
jiffie longer than it should when polling.

Leigh,

it is not about "waiting longer than it should" but increasing the
_timeout_ to more than one jiffy. If you poll and everything is fine
with mdio, you exit way before timeout. If mdio hangs, it does not
really matter if you timeout after 10, 20, or even 100ms.

Anyway, I am fine with every patch that increases timeout to more than
one jiffy. Also, Russell just sent a patch to separate irq/polling
completely. Maybe give it a try.

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Discussion]     [TCP Instrumentation]     [Ethernet Bridging]     [Linux Wireless Networking]     [Linux WPAN Networking]     [Linux Host AP]     [Linux WPAN Networking]     [Linux Bluetooth Networking]     [Linux ATH6KL Networking]     [Linux Networking Users]     [Linux Coverity]     [VLAN]     [Git]     [IETF Annouce]     [Linux Assembly]     [Security]     [Bugtraq]     [Yosemite Information]     [MIPS Linux]     [ARM Linux Kernel]     [ARM Linux]     [Linux Virtualization]     [Linux IDE]     [Linux RAID]     [Linux SCSI]