RE: [PATCH v4 6/8] iio: imu: adis16607: Add FIFO-based buffered/triggered capture

[Sabau, Radu bogdan]

> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Monday, October 26, 2026 2:09 AM

...

> > +static void adis16607_fifo_push_sample(struct iio_dev *indio_dev,
> > +				       u8 *fifo_data, s64 timestamp)
> > +{
> > +	struct adis16607 *st = iio_priv(indio_dev);
> > +	u16 msw, lsw;
> > +	s32 val24;
> > +	int bit, offset = 0;
> > +
> > +	memset(st->data, 0, sizeof(st->data));
> > +
> > +	for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev-
> >masklength) {
> iio_for_each_active_channel()
> 
> As sashiko notes, masklength is private.
> 
> There are a load of places we need to update to stop accessing these
> directly but please don't add another one.
> 
> It's on my list to throw some LLM tokens ago once I get out from
> under review backlog.
> 
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20261023-
> adis16607-v4-0-
> a4d541ebfd13*40analog.com__;IyU!!A3Ni8CS0y2Y!9cguUuIwYMdnmBcoNR
> ye0_U99KUnJt04T6XlSnvS-9rxh5-8Y2fQXC39Mxsw3erc_JwvyFp3ssecZg$
> Check everything else reported.
> 
> FWIW there are also some nasty race conditions around active_scan_mask
> being available when the buffer is being torn down. You might
> want to close those off by claiming buffered mode to prevent
> such a transition.
> 

Agreed, I'll switch to iio_for_each_active_channel() and I'll go through the
rest of the Sashiko findings for v5.

On the active_scan_mask races, I'd like to check which path you mean.
In this driver the only reader of the mask is the trigger handler
(fifo_push_sample()), which runs as the pollfunc.
As far as I can tell, iio_trigger_detach_poll_func()'s free_irq() waits for it
to finish before iio_disable_buffers() frees the mask, so I couldn't find a
window where the handler sees a stale or NULL mask today.
Even so, I'd like to close this off for good, so that a later change to the
driver or to adislib can't introduce the race.

Claiming buffer mode inside the handler looks like it would deadlock
against iio_disable_buffers(), which holds mlock while waiting for the
handler in free_irq().

Would snapshotting the active-channel layout in update_scan_mode()
(called before the pollfunc is attached) and having the handler read
only that copy be an acceptable way to close it?