[PATCH 1/3] iio: Add data_available callback for buffers

[PATCH 1/3] iio: Add data_available callback for buffers

[Lars-Peter Clausen]

This patch adds a new data_available() callback to the iio_buffer_access_funcs
struct. The callback is used to indicate whether data is available in the buffer
for reading. It is meant to replace the stufftoread flag from the iio_buffer
struct. The reasoning for this is that the buffer implementation usually can
determine whether data is available rather easily based on its state, on the
other hand it can be rather tricky to update the stufftoread flag in a race free
way.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 10 +++++++++-
 include/linux/iio/buffer.h        |  3 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 7f9152c..4dcc3a0 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -37,6 +37,14 @@ static bool iio_buffer_is_active(struct iio_buffer *buf)
 	return !list_empty(&buf->buffer_list);
 }
 
+static bool iio_buffer_data_available(struct iio_buffer *buf)
+{
+	if (buf->access->data_available)
+		return buf->access->data_available(buf);
+
+	return buf->stufftoread;
+}
+
 /**
  * iio_buffer_read_first_n_outer() - chrdev read for buffer access
  *
@@ -70,7 +78,7 @@ unsigned int iio_buffer_poll(struct file *filp,
 		return -ENODEV;
 
 	poll_wait(filp, &rb->pollq, wait);
-	if (rb->stufftoread)
+	if (iio_buffer_data_available(rb))
 		return POLLIN | POLLRDNORM;
 	/* need a way of knowing if there may be enough data... */
 	return 0;
diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
index 15607b4..5193927 100644
--- a/include/linux/iio/buffer.h
+++ b/include/linux/iio/buffer.h
@@ -21,6 +21,8 @@ struct iio_buffer;
  * struct iio_buffer_access_funcs - access functions for buffers.
  * @store_to:		actually store stuff to the buffer
  * @read_first_n:	try to get a specified number of bytes (must exist)
+ * @data_available:	indicates whether data for reading from the buffer is
+ *			available.
  * @request_update:	if a parameter change has been marked, update underlying
  *			storage.
  * @get_bytes_per_datum:get current bytes per datum
@@ -43,6 +45,7 @@ struct iio_buffer_access_funcs {
 	int (*read_first_n)(struct iio_buffer *buffer,
 			    size_t n,
 			    char __user *buf);
+	bool (*data_available)(struct iio_buffer *buffer);
 
 	int (*request_update)(struct iio_buffer *buffer);
 
-- 
1.8.0

[PATCH 2/3] iio: kfifo_buf: Implement data_available() callback

[Lars-Peter Clausen]

This patch implements the data_available() callback for the kfifo buffer instead
of using the stufftoread flag. The kfifo used by the buffer already knows
whether it is empty or not based on the position of its read and write pointer.
Using this makes it a lot easier to tell whether data is available or not and it
is not necessary to take special measures to ensure that no race conditions
between reading and writing from the buffer occur.

Note, that we still have to take the buffers lock to protect against concurrent
resizeing of the kfifo.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/kfifo_buf.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 95c6fc8..7134e8a 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -42,7 +42,6 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
 	} else {
 		kfifo_reset_out(&buf->kf);
 	}
-	r->stufftoread = false;
 	mutex_unlock(&buf->user_lock);
 
 	return ret;
@@ -108,7 +107,7 @@ static int iio_store_to_kfifo(struct iio_buffer *r,
 	ret = kfifo_in(&kf->kf, data, 1);
 	if (ret != 1)
 		return -EBUSY;
-	r->stufftoread = true;
+
 	wake_up_interruptible_poll(&r->pollq, POLLIN | POLLRDNORM);
 
 	return 0;
@@ -127,13 +126,6 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
 		ret = -EINVAL;
 	else
 		ret = kfifo_to_user(&kf->kf, buf, n, &copied);
-
-	if (kfifo_is_empty(&kf->kf))
-		r->stufftoread = false;
-	/* verify it is still empty to avoid race */
-	if (!kfifo_is_empty(&kf->kf))
-		r->stufftoread = true;
-
 	mutex_unlock(&kf->user_lock);
 	if (ret < 0)
 		return ret;
@@ -141,6 +133,18 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
 	return copied;
 }
 
