On Mon, Oct 21, 2024 at 02:54:15PM +0200, Matteo Martelli wrote:
> Consumers need to call the producer's read_avail_release_resource()
> callback after reading producer's available info. To avoid a race
> condition with the producer unregistration, change inkern
> iio_channel_read_avail() so that it copies the available info from the
> producer and immediately calls its release callback with info_exists
> locked.
>
> Also, modify the users of iio_read_avail_channel_raw() and
> iio_read_avail_channel_attribute() to free the copied available buffers
> after calling these functions. To let users free the copied buffer with
> a cleanup pattern, also add a iio_read_avail_channel_attr_retvals()
> consumer helper that is equivalent to iio_read_avail_channel_attribute()
> but stores the available values in the returned variable.
...
> +static void dpot_dac_read_avail_release_res(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int *vals, long mask)
> +{
> + kfree(vals);
> +}
> +
> static int dpot_dac_write_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> @@ -125,6 +132,7 @@ static int dpot_dac_write_raw(struct iio_dev *indio_dev,
> static const struct iio_info dpot_dac_info = {
> .read_raw = dpot_dac_read_raw,
> .read_avail = dpot_dac_read_avail,
> + .read_avail_release_resource = dpot_dac_read_avail_release_res,
> .write_raw = dpot_dac_write_raw,
> };
I have a problem with this approach. The issue is that we allocate
memory in one place and must clear it in another. This is not well
designed thingy in my opinion. I was thinking a bit of the solution and
at least these two comes to my mind:
1) having a special callback for .read_avail_with_copy (choose better
name) that will dump the data to the intermediate buffer and clean it
after all;
2) introduce a new type (or bit there), like IIO_AVAIL_LIST_ALLOC.
In any case it looks fragile and not scalable. I propose to drop this
and think again.
Yes, yes, I'm fully aware about the problem you are trying to solve and
agree on the report, I think this solution is not good enough.
--
With Best Regards,
Andy Shevchenko
