Re: [PATCH v1 3/3] iio:pressure: initial zpa2326 barometer support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 08/29/2016 09:01 PM, Jonathan Cameron wrote:
On 25/08/16 15:45, Gregor Boirie wrote:
Answers inline

On 08/25/2016 08:34 AM, Peter Meerwald-Stadler wrote:

+
+    zpa_init_runtime(slave);
+
+    err = devm_iio_device_register(slave, indio_dev);
can't use devm_ register if there is a non-empty remove()
Removal hooks are registered onto bus specific devices not onto
IIO ones. Enabling devm debug outputs a removal flow trace that
seems correct. Is this really wrong ?
Yes.  The unwind of devm_iio_device_register will occur after the
whatever is in the remove function.  As devm_iio_device_unregister
is responsible for removal of the userspace and other interfaces
until that happens they are still present.

Hence whilst a remove is going on the device can be powered
down but the interfaces still exposed.  A read at that point
is going to cause some an error to occur.

Hence, using the devm form is fine if everything else is
handled automatically, i.e. there is nothing in remove.
Otherwise, you almost always have a race condition.

It's a minor one given how often people remove modules, but
worth cleaning up anyway!
Got it.

But something still makes me feel uncomfortable. Shouldn't we ensure the
bus specific driver (i.e., zpa2326_i2c or zpa2326_spi in this case) be refcounted
upon entry into the IIO layer ? Using try_module_get()/module_put() would :
* prevent any attempt to remove module while being used
* prevent any attempt to use module while it is going away.

I'm not quite sure here because of possible race conditions however (feels like
mixing devm_ and module layers).

Anyway, it seems quite strange module removal succeeds despite being under use by userspace. This leads to inconsistent system state where userspace process gets stuck in poll syscall, waiting for data that will never come since modprobing
module again will in the end allocate an new IIO device, etc...

confused,
Grégor.


Jonathan
Many thanks for the review. Regards,
Greg.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux