Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support
On Sun, Feb 14, 2021 at 5:58 PM Jonathan Cameron wrote:
>
> On Fri, 12 Feb 2021 12:11:43 +0200
> Alexandru Ardelean wrote:
>
> > Following a recent update to the IIO buffer infrastructure, this change
> > adds a basic example on how to access an IIO buffer via the new mmap()
> > interface.
> >
> > The ioctl() for the high-speed mode needs to be enabled right from the
> > start, before setting any parameters via sysfs (length, enable, etc), to
> > make sure that the mmap mode is used and not the fileio mode.
> >
> > Signed-off-by: Alexandru Ardelean
>
> Just one small question on error handling. Otherwise this looks fine to me.
>
> thanks,
>
> Jonathan
>
> > ---
> > tools/iio/iio_generic_buffer.c | 183 +++--
> > 1 file changed, 177 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> > index fdd08514d556..675a7e6047e0 100644
> > --- a/tools/iio/iio_generic_buffer.c
> > +++ b/tools/iio/iio_generic_buffer.c
> > @@ -31,6 +31,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include "iio_utils.h"
> >
> > @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char
> > *dev_dir_name, int buffer_idx, int e
> > return 0;
> > }
> >
> > +struct mmap_block {
> > + struct iio_buffer_block block;
> > + void *addr;
> > +};
> > +
> > +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int
> > block_size,
> > + int nblocks)
> > +{
> > + struct iio_buffer_block_alloc_req req = { 0 };
> > + struct mmap_block *mmaps = NULL;
> > + int mmaps_cnt = 0;
> > + int i, ret;
> > +
> > + /**
> > + * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> > + * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> > + * and other error types.
> > + */
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> > + if (ret < 0) {
> > + errno = ENOSYS;
> > + return NULL;
> > + }
> > +
> > + /* for now, this */
> > + req.id = 0;
> > + req.type = 0;
> > + req.size = block_size;
> > + req.count = nblocks;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, &req);
> > + if (ret < 0)
> > + return NULL;
> > +
> > + if (req.count == 0) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + if (req.count < nblocks) {
> > + fprintf(stderr, "Requested %d blocks, got %d\n",
> > + nblocks, req.count);
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + mmaps = calloc(req.count, sizeof(*mmaps));
> > + if (!mmaps) {
> > + errno = ENOMEM;
> > + return NULL;
> > + }
> > +
> > + for (i = 0; i < req.count; i++) {
> > + mmaps[i].block.id = i;
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL,
> > &mmaps[i].block);
> > + if (ret < 0)
> > + goto error;
> > +
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL,
> > &mmaps[i].block);
> > + if (ret < 0)
> > + goto error;
>
> > +
> > + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> > + PROT_READ | PROT_WRITE, MAP_SHARED,
> > + buf_fd, mmaps[i].block.data.offset);
> > +
> > + if (mmaps[i].addr == MAP_FAILED)
> > + goto error;
> > +
> > + mmaps_cnt++;
> > + }
> > +
> > + return mmaps;
> > +
> > +error:
> > + for (i = 0; i < mmaps_cnt; i++)
> > + munmap(mmaps[i].addr, mmaps[i].block.size);
> > + free(mmaps);
>
> No free of the blocks? We have unmapped them but I'd imagine we'd also
> need to free them from the driver side.
Good catch.
>
> > + return NULL;
> > +}
> > +
> > +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> > +struct mmap_block *mmaps, unsigned int mmaps_cnt)
> > +{
> > + struct iio_buffer_block block;
> > + int ret;
> > +
> > + /**
> > + * This is where some buffer-pool management can do wonders,
> > + * but for the sake of this sample-code, we're just going to
> > + * copy the data and re-enqueue it back
> > + */
> > + memset(&block, 0, sizeof(block));
> > + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, &block);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* check for weird conditions */
> > + if (block.bytes_used > block_size) {
> > + fprintf(stderr,
> > + "Got a bigger block (%u) than expected (%u)\n",
> > + block.bytes_used, block_size);
> > + return -EFBIG;
> > + }
> > +
> > + if (block.bytes_used < block_size) {
> > + /**
> > +
Re: [PATCH v2 3/3] tools: iio: add example for high-speed buffer support
On Fri, 12 Feb 2021 12:11:43 +0200
Alexandru Ardelean wrote:
> Following a recent update to the IIO buffer infrastructure, this change
> adds a basic example on how to access an IIO buffer via the new mmap()
> interface.
>
> The ioctl() for the high-speed mode needs to be enabled right from the
> start, before setting any parameters via sysfs (length, enable, etc), to
> make sure that the mmap mode is used and not the fileio mode.
>
> Signed-off-by: Alexandru Ardelean
Just one small question on error handling. Otherwise this looks fine to me.
thanks,
Jonathan
> ---
> tools/iio/iio_generic_buffer.c | 183 +++--
> 1 file changed, 177 insertions(+), 6 deletions(-)
>
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index fdd08514d556..675a7e6047e0 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include "iio_utils.h"
>
> @@ -239,6 +240,132 @@ static int enable_disable_all_channels(char
> *dev_dir_name, int buffer_idx, int e
> return 0;
> }
>
> +struct mmap_block {
> + struct iio_buffer_block block;
> + void *addr;
> +};
> +
> +static struct mmap_block *enable_high_speed(int buf_fd, unsigned int
> block_size,
> + int nblocks)
> +{
> + struct iio_buffer_block_alloc_req req = { 0 };
> + struct mmap_block *mmaps = NULL;
> + int mmaps_cnt = 0;
> + int i, ret;
> +
> + /**
> + * Validate we can do high-speed by issuing BLOCK_FREE ioctl.
> + * If using just BLOCK_ALLOC it's distinguish between ENOSYS
> + * and other error types.
> + */
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_FREE_IOCTL, 0);
> + if (ret < 0) {
> + errno = ENOSYS;
> + return NULL;
> + }
> +
> + /* for now, this */
> + req.id = 0;
> + req.type = 0;
> + req.size = block_size;
> + req.count = nblocks;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ALLOC_IOCTL, &req);
> + if (ret < 0)
> + return NULL;
> +
> + if (req.count == 0) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + if (req.count < nblocks) {
> + fprintf(stderr, "Requested %d blocks, got %d\n",
> + nblocks, req.count);
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + mmaps = calloc(req.count, sizeof(*mmaps));
> + if (!mmaps) {
> + errno = ENOMEM;
> + return NULL;
> + }
> +
> + for (i = 0; i < req.count; i++) {
> + mmaps[i].block.id = i;
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_QUERY_IOCTL,
> &mmaps[i].block);
> + if (ret < 0)
> + goto error;
> +
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_ENQUEUE_IOCTL,
> &mmaps[i].block);
> + if (ret < 0)
> + goto error;
> +
> + mmaps[i].addr = mmap(0, mmaps[i].block.size,
> + PROT_READ | PROT_WRITE, MAP_SHARED,
> + buf_fd, mmaps[i].block.data.offset);
> +
> + if (mmaps[i].addr == MAP_FAILED)
> + goto error;
> +
> + mmaps_cnt++;
> + }
> +
> + return mmaps;
> +
> +error:
> + for (i = 0; i < mmaps_cnt; i++)
> + munmap(mmaps[i].addr, mmaps[i].block.size);
> + free(mmaps);
No free of the blocks? We have unmapped them but I'd imagine we'd also
need to free them from the driver side.
> + return NULL;
> +}
> +
> +static int read_high_speed(int buf_fd, char *data, unsigned int block_size,
> +struct mmap_block *mmaps, unsigned int mmaps_cnt)
> +{
> + struct iio_buffer_block block;
> + int ret;
> +
> + /**
> + * This is where some buffer-pool management can do wonders,
> + * but for the sake of this sample-code, we're just going to
> + * copy the data and re-enqueue it back
> + */
> + memset(&block, 0, sizeof(block));
> + ret = ioctl(buf_fd, IIO_BUFFER_BLOCK_DEQUEUE_IOCTL, &block);
> + if (ret < 0)
> + return ret;
> +
> + /* check for weird conditions */
> + if (block.bytes_used > block_size) {
> + fprintf(stderr,
> + "Got a bigger block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + return -EFBIG;
> + }
> +
> + if (block.bytes_used < block_size) {
> + /**
> + * This can be normal, with some real-world data
> + * terminating abruptly. But log it.
> + */
> + fprintf(stderr,
> + "Got a smaller block (%u) than expected (%u)\n",
> + block.bytes_used, block_size);
> + }
> +
> + /* memcpy() the data, we lose some more performance here
