Re: [PATCH RFT] iio: Fix core buffer demux failure to account for unwanted channels at tail

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 5 Aug 2025 14:41:03 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Aug 5, 2025 at 10:16 AM Nuno Sá <noname.nuno@gmail.com> wrote:
> > On Mon, Aug 04, 2025 at 06:02:22PM +0200, Andy Shevchenko wrote:  
> > > On Mon, Aug 4, 2025 at 5:37 PM Nuno Sá <noname.nuno@gmail.com> wrote:  
> > > > On Sat, Aug 02, 2025 at 06:15:39PM +0100, Jonathan Cameron wrote:  
> 
> ...
> 
> > > > > +     /* Walk remaining bits of active_scan_mask */
> > > > > +     in_ind = find_next_bit(indio_dev->active_scan_mask, masklength,
> > > > > +                            in_ind + 1);  
> > > >
> > > > I wonder if it matters to check that in_ind + 1 is in fact lower than
> > > > masklength? Not that it will be an issue for find_next_bit() but we will
> > > > fail the expectation:
> > > >
> > > > if (unlikely(__start >= sz)) [1]
> > > >
> > > > And being this a sensible path, I thought it's worth (at least) questioning...  
> > >
> > > It doesn't matter. The find_*_bit() are all aligned to return sz for
> > > anything "not found anymore" cases, so it will be okay.  
> >
> > I know :):
> >
> > "...Not that it will be an issue for find_next_bit()..."
> >
> > I was mostly worried by performance as we'll have a compiler hint that
> > will pretty much fail (that ´if (unlikely(__start >= sz))' for every sample we push and
> > I guess the CPU will have to unroll that prediction. Maybe it will be smart enough to
> > adapt.  
> 
> Ah, I see now. Yeah, there might be a hint to skip the branch which is
> unlikely() for.

Assuming I remember how this all works...

This doesn't happen on the fast path (pushing samples)
It's a setup activity on a buffer being enabled.  The code is
generating a table of offsets and sizes that are then used to
on every sample.  So I don't think it's worth bothering to optimize it.

Jonathan

> 
> > But as I said, it might be neglectable but still worth at least
> > questioning...
> >  
> > > > Other than that kind of nit comment, patch looks good.
> > > >
> > > > [1]: https://elixir.bootlin.com/linux/v6.16/source/lib/find_bit.c#L50  
> > >  
> > > > > +     while (in_ind != masklength) {
> > > > > +             ret = iio_storage_bytes_for_si(indio_dev, in_ind);
> > > > > +             if (ret < 0)
> > > > > +                     goto error_clear_mux_table;
> > > > > +
> > > > > +             length = ret;
> > > > > +             /* Make sure we are aligned */
> > > > > +             in_loc = roundup(in_loc, length) + length;
> > > > > +             in_ind = find_next_bit(indio_dev->active_scan_mask,
> > > > > +                                    masklength, in_ind + 1);
> > > > > +     }  
>