netdev - Re: Spurious timeouts in mvmdio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131203234209.GW16735@n2100.arm.linux.org.uk>
Date:	Tue, 3 Dec 2013 23:42:09 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Leigh Brown <leigh@...inno.co.uk>
Cc:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Nicolas Schichan <nschichan@...ebox.fr>,
	Jason Cooper <jason@...edaemon.net>, netdev@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Florian Fainelli <florian@...nwrt.org>,
	"David S. Miller" <davem@...emloft.net>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: Spurious timeouts in mvmdio

On Tue, Dec 03, 2013 at 11:38:23PM +0000, Leigh Brown wrote:
> On 2013-12-03 23:17, Sebastian Hesselbarth wrote:
>> 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.

Alternatively, code it like this instead.

 drivers/net/ethernet/marvell/mvmdio.c |   32 +++++++++++++++-----------------
 1 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 7354960b583b..a6f59831fc5b 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -76,31 +76,29 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)
 {
 	struct orion_mdio_dev *dev = bus->priv;
 	unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);
-	unsigned long end = jiffies + timeout;
-	int timedout = 0;
 
-	while (1) {
-	        if (orion_mdio_smi_is_done(dev))
+	if (dev->err_interrupt > 0) {
+		if (wait_event_timeout(dev->smi_busy_wait,
+				       orion_mdio_smi_is_done(dev),
+				       1 + timeout))
 			return 0;
-	        else if (timedout)
-			break;
+	} else {
+		unsigned long end = jiffies + timeout;
 
-	        if (dev->err_interrupt <= 0) {
-			usleep_range(MVMDIO_SMI_POLL_INTERVAL_MIN,
-				     MVMDIO_SMI_POLL_INTERVAL_MAX);
+		while (1) {
+	        	if (orion_mdio_smi_is_done(dev))
+				return 0;
 
 			if (time_is_before_jiffies(end))
-				++timedout;
-	        } else {
-			wait_event_timeout(dev->smi_busy_wait,
-				           orion_mdio_smi_is_done(dev),
-				           timeout);
-
-			++timedout;
-	        }
+				break;
+
+			usleep_range(MVMDIO_SMI_POLL_INTERVAL_MIN,
+				     MVMDIO_SMI_POLL_INTERVAL_MAX);
+		}
 	}
 
 	dev_err(bus->parent, "Timeout: SMI busy for too long\n");
+
 	return  -ETIMEDOUT;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html