On Fri, 2010-09-03 at 00:02 +0800, Liang Li wrote:
> It's common sense that when we should do change to driver ring
> desc/buffer etc only after 'stop/shutdown' the device. When we
> do change while devices/driver is running, kernel oops occur:
[...]
> + printk(KERN_INFO "Reactivating interface %s.\n", netdev->name);
> + ret = ucc_geth_open(netdev);
> + if (ret) {
> + printk(KERN_WARNING "uec_set_ringparam: set ring param
> for running"
> + " interface %s failed. Please try
> again.\n", netdev->name);
> + dev_close(netdev);
[...]
If ucc_geth_open() failed you MUST NOT call ucc_geth_close(), but that
is what dev_close() is going to do. But the device is still flagged as
running so 'ifconfig down' is going to call dev_close() as well. There
is no way out.
This is why I said you must call dev_close() and then dev_open()
instead. Then if dev_open() fails, just print the error, e.g.:
dev_close(netdev);
ret = dev_open(netdev);
if (ret)
netdev_err(netdev,
"uec_set_ringparam: failed to restart"
" interface with new ring parameters\n");
(And I think this really is a serious error, hence the 'err' rather than
'warning' severity.)
(By the way, I noticed there are other places where ucc_geth_close() and
ucc_geth_open() are called, without error checking. These are also
bugs, but that doesn't justify adding new bugs.)
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