+static bool iio_kfifo_buf_data_available(struct iio_buffer *r)
+{
+	struct iio_kfifo *kf = iio_to_kfifo(r);
+	bool empty;
+
+	mutex_lock(&kf->user_lock);
+	empty = kfifo_is_empty(&kf->kf);
+	mutex_unlock(&kf->user_lock);
+
+	return !empty;
+}
+
 static void iio_kfifo_buffer_release(struct iio_buffer *buffer)
 {
 	struct iio_kfifo *kf = iio_to_kfifo(buffer);
@@ -153,6 +157,7 @@ static void iio_kfifo_buffer_release(struct iio_buffer *buffer)
 static const struct iio_buffer_access_funcs kfifo_access_funcs = {
 	.store_to = &iio_store_to_kfifo,
 	.read_first_n = &iio_read_first_n_kfifo,
+	.data_available = iio_kfifo_buf_data_available,
 	.request_update = &iio_request_update_kfifo,
 	.get_bytes_per_datum = &iio_get_bytes_per_datum_kfifo,
 	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
-- 
1.8.0

[PATCH 3/3] iio: Add support for blocking IO on buffers

[Lars-Peter Clausen]

Currently the IIO buffer interface only allows non-blocking reads. This patch
adds support for blocking IO. In blocking mode the thread will go to sleep if no
data is available and will wait for the buffer implementation to signal that new
data is available by waking up the buffers waitqueue.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 4dcc3a0..c67d83b 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -56,13 +56,34 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
 {
 	struct iio_dev *indio_dev = filp->private_data;
 	struct iio_buffer *rb = indio_dev->buffer;
+	int ret;
 
 	if (!indio_dev->info)
 		return -ENODEV;
 
 	if (!rb || !rb->access->read_first_n)
 		return -EINVAL;
-	return rb->access->read_first_n(rb, n, buf);
+
+	do {
+		if (!iio_buffer_data_available(rb)) {
+			if (filp->f_flags & O_NONBLOCK)
+				return -EAGAIN;
+
+			ret = wait_event_interruptible(rb->pollq,
+					iio_buffer_data_available(rb) ||
+					indio_dev->info == NULL);
+			if (ret)
+				return ret;
+			if (indio_dev->info == NULL)
+				return -ENODEV;
+		}
+
+		ret = rb->access->read_first_n(rb, n, buf);
+		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
+			ret = -EAGAIN;
+	 } while (ret == 0);
+
+	return ret;
 }
 
 /**
-- 
1.8.0

Re: [PATCH 1/3] iio: Add data_available callback for buffers

[Jonathan Cameron]

On 11/25/13 14:56, Lars-Peter Clausen wrote:
> This patch adds a new data_available() callback to the iio_buffer_access_funcs
> struct. The callback is used to indicate whether data is available in the buffer
> for reading. It is meant to replace the stufftoread flag from the iio_buffer
> struct. The reasoning for this is that the buffer implementation usually can
> determine whether data is available rather easily based on its state, on the
> other hand it can be rather tricky to update the stufftoread flag in a race free
> way.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

A sensible change - applied to the togreg branch of iio.git.

> ---
>  drivers/iio/industrialio-buffer.c | 10 +++++++++-
>  include/linux/iio/buffer.h        |  3 +++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 7f9152c..4dcc3a0 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -37,6 +37,14 @@ static bool iio_buffer_is_active(struct iio_buffer *buf)
>  	return !list_empty(&buf->buffer_list);
>  }
>  
> +static bool iio_buffer_data_available(struct iio_buffer *buf)
> +{
> +	if (buf->access->data_available)
> +		return buf->access->data_available(buf);
> +
> +	return buf->stufftoread;
> +}
> +
>  /**
>   * iio_buffer_read_first_n_outer() - chrdev read for buffer access
>   *
> @@ -70,7 +78,7 @@ unsigned int iio_buffer_poll(struct file *filp,
>  		return -ENODEV;
>  
>  	poll_wait(filp, &rb->pollq, wait);
> -	if (rb->stufftoread)
> +	if (iio_buffer_data_available(rb))
>  		return POLLIN | POLLRDNORM;
>  	/* need a way of knowing if there may be enough data... */
>  	return 0;
> diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h
> index 15607b4..5193927 100644
> --- a/include/linux/iio/buffer.h
> +++ b/include/linux/iio/buffer.h
> @@ -21,6 +21,8 @@ struct iio_buffer;
>   * struct iio_buffer_access_funcs - access functions for buffers.
>   * @store_to:		actually store stuff to the buffer
>   * @read_first_n:	try to get a specified number of bytes (must exist)
> + * @data_available:	indicates whether data for reading from the buffer is
> + *			available.
>   * @request_update:	if a parameter change has been marked, update underlying
>   *			storage.
>   * @get_bytes_per_datum:get current bytes per datum
> @@ -43,6 +45,7 @@ struct iio_buffer_access_funcs {
>  	int (*read_first_n)(struct iio_buffer *buffer,
>  			    size_t n,
>  			    char __user *buf);
> +	bool (*data_available)(struct iio_buffer *buffer);
>  
>  	int (*request_update)(struct iio_buffer *buffer);
>  
> 

Re: [PATCH 2/3] iio: kfifo_buf: Implement data_available() callback

[Jonathan Cameron]

On 11/25/13 14:56, Lars-Peter Clausen wrote:
> This patch implements the data_available() callback for the kfifo buffer instead
> of using the stufftoread flag. The kfifo used by the buffer already knows
> whether it is empty or not based on the position of its read and write pointer.
> Using this makes it a lot easier to tell whether data is available or not and it
> is not necessary to take special measures to ensure that no race conditions
> between reading and writing from the buffer occur.
> 
> Note, that we still have to take the buffers lock to protect against concurrent
> resizeing of the kfifo.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Looks good. Applied to the togreg branch of iio.git.

Thanks,

Jonathan
> ---
>  drivers/iio/kfifo_buf.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 95c6fc8..7134e8a 100644
> --- a/drivers/iio/kfifo_buf.c
> +++ b/drivers/iio/kfifo_buf.c
> @@ -42,7 +42,6 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
>  	} else {
>  		kfifo_reset_out(&buf->kf);
>  	}
> -	r->stufftoread = false;
>  	mutex_unlock(&buf->user_lock);
>  
>  	return ret;
> @@ -108,7 +107,7 @@ static int iio_store_to_kfifo(struct iio_buffer *r,
>  	ret = kfifo_in(&kf->kf, data, 1);
>  	if (ret != 1)
>  		return -EBUSY;
> -	r->stufftoread = true;
> +
>  	wake_up_interruptible_poll(&r->pollq, POLLIN | POLLRDNORM);
>  
>  	return 0;
> @@ -127,13 +126,6 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
>  		ret = -EINVAL;
>  	else
>  		ret = kfifo_to_user(&kf->kf, buf, n, &copied);
> -
> -	if (kfifo_is_empty(&kf->kf))
> -		r->stufftoread = false;
> -	/* verify it is still empty to avoid race */
> -	if (!kfifo_is_empty(&kf->kf))
> -		r->stufftoread = true;
> -
>  	mutex_unlock(&kf->user_lock);
>  	if (ret < 0)
>  		return ret;
> @@ -141,6 +133,18 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
>  	return copied;
>  }
>  
> +static bool iio_kfifo_buf_data_available(struct iio_buffer *r)
> +{
> +	struct iio_kfifo *kf = iio_to_kfifo(r);
> +	bool empty;
> +
> +	mutex_lock(&kf->user_lock);
> +	empty = kfifo_is_empty(&kf->kf);
> +	mutex_unlock(&kf->user_lock);
> +
> +	return !empty;
> +}
> +
>  static void iio_kfifo_buffer_release(struct iio_buffer *buffer)
>  {
>  	struct iio_kfifo *kf = iio_to_kfifo(buffer);
> @@ -153,6 +157,7 @@ static void iio_kfifo_buffer_release(struct iio_buffer *buffer)
>  static const struct iio_buffer_access_funcs kfifo_access_funcs = {
>  	.store_to = &iio_store_to_kfifo,
>  	.read_first_n = &iio_read_first_n_kfifo,
> +	.data_available = iio_kfifo_buf_data_available,
>  	.request_update = &iio_request_update_kfifo,
>  	.get_bytes_per_datum = &iio_get_bytes_per_datum_kfifo,
>  	.set_bytes_per_datum = &iio_set_bytes_per_datum_kfifo,
> 

Re: [PATCH 3/3] iio: Add support for blocking IO on buffers

[Jonathan Cameron]

On 11/25/13 14:56, Lars-Peter Clausen wrote:
> Currently the IIO buffer interface only allows non-blocking reads. This patch
> adds support for blocking IO. In blocking mode the thread will go to sleep if no
> data is available and will wait for the buffer implementation to signal that new
> data is available by waking up the buffers waitqueue.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git

Nice little bit of functionality. Thanks.

Jonathan
> ---
>  drivers/iio/industrialio-buffer.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 4dcc3a0..c67d83b 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -56,13 +56,34 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
>  {
>  	struct iio_dev *indio_dev = filp->private_data;
>  	struct iio_buffer *rb = indio_dev->buffer;
> +	int ret;
>  
>  	if (!indio_dev->info)
>  		return -ENODEV;
>  
>  	if (!rb || !rb->access->read_first_n)
>  		return -EINVAL;
> -	return rb->access->read_first_n(rb, n, buf);
> +
> +	do {
> +		if (!iio_buffer_data_available(rb)) {
> +			if (filp->f_flags & O_NONBLOCK)
> +				return -EAGAIN;
> +
> +			ret = wait_event_interruptible(rb->pollq,
> +					iio_buffer_data_available(rb) ||
> +					indio_dev->info == NULL);
> +			if (ret)
> +				return ret;
> +			if (indio_dev->info == NULL)
> +				return -ENODEV;
> +		}
> +
> +		ret = rb->access->read_first_n(rb, n, buf);
> +		if (ret == 0 && (filp->f_flags & O_NONBLOCK))
> +			ret = -EAGAIN;
> +	 } while (ret == 0);
> +
> +	return ret;
>  }
>  
>  /**
>