[PATCH 0/2] IIO: Kfifo usage improvements

[PATCH 0/2] IIO: Kfifo usage improvements

[Jonathan Cameron]

Hi All,

These have both been on the list before in various forms, be
it burried deep in threads.

Anyhow, please test these and review.

Jonathan

Jonathan Cameron (2):
  iio:kfifo_buf  Take advantage of the fixed record size used in IIO
  iio: kfifo - add poll support.

 drivers/iio/kfifo_buf.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

-- 
1.7.11.1

[PATCH 1/2] iio:kfifo_buf Take advantage of the fixed record size used in IIO

[Jonathan Cameron]

By bypassing the standard macros for setting up the kfifo we can
take advantage of the fixed record size implementation without
having to have a type to pass in (from which the size of an element
is normally established).

In IIO we have variable 'scans' as our records in which any element
can be present or not.  They do not however vary when we are
actually filling or reading from the buffer.  Thus we have a fixed
record size whenever we are actually running.  As setup and tear
down are not in the fast path we can take the overhead of reinitializing
the kfifo every time.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/kfifo_buf.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 6bf9d05..6ec763f 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -22,7 +22,8 @@ static inline int __iio_allocate_kfifo(struct iio_kfifo *buf,
 		return -EINVAL;
 
 	__iio_update_buffer(&buf->buffer, bytes_per_datum, length);
-	return kfifo_alloc(&buf->kf, bytes_per_datum*length, GFP_KERNEL);
+	return __kfifo_alloc((struct __kfifo *)&buf->kf, length,
+			     bytes_per_datum, GFP_KERNEL);
 }
 
 static int iio_request_update_kfifo(struct iio_buffer *r)
@@ -94,9 +95,10 @@ static int iio_store_to_kfifo(struct iio_buffer *r,
 {
 	int ret;
 	struct iio_kfifo *kf = iio_to_kfifo(r);
-	ret = kfifo_in(&kf->kf, data, r->bytes_per_datum);
-	if (ret != r->bytes_per_datum)
+	ret = kfifo_in(&kf->kf, data, 1);
+	if (ret != 1)
 		return -EBUSY;
+
 	return 0;
 }
 
@@ -106,11 +108,12 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
 	int ret, copied;
 	struct iio_kfifo *kf = iio_to_kfifo(r);
 
-	if (n < r->bytes_per_datum)
+	if (n < r->bytes_per_datum || r->bytes_per_datum == 0)
 		return -EINVAL;
 
-	n = rounddown(n, r->bytes_per_datum);
 	ret = kfifo_to_user(&kf->kf, buf, n, &copied);
+	if (ret < 0)
+		return ret;
 
 	return copied;
 }
-- 
1.7.11.1

[PATCH 2/2] iio: kfifo - add poll support.

[Jonathan Cameron]

This buffer implementation was missing poll support.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/kfifo_buf.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 6ec763f..63da424 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -6,6 +6,7 @@
 #include <linux/kfifo.h>
 #include <linux/mutex.h>
 #include <linux/iio/kfifo_buf.h>
+#include <linux/sched.h>
 
 struct iio_kfifo {
 	struct iio_buffer buffer;
@@ -36,6 +37,7 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
 	kfifo_free(&buf->kf);
 	ret = __iio_allocate_kfifo(buf, buf->buffer.bytes_per_datum,
 				   buf->buffer.length);
+	r->stufftoread = false;
 error_ret:
 	return ret;
 }
@@ -82,6 +84,9 @@ static int iio_set_bytes_per_datum_kfifo(struct iio_buffer *r, size_t bpd)
 
 static int iio_set_length_kfifo(struct iio_buffer *r, int length)
 {
+	/* Avoid an invalid state */
+	if (length < 2)
+		length = 2;
 	if (r->length != length) {
 		r->length = length;
 		iio_mark_update_needed_kfifo(r);
@@ -98,6 +103,8 @@ 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(&r->pollq);
 
 	return 0;
 }
@@ -115,6 +122,12 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
 	if (ret < 0)
 		return ret;
 
+	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;
+
 	return copied;
 }
 
