[i2c] [PATCH] i2c: Clean up core locking

[Jean Delvare]

Hi David,

On Thu, 6 Dec 2007 08:39:22 -0800, David Brownell wrote:
> On Thursday 06 December 2007, Jean Delvare wrote:
> > ... limit the code sections where
> > we hold that mutex to the minimum.
> 
> That code section should be a bit larger in one case, see below.
> 
> (...)
> > @@ -618,8 +616,6 @@ void i2c_del_driver(struct i2c_driver *d
> >  	struct i2c_client  *client;
> >  	struct i2c_adapter *adap;
> >  
> > -	mutex_lock(&core_lists);
> > -
> >  	/* new-style driver? */
> >  	if (is_newstyle_driver(driver))
> >  		goto unregister;
> > @@ -628,6 +624,7 @@ void i2c_del_driver(struct i2c_driver *d
> >  	 * attached. If so, detach them to be able to kill the driver
> >  	 * afterwards.
> >  	 */
> > +	down(&i2c_adapter_class.sem);
> >  	list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
> >  		if (driver->detach_adapter) {
> >  			if (driver->detach_adapter(adap)) {
> > @@ -652,12 +649,11 @@ void i2c_del_driver(struct i2c_driver *d
> >  			}
> >  		}
> >  	}
> > +	up(&i2c_adapter_class.sem);
> >  
> >   unregister:
> >  	driver_unregister(&driver->driver);
> 
> There's a race here for legacy drivers.  Consider that someone can
> add a device between the up() and the driver_unregister(), and the
> attach_adapter() loop may have bound that device to this driver.
> 
> 
> >  	pr_debug("i2c-core: driver [%s] unregistered\n", driver->driver.name);
> > -
> > -	mutex_unlock(&core_lists);
> >  }

Hmm, that's right, good catch. Damn legacy drivers :( What a pain.

I've been thinking about it for 2 hours now, and it turns out that I
simply don't understand it all good enough to make safe changes in this
area. It's too complex for me it seems. So I'm going to simply drop
this patch. All I can do is rename the core_lists mutex to core_lock,
but that's about it. 

-- 
Jean Delvare