On 18/08/11 19:43, Devin Heitmueller wrote:
You would be well served to break this into a patch series, as it tends to be
difficult to review a whole series of changes in a single patch. You seem to
be mixed in a bunch of "useless" changes alongside functional changes. For
example, if you're trying to add in a missing goto inside an exception block,
doing that at the same time as renaming instances of "errCode" to "retval"
just creates confusion.
Actually, those two particular changes go together because I'm replacing "return
errCode" with a goto to "return retval". Ultimately, errCode becomes an unused
variable.
And finally, the mutex structure used for the modules is somewhat complicated
due to to the need to keep the analog side of the board locked while
initializing the digital side. This code was added specifically to prevent
race conditions that were seen during initialization as things like udev and
dbus attempted to connect to the newly created V4L device while the em28xx-dvb
module was still coming up.
OK, thanks. I've been tackling this problem from the "We must always take lock A
before lock B, and never vice versa" point of view. So the order is:
- take device mutex
- enter em28xx_init_dev()
- enter em28xx_init_extension()
- take device list mutex
- call init() function for every extension with this device
Since dvb_init() is the init() function for the em28xx-dvb extension, it follows
that it cannot take the device's mutex again. The problem is therefore moved to
em28xx_dvb_register(), which takes the device list mutex and yet MUST not take
the mutex for any device in the list.
Combining em28xx_add_into_devlist() with em28xx_init_extension() (and similarly
em28xx_remove_from_devlist() with em28xx_close_extension()) means that the
device list must always contain a list of devices that has been initialised
against every extension in the extension list.
I can probably factor out the simpler patches first, such as using the bit
operations on em28xx_devused, and the memory leak in em28xx_v4l2_close(). And
the spelling fixes...
Cheers,
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
