netdev
[Top] [All Lists]

Re: [PATCH/RFC] enabling netdev boot options

To: Andrew Morton <akpm@xxxxxxxx>
Subject: Re: [PATCH/RFC] enabling netdev boot options
From: "Randy.Dunlap" <rddunlap@xxxxxxxx>
Date: Sat, 8 Nov 2003 17:03:42 -0800
Cc: netdev@xxxxxxxxxxx, saw@xxxxxxxxxxxxx
In-reply-to: <20031108162248.5846ab46.akpm@osdl.org>
Organization: OSDL
References: <20031108160416.236ec29d.rddunlap@osdl.org> <20031108162248.5846ab46.akpm@osdl.org>
Sender: netdev-bounce@xxxxxxxxxxx
On Sat, 8 Nov 2003 16:22:48 -0800 Andrew Morton <akpm@xxxxxxxx> wrote:

| "Randy.Dunlap" <rddunlap@xxxxxxxx> wrote:
| >
| > 
| > I have modified 3c59x.c (for 2.4.22 and 2.6.0-test9) and eepro100
| > (for 2.6.0-test9 only) to check for netdev= boot options.
| 
| A worthy project, thanks.
| 
| > +   rtnl_lock();
| > +
| > +   retval = dev_alloc_name(dev, dev->name);
| > +   if (retval < 0)
| > +           goto err_free_unlock;
| > +
| > +   rtnl_unlock();
| 
| It would be better to move this locking into the core net layer. 
| Call dev_alloc_name_which_takes_rtnl_lock() here ;)

Makes sense, but I'm wary of some drivers which do rtnl_lock()
early and hold it for long times (like eepro100).
Methinks that it shouldn't do that, but that's a different patch.

| > @@ -1351,8 +1362,10 @@ free_ring:
| >  free_region:
| >     if (vp->must_free_region)
| >             release_region(ioaddr, vci->io_size);
| > -   kfree (dev);
| > +err_free_unlock:
| >     printk(KERN_ERR PFX "vortex_probe1 fails.  Returns %d\n", retval);
| > +   rtnl_unlock();
| > +   kfree (dev);
| >  out:
| >     return retval;
| >  }
| 
| This causes an rtnl_unlock() imbalance if vortex_probe1() takes the
| `goto free_region' path, does it not??

I'll check that again.  Might be right.

--
~Randy

<Prev in Thread] Current Thread [Next in Thread>