Re: [PATCH v2 5/7] iio: inkern: copy/release available info from producer

[Nuno Sá]

On Mon, 2024-10-07 at 10:37 +0200, Matteo Martelli wrote:
> Consumers need to call the read_avail_release_resource after reading the
> available info. To call the release with info_exists locked, copy the
> available info from the producer and immediately call its release
> callback. With this change, users of iio_read_avail_channel_raw() and
> iio_read_avail_channel_attribute() must free the copied avail info after
> calling them.
> 
> Signed-off-by: Matteo Martelli <matteomartelli3@xxxxxxxxx>
> ---
>  drivers/iio/inkern.c         | 64 +++++++++++++++++++++++++++++++++-----------
>  include/linux/iio/consumer.h |  4 +--
>  2 files changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index
> 7f325b3ed08fae6674245312cf8f57bb151006c0..cc65ef79451e5aa2cea447e168007a447ffc0d91
> 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -760,9 +760,25 @@ static int iio_channel_read_avail(struct iio_channel *chan,
>  	if (!iio_channel_has_available(chan->channel, info))
>  		return -EINVAL;
>  
> -	if (iio_info->read_avail)
> -		return iio_info->read_avail(chan->indio_dev, chan->channel,
> -					    vals, type, length, info);
> +	if (iio_info->read_avail) {
> +		const int *vals_tmp;
> +		int ret;
> +
> +		ret = iio_info->read_avail(chan->indio_dev, chan->channel,
> +					   &vals_tmp, type, length, info);
> +		if (ret < 0)
> +			return ret;
> +
> +		*vals = kmemdup_array(vals_tmp, *length, sizeof(int), GFP_KERNEL);
> +		if (!*vals)
> +			return -ENOMEM;
> +

Not a big deal but I would likely prefer to avoid yet another copy. If I'm
understanding things correctly, I would rather create an inkern wrapper API like 
iio_channel_read_avail_release_resource() - maybe something with a smaller name :).
Hence, the lifetime of the data would be only controlled by the producer of it. It
would also produce a smaller diff (I think). I just find it a bit confusing that we
duplicate the data in here and the producer also duplicates it on the ->read_avail()
call. Another advantage I see is that often the available data is indeed const in
which case no kmemdup_array() is needed at all.

- Nuno Sá