Re: [PATCH] iio: pressure: rohm-bm1390: notify trigger on all error paths

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 18 May 2026 09:59:24 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, May 18, 2026 at 08:21:17AM +0300, Matti Vaittinen wrote:
> > On 17/05/2026 20:12, David Lechner wrote:  
> > > On 5/17/26 11:08 AM, Stepan Ionichev wrote:  
> 
> ...
> 
> > Maybe it would be better to do something like:
> > 
> > void iio_trigger_poll_nested(struct iio_trigger *trig)
> > {
> >         int i;
> > 
> >         if (!atomic_read(&trig->use_count)) {
> >                 atomic_set(&trig->use_count,
> > CONFIG_IIO_CONSUMERS_PER_TRIGGER);  
> 
> Just in case somebody is going to do that, avoid doing atomic_read() followed
> by atomic_set(). This is typical TOCTOU issue. This should be something like
> atomic_xchg() or atomic_add_return() or something like this in a single atomic
> operation.

Just to clarify - the current code is fine.  This got reported a few years
back and I did the analysis to prove it. From what I recall the key is
that the state space isn't as complex as it immediately looks.
That counter is either non 0 at the start (we don't use it here and we
skip an interrupt - that's actually the desired behaviour if the trigger is running
too fast - triggers must survive that - reenable() callback is there to make that
all work).

Otherwise there is a single path that sets it and we know any decrement until after
that happens would have undeflowed (and hence was a bug). The rest are decrement
only and it can never go to less than 0.

Hence it is fine.

Agreed things get messy if we make this alg any more complex though!

J
> 
> >                 for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
> >                         if (trig->subirqs[i].enabled)
> >                                 handle_nested_irq(trig->subirq_base + i);
> >                         else
> >                                 iio_trigger_notify_done(trig);
> >                 }
> > 		atomic_set(&trig->use_count, 0); /* Clear the use_count if drivers didn't
> > */
> >         }
> > }
> > 
> > to prevent this class of problems once and for all. But yeah, wiser minds
> > have designed this - so let's hear some other opinions as well :)  
> 
>