On 2020-08-13 11:46:30 [+0200], Lars-Peter Clausen wrote: > > It should not affect the IRQ handlers of individual drivers. The hrtimer > triggers acts like an IRQ chip and will call generic_handle_irq() to > multiplex the interrupt handling onto all consumers. As far as I understand > it there is a requirement that generic_handle_irq() is called in hard irq > context, even with PREEMT_RT=y. That is correct. > If you are running with forced IRQ threads the only thing that will then > happen in the actual hard IRQ context is the launching of the IRQ threads. > Th e IRQ handler of the device driver will run in a threaded IRQ. So if it is really just the wakeup of the IRQ-thread then it should be okay. One thing: iio_trigger_poll() may invoke iio_trigger_notify_done(). This would invoke trig->ops->try_reenable callback if available. I grepped and found - bma180_trig_try_reen() It appears to perform i2c_smbus_read_byte_data() and smbus sounds sleeping. I don't know if it attempts to acquire any spinlock_t but it will be wrong on RT. - bmc150_accel_trig_try_reen() This one has mutex_lock() which is wrong even on !RT in this context (unless the previous `if' saves us). - mxc4005_trigger_try_reen() This one uses regmap_write(). regmap internally uses a lock and the config does not disable / provide a lock. This means regmap_lock_mutex() is used (or regmap_lock_spinlock() in case of bus->fast_io but I doubt it with i2c). Am I looking somehow wrong at this or did just nobody try the combination of one of the three drivers here together with the hrtimer trigger? > > > > Has this change (including the second patch in thread) been tested on RT > > in terms of locking and latency? > > It has not been tested in terms of latency. But like I said if you are > running with forced IRQ threads the effect should be minimal. > > Without this patch there is an correctness issue when PREEMT_RT=y since > generic_handle_irq() runs with interrupts on which breaks its internal > assumptions. I'm trying to understand the scope of the change. As I said above, if it is just wakeup of the thread, then it is fine. I have memory of people running iio drivers (or triggers) in hardirq-context for $reason and try avoid something like this. Sebastian
