[i2c] [PATCH] i2c: Clean up core locking
David Brownell
david-b at pacbell.net
Fri Dec 7 19:04:46 CET 2007
> > > @@ -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
Wouldn't it suffice to just add a flag to the driver saying whether
an attach_adapter() call would be valid? You can prevent the loop
fairly directly, rather than by code flow. Solving this puzzler by
codeflow morphing is probably a losing game.
- Dave
More information about the i2c
mailing list