Possible init bug in ibm_newemac/core.c

Possible init bug in ibm_newemac/core.c

[Darcy Watkins]

Hello,

In function...

static int __devinit emac_probe(struct of_device *ofdev,
				const struct of_device_id *match)


... in 

   drivers/net/ibm_newemac/core.c

... there is a chunk of code as follows (The // DLW - BUG ... comment is
mine)  ...

	/* Find PHY if any */
	// DLW - BUG - This uses an uninitialized spinlock (potential badness).
	err = emac_init_phy(dev);
	if (err != 0)
		goto err_detach_tah;

	/* Fill in the driver function table */
	ndev->open = &emac_open;
	if (dev->tah_dev)
		ndev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
	ndev->tx_timeout = &emac_tx_timeout;
	ndev->watchdog_timeo = 5 * HZ;
	ndev->stop = &emac_close;
	ndev->get_stats = &emac_stats;
	ndev->set_multicast_list = &emac_set_multicast_list;
	ndev->do_ioctl = &emac_ioctl;
	if (emac_phy_supports_gige(dev->phy_mode)) {
		ndev->hard_start_xmit = &emac_start_xmit_sg;
		ndev->change_mtu = &emac_change_mtu;
		dev->commac.ops = &emac_commac_sg_ops;
	} else {
		ndev->hard_start_xmit = &emac_start_xmit;
	}
	SET_ETHTOOL_OPS(ndev, &emac_ethtool_ops);

	netif_carrier_off(ndev);
	netif_stop_queue(ndev);

	err = register_netdev(ndev);
	if (err) {
		printk(KERN_ERR "%s: failed to register net device (%d)!\n",
		       np->full_name, err);
		goto err_detach_tah;
	}

init_emac_phy() uses a spinlock that isn't initialized until
register_netdev() is invoked.  I tried moving the init_emac_phy() to be
after the register_netdev().  The fix appeared to work on one board
variant (with two Ethernet ports) but kernel panics during boot on
another board variant (with one Etherent port and less memory).

So I have reverted my lame hack attempt and am reporting this so that an
expert can check into it.

The bug normally goes unnoticed until you turn on spinlock and/or
rtmutex debuggging in the kernel config - then the debugging magic
checks will catch it during boot.

This is based on kernel 2.6.25.8-rt7 and then upgraded to be
2.6.25.13-rt7 using incremental patches - running on an AMCC PPC405EP
but I think you could reproduce it using 2.6.25.8-rt7 (or possibly other
kernel versions).


-- 


Regards,

Darcy

--------------
Darcy L. Watkins - Senior Software Developer
Tranzeo Wireless Technologies, Inc.
19273 Fraser Way, Pitt Meadows, BC, Canada V3Y 2V4
T:604-460-6002 ext:410
http://www.tranzeo.com

Re: Possible init bug in ibm_newemac/core.c

[Benjamin Krill]

Hi Darcy,

>The bug normally goes unnoticed until you turn on spinlock and/or
>rtmutex debuggging in the kernel config - then the debugging magic
>checks will catch it during boot.

Have a look at thread:
http://ozlabs.org/pipermail/linuxppc-dev/2008-August/061631.html

cheers
 ben

Re: Possible init bug in ibm_newemac/core.c

[Darcy Watkins]

Hi Ben,

Thanks for the pointer the the thread.  What they did was exactly what I
did (and then later reverted).  I guess my "lame hack attempt" wasn't so
lame after all.

It works fine on my "mule" board with more memory and two Ethernet
ports, but fails on the production board having less memory and only one
Ethernet port.

This probably means that I need to look into something else.

In our setup, the DTS always says two Ethernet ports.  It may be that
the failure of the probe in the case of the non-existent Ethernet port
causes the kernel panic when the order of execution of those two
functions are reversed.

Thanks again.  This gives me a few more leads to chase down.

Darcy


On Fri, 2008-08-15 at 10:48 -0700, Benjamin Krill wrote:
> Hi Darcy,
> 
> >The bug normally goes unnoticed until you turn on spinlock and/or
> >rtmutex debuggging in the kernel config - then the debugging magic
> >checks will catch it during boot.
> 
> Have a look at thread:
> http://ozlabs.org/pipermail/linuxppc-dev/2008-August/061631.html
> 
> cheers
>  ben
> 

-- 


Regards,

Darcy

--------------
Darcy L. Watkins - Senior Software Developer
Tranzeo Wireless Technologies, Inc.
19273 Fraser Way, Pitt Meadows, BC, Canada V3Y 2V4
T:604-460-6002 ext:410
http://www.tranzeo.com

Re: Possible init bug in ibm_newemac/core.c

[Darcy Watkins]

Good news!  The patch in the URL is what I tried earlier (as mentioned
below).  The one in the followup post on the thread - I didn't try,
until now.  It appears to work properly on both boards.

Cheers and Thanks!

On Fri, 2008-08-15 at 13:00 -0700, Darcy Watkins wrote:
> --snip!-- What they did was exactly what I
> did (and then later reverted).  
--snip!--

> On Fri, 2008-08-15 at 10:48 -0700, Benjamin Krill wrote:
--snip!--
> > Have a look at thread:
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-August/061631.html
--snip!--
-- 


Regards,

Darcy

--------------
Darcy L. Watkins - Senior Software Developer
Tranzeo Wireless Technologies, Inc.
19273 Fraser Way, Pitt Meadows, BC, Canada V3Y 2V4
T:604-460-6002 ext:410
http://www.tranzeo.com