On 11/01/2012 12:09 AM, Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/29/2012 08:52 AM, Guennadi Liakhovetski wrote:
+/*
+ * Typically this function will be called during bridge driver probing. It
+ * installs bus notifiers to handle asynchronously probing subdevice drivers.
+ * Once the bridge driver probing completes, subdevice drivers, waiting in
+ * EPROBE_DEFER state are re-probed, at which point they get their platform
+ * data, which allows them to complete probing.
+ */
+int v4l2_async_group_probe(struct v4l2_async_group *group)
+{
+ struct v4l2_async_subdev *asd, *tmp;
+ bool i2c_used = false, platform_used = false;
+ int ret;
+
+ /* This group is inactive so far - no notifiers yet */
+ list_for_each_entry_safe(asd, tmp,&group->group, list) {
+ if (asd->sdpd.subdev) {
+ /* Simulate a BIND event */
+ if (group->bind_cb)
+ group->bind_cb(group, asd);
+
Still we can't be sure at this moment asd->sdpd.subdev's driver is
valid and not unloaded, can we ?
In the case when a sub-device driver is probed after the host driver
(a caller of this function) I assume doing
asd->sdpd.subdev = i2c_get_clientdata(to_i2c_client(dev));
...
ret = v4l2_device_register_subdev(v4l2_dev, asd->sdpd.subdev);
is safe, because it is done in the i2c bus notifier callback itself,
i.e. under device_lock(dev).
But for these already probed sub-devices, how do we prevent races from
subdev module unloading ? By not setting CONFIG_MODULE_UNLOAD?... ;)
Right, I also think there's a race there. I have a solution for it - in
the current mainline version of sh_mobile_ceu_camera.c look at the code
around the line
err = bus_register_notifier(&platform_bus_type,&wait.notifier);
sh_mobile_ceu_probe(). I think, that guarantees, that we either lock the
module _safely_ in memory per try_module_get(dev->driver->owner) or get
notified, that the module is unavailable. It looks ugly, but I don't have
a better solution ATM. We could do the same here too.
IMHO even "ugly" solution is better than completely ignoring the problem.
I have some doubts whether your method eliminates the race issue. Firstly,
shouldn't the bus_notify callback [1] be active on BUS_NOTIFY_UNBIND_DRIVER,
rather than US_NOTIFY_UNBOUND_DRIVER ? Upon US_NOTIFY_UNBOUND_DRIVER
dev->driver is already NULL and still it is being referenced in a call to
try_module_get() (line 2224, [1]).
Secondly, what guarantees that before bus_register_notifier() call [1],
we are not already after blocking_notifier_call_chain() (line 504, [2])
which means we miss the notification and the sub-device driver is going
away together with its module under our feet ?
Hmm, please ignore that one, of course in this case dev->driver is NULL
and branch after this line
if (!csi2_pdev->dev.driver) {
is entered.
[1] http://lxr.linux.no/#linux+v3.6/drivers/media/video/sh_mobile_ceu_camera.c#L2055
[2] http://lxr.linux.no/#linux+v3.6/drivers/base/dd.c#L478
--
Thanks,
Sylwester
--
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
