netdev - Re: [PATCH] forcedeth: mgmt unit interface changes

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: <20090127172705.d6ad4c56.akpm@linux-foundation.org>
Date:	Tue, 27 Jan 2009 17:27:05 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Ayaz Abdulla <aabdulla@...dia.com>
Cc:	manfred@...orfullife.com, jgarzik@...ox.com, netdev@...r.kernel.org
Subject: Re: [PATCH] forcedeth: mgmt unit interface changes

On Fri, 23 Jan 2009 16:01:58 -0500
Ayaz Abdulla <aabdulla@...dia.com> wrote:

> +static int nv_mgmt_get_version(struct net_device *dev)
> +{
> +	struct fe_priv *np = netdev_priv(dev);
> +	u8 __iomem *base = get_hwbase(dev);
> +	u32 data_ready = readl(base + NvRegTransmitterControl);
> +	u32 data_ready2;
> +	int i;
> +
> +	writel(NVREG_MGMTUNITGETVERSION, base + NvRegMgmtUnitGetVersion);
> +	writel(data_ready ^ NVREG_XMITCTL_DATA_START, base + NvRegTransmitterControl);
> +	for (i = 0; i < 1000000; i++) {
> +		data_ready2 = readl(base + NvRegTransmitterControl);
> +		if ((data_ready & NVREG_XMITCTL_DATA_READY) != (data_ready2 & NVREG_XMITCTL_DATA_READY))
> +			break;
> +		udelay(50);
> +	}
> +
> +	if (i == 1000000 || (data_ready2 & NVREG_XMITCTL_DATA_ERROR))
> +		return 0;
> +
> +	np->mgmt_version = readl(base + NvRegMgmtUnitVersion) & NVREG_MGMTUNITVERSION;
> +
> +	return 1;
> +}

whee, a 50 second busy-wait.

Unnecessarily, afacit.  The sole caller calls this function from
->probe without any locks held?

I'd suggest that we

a) use schedule_timeout_uninterruptible(1) and

b) add a bit of user feedback (printk(".")?) so they don't get bored
and hit the reset button (remember those?)

--
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