[i2c] [patch 2.6.20-rc1 5/6] remove i2c_adapter.dev from i2c core and adapters
david-b at pacbell.net
Fri Jan 5 06:32:11 CET 2007
On Wednesday 03 January 2007 11:04 pm, Jean Delvare wrote:
> On Wed, 3 Jan 2007 11:53:18 -0800, David Brownell wrote:
> > On Wednesday 03 January 2007 5:26 am, Jean Delvare wrote:
> > >
> > > OK, thanks for the explanation. Now just to make it clear, is there any
> > > bug with regards to completions in the curent i2c-core/i2c-isa code?
> > Just the design issue of synchronizing at that point, which tend to hide
> > bugs that can be triggered by the "cd drv_directory; rmmod driver" kind of
> > scenario I mentioned. Normally one wants a release() to just free memory.
> > But changing that right now would clearly be intrusive, and in any case
> > would need to be preceded by removing a lot of duplication of driver core
> > functionality from the i2c core. Which we've established can't all be done
> > very quickly (unfortunately).
> Do you mean that release() callbacks usually don't use a completion at
> all as i2c-core does?
Ideally they just release memory, now that it's known to no longer
be in use.
> If so, can you explain why we needed to do it
> differently? Sorry if you explained that already, but I'm not familiar
> with the right way to do it and it'll take some times before I realize
> all the things i2c-core does the wrong way.
When you rmmod a driver, sysfs files it created (directly or otherwise)
can still be in active use. So there's a need for some level of
synchronization to make sure that, for example, the release() method
is still in memory so it can be called.
That can get very messy. LOTS of driver code gets it wrong, and it's
not always clear how to sort it all out. Part of the fix is to use
a driver-model friendly approach whereby all the sysfs nodes (files
and directories, devices etc) are created by the driver core, which
will still be around to release the memory.
Meanwhile, one of the best workarounds is to synchronize like this.
It's not without problems -- e.g. "cd driver_created_directory" then
"rmmod driver", and watch it hang -- but it's usually OK>
> > > Right now we are initializing the completion right before releasing the
> > > device, rather than when creating the device as you're suggesting.
> > That's probably not an issue, although it's not an idiom to promote.
> > > Given that the completion is only accessed when releasing the device, I
> > > don't think it makes any difference, it sounds more like a personnal
> > > implementation preference. Or am I missing something?
> > The reason to prefer the "init at init time" idiom is not that it can't
> > be done "correctly" otherwise, but that it's more _obviously_ correct
> > that way ... as well as less redundant. (There are multiple exit paths,
> > all of which "init" that state, but only one init path...) As well as
> > being the traditional way to do things.
> OK, thanks for the clarification. If you want to send a patch moving
> the completion initializations to where they belong, I'll be happy to
> apply it. Such a patch would be sequenced after your patch #6.
Maybe. I'd rather provide current versions of the "Part II" patches,
since those are lots more important to get merged.
More information about the i2c