@@ -139,7 +152,7 @@ struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev)
 	iio_buffer_init(&kf->buffer);
 	kf->buffer.attrs = &iio_kfifo_attribute_group;
 	kf->buffer.access = &kfifo_access_funcs;
-
+	kf->buffer.length = 2;
 	return &kf->buffer;
 }
 EXPORT_SYMBOL(iio_kfifo_allocate);
-- 
1.7.11.1

Re: [PATCH 1/2] iio:kfifo_buf Take advantage of the fixed record size used in IIO

[Lars-Peter Clausen]

On 06/30/2012 02:52 PM, Jonathan Cameron wrote:
> @@ -106,11 +108,12 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
>  	int ret, copied;
>  	struct iio_kfifo *kf = iio_to_kfifo(r);
>  
> -	if (n < r->bytes_per_datum)
> +	if (n < r->bytes_per_datum || r->bytes_per_datum == 0)
>  		return -EINVAL;

Strictly speaking this is a unrelated change, but well...

I'll apply the kfifo patches to my IIO working tree and see if I'll get any
issues.

- Lars

Re: [PATCH 1/2] iio:kfifo_buf Take advantage of the fixed record size used in IIO

[Jonathan Cameron]

On 7/2/2012 12:56 PM, Lars-Peter Clausen wrote:
> On 06/30/2012 02:52 PM, Jonathan Cameron wrote:
>> @@ -106,11 +108,12 @@ static int iio_read_first_n_kfifo(struct iio_buffer *r,
>>   	int ret, copied;
>>   	struct iio_kfifo *kf = iio_to_kfifo(r);
>>   
>> -	if (n < r->bytes_per_datum)
>> +	if (n < r->bytes_per_datum || r->bytes_per_datum == 0)
>>   		return -EINVAL;
> Strictly speaking this is a unrelated change, but well...
Fair point. Naughty me ;(  Will split out as a separate patch on next 
version.
>
> I'll apply the kfifo patches to my IIO working tree and see if I'll get any
> issues.
Thanks

Re: [PATCH 2/2] iio: kfifo - add poll support.

[Marek Vasut]

Dear Jonathan Cameron,

> This buffer implementation was missing poll support.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>

Was this ever applied ?

> ---
>  drivers/iio/kfifo_buf.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 6ec763f..63da424 100644
> --- a/drivers/iio/kfifo_buf.c
> +++ b/drivers/iio/kfifo_buf.c
> @@ -6,6 +6,7 @@
>  #include <linux/kfifo.h>
>  #include <linux/mutex.h>
>  #include <linux/iio/kfifo_buf.h>
> +#include <linux/sched.h>
> 
>  struct iio_kfifo {
>  	struct iio_buffer buffer;
> @@ -36,6 +37,7 @@ static int iio_request_update_kfifo(struct iio_buffer *r)
>  	kfifo_free(&buf->kf);
>  	ret = __iio_allocate_kfifo(buf, buf->buffer.bytes_per_datum,
>  				   buf->buffer.length);
> +	r->stufftoread = false;
>  error_ret:
>  	return ret;
>  }
> @@ -82,6 +84,9 @@ static int iio_set_bytes_per_datum_kfifo(struct
> iio_buffer *r, size_t bpd)
> 
>  static int iio_set_length_kfifo(struct iio_buffer *r, int length)
>  {
> +	/* Avoid an invalid state */
> +	if (length < 2)
> +		length = 2;
>  	if (r->length != length) {
>  		r->length = length;
>  		iio_mark_update_needed_kfifo(r);
> @@ -98,6 +103,8 @@ 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(&r->pollq);
> 
>  	return 0;
>  }
> @@ -115,6 +122,12 @@ static int iio_read_first_n_kfifo(struct iio_buffer
> *r, if (ret < 0)
>  		return ret;
> 
> +	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;
> +
>  	return copied;
>  }
> 
> @@ -139,7 +152,7 @@ struct iio_buffer *iio_kfifo_allocate(struct iio_dev
> *indio_dev) iio_buffer_init(&kf->buffer);
>  	kf->buffer.attrs = &iio_kfifo_attribute_group;
>  	kf->buffer.access = &kfifo_access_funcs;
> -
> +	kf->buffer.length = 2;
>  	return &kf->buffer;
>  }
>  EXPORT_SYMBOL(iio_kfifo_allocate);

Best regards,
Marek Vasut

Re: [PATCH 2/2] iio: kfifo - add poll support.

[Jonathan Cameron]

Marek Vasut <marex@denx.de> wrote:

>Dear Jonathan Cameron,
>
>> This buffer implementation was missing poll support.
>> 
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>
>Was this ever applied ?
On todo  list.  Always easier to not get around to my own patches when running low on time.  Will hopefully get time on Monday...
>
>> ---
>>  drivers/iio/kfifo_buf.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
>> index 6ec763f..63da424 100644
>> --- a/drivers/iio/kfifo_buf.c
>> +++ b/drivers/iio/kfifo_buf.c
>> @@ -6,6 +6,7 @@
>>  #include <linux/kfifo.h>
>>  #include <linux/mutex.h>
>>  #include <linux/iio/kfifo_buf.h>
>> +#include <linux/sched.h>
>> 
>>  struct iio_kfifo {
>>  	struct iio_buffer buffer;
>> @@ -36,6 +37,7 @@ static int iio_request_update_kfifo(struct
>iio_buffer *r)
>>  	kfifo_free(&buf->kf);
>>  	ret = __iio_allocate_kfifo(buf, buf->buffer.bytes_per_datum,
>>  				   buf->buffer.length);
>> +	r->stufftoread = false;
>>  error_ret:
>>  	return ret;
>>  }
>> @@ -82,6 +84,9 @@ static int iio_set_bytes_per_datum_kfifo(struct
>> iio_buffer *r, size_t bpd)
>> 
>>  static int iio_set_length_kfifo(struct iio_buffer *r, int length)
>>  {
>> +	/* Avoid an invalid state */
>> +	if (length < 2)
>> +		length = 2;
>>  	if (r->length != length) {
>>  		r->length = length;
>>  		iio_mark_update_needed_kfifo(r);
>> @@ -98,6 +103,8 @@ 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(&r->pollq);
>> 
>>  	return 0;
>>  }
>> @@ -115,6 +122,12 @@ static int iio_read_first_n_kfifo(struct
>iio_buffer
>> *r, if (ret < 0)
>>  		return ret;
>> 
>> +	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;
>> +
>>  	return copied;
>>  }
>> 
>> @@ -139,7 +152,7 @@ struct iio_buffer *iio_kfifo_allocate(struct
>iio_dev
>> *indio_dev) iio_buffer_init(&kf->buffer);
>>  	kf->buffer.attrs = &iio_kfifo_attribute_group;
>>  	kf->buffer.access = &kfifo_access_funcs;
>> -
>> +	kf->buffer.length = 2;
>>  	return &kf->buffer;
>>  }
>>  EXPORT_SYMBOL(iio_kfifo_allocate);
>
>Best regards,
>Marek Vasut
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 2/2] iio: kfifo - add poll support.

[Marek Vasut]

Dear Jonathan Cameron,

> Marek Vasut <marex@denx.de> wrote:
> >Dear Jonathan Cameron,
> >
> >> This buffer implementation was missing poll support.
> >> 
> >> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> >
> >Was this ever applied ?
> 
> On todo  list.  Always easier to not get around to my own patches when
> running low on time.  Will hopefully get time on Monday...

Don't sweat it, thanks for the update! :)
[...]

> >Best regards,
> >Marek Vasut

Best regards,
Marek Vasut