[PATCH 1/2] staging: greybus: remove unused kfifo_ts

[PATCH 1/2] staging: greybus: remove unused kfifo_ts

[Arnd Bergmann]

As of commit 8e1d6c336d74 ("greybus: loopback: drop bus aggregate
calculation"), nothing ever reads from kfifo_ts, so there is no
reason to write to it or even allocate it any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/greybus/loopback.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 08e255884206..85046fb16aad 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -72,7 +72,6 @@ struct gb_loopback {
 
 	struct dentry *file;
 	struct kfifo kfifo_lat;
-	struct kfifo kfifo_ts;
 	struct mutex mutex;
 	struct task_struct *task;
 	struct list_head entry;
@@ -262,7 +261,6 @@ static void gb_loopback_check_attr(struct gb_loopback *gb)
 			 gb->iteration_max, kfifo_depth);
 	}
 	kfifo_reset_out(&gb->kfifo_lat);
-	kfifo_reset_out(&gb->kfifo_ts);
 
 	switch (gb->type) {
 	case GB_LOOPBACK_TYPE_PING:
@@ -387,13 +385,6 @@ static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)
 	return __gb_loopback_calc_latency(t1, t2);
 }
 
-static void gb_loopback_push_latency_ts(struct gb_loopback *gb,
-					struct timeval *ts, struct timeval *te)
-{
-	kfifo_in(&gb->kfifo_ts, (unsigned char *)ts, sizeof(*ts));
-	kfifo_in(&gb->kfifo_ts, (unsigned char *)te, sizeof(*te));
-}
-
 static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
 				      void *request, int request_size,
 				      void *response, int response_size)
@@ -433,7 +424,6 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
 	do_gettimeofday(&te);
 
 	/* Calculate the total time the message took */
-	gb_loopback_push_latency_ts(gb, &ts, &te);
 	gb->elapsed_nsecs = gb_loopback_calc_latency(&ts, &te);
 
 out_put_operation:
@@ -521,11 +511,9 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
 				err = true;
 	}
 
-	if (!err) {
-		gb_loopback_push_latency_ts(gb, &op_async->ts, &te);
+	if (!err)
 		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
 							     &te);
-	}
 
 	if (op_async->pending) {
 		if (err)
@@ -1241,18 +1229,12 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
 		retval = -ENOMEM;
 		goto out_conn;
 	}
-	if (kfifo_alloc(&gb->kfifo_ts, kfifo_depth * sizeof(struct timeval) * 2,
-			  GFP_KERNEL)) {
-		retval = -ENOMEM;
-		goto out_kfifo0;
-	}
-
 	/* Fork worker thread */
 	mutex_init(&gb->mutex);
 	gb->task = kthread_run(gb_loopback_fn, gb, "gb_loopback");
 	if (IS_ERR(gb->task)) {
 		retval = PTR_ERR(gb->task);
-		goto out_kfifo1;
+		goto out_kfifo;
 	}
 
 	spin_lock_irqsave(&gb_dev.lock, flags);
@@ -1266,9 +1248,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
 
 	return 0;
 
-out_kfifo1:
-	kfifo_free(&gb->kfifo_ts);
-out_kfifo0:
+out_kfifo:
 	kfifo_free(&gb->kfifo_lat);
 out_conn:
 	device_unregister(dev);
@@ -1302,7 +1282,6 @@ static void gb_loopback_disconnect(struct gb_bundle *bundle)
 		kthread_stop(gb->task);
 
 	kfifo_free(&gb->kfifo_lat);
-	kfifo_free(&gb->kfifo_ts);
 	gb_connection_latency_tag_disable(gb->connection);
 	debugfs_remove(gb->file);
 
-- 
2.9.0

[PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals

[Arnd Bergmann]

This driver is the only one using the deprecated timeval_to_ns()
helper. Changing it from do_gettimeofday() to ktime_get() makes
the code more efficient, more robust against concurrent
settimeofday(), more accurate and lets us get rid of that helper
in the future.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/greybus/loopback.c | 42 ++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
index 85046fb16aad..3d92638c424b 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -58,7 +58,7 @@ static struct gb_loopback_device gb_dev;
 struct gb_loopback_async_operation {
 	struct gb_loopback *gb;
 	struct gb_operation *operation;
-	struct timeval ts;
+	ktime_t ts;
 	struct timer_list timer;
 	struct list_head entry;
 	struct work_struct work;
@@ -81,7 +81,7 @@ struct gb_loopback {
 	atomic_t outstanding_operations;
 
 	/* Per connection stats */
-	struct timeval ts;
+	ktime_t ts;
 	struct gb_loopback_stats latency;
 	struct gb_loopback_stats throughput;
 	struct gb_loopback_stats requests_per_second;
@@ -375,14 +375,9 @@ static u64 __gb_loopback_calc_latency(u64 t1, u64 t2)
 		return NSEC_PER_DAY - t2 + t1;
 }
 
-static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)
+static u64 gb_loopback_calc_latency(ktime_t ts, ktime_t te)
 {
-	u64 t1, t2;
-
-	t1 = timeval_to_ns(ts);
-	t2 = timeval_to_ns(te);
-
-	return __gb_loopback_calc_latency(t1, t2);
+	return __gb_loopback_calc_latency(ktime_to_ns(ts), ktime_to_ns(te));
 }
 
 static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
@@ -390,10 +385,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
 				      void *response, int response_size)
 {
 	struct gb_operation *operation;
-	struct timeval ts, te;
+	ktime_t ts, te;
 	int ret;
 
-	do_gettimeofday(&ts);
+	ts = ktime_get();
 	operation = gb_operation_create(gb->connection, type, request_size,
 					response_size, GFP_KERNEL);
 	if (!operation)
@@ -421,10 +416,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
 		}
 	}
 
-	do_gettimeofday(&te);
+	te = ktime_get();
 
 	/* Calculate the total time the message took */
-	gb->elapsed_nsecs = gb_loopback_calc_latency(&ts, &te);
+	gb->elapsed_nsecs = gb_loopback_calc_latency(ts, te);
 
 out_put_operation:
 	gb_operation_put(operation);
@@ -492,10 +487,10 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
 {
 	struct gb_loopback_async_operation *op_async;
 	struct gb_loopback *gb;
-	struct timeval te;
+	ktime_t te;
 	bool err = false;
 
-	do_gettimeofday(&te);
+	te = ktime_get();
 	op_async = gb_loopback_operation_find(operation->id);
 	if (!op_async)
 		return;
@@ -512,8 +507,7 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
 	}
 
 	if (!err)
-		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
-							     &te);
+		gb->elapsed_nsecs = gb_loopback_calc_latency(op_async->ts, te);
 
 	if (op_async->pending) {
 		if (err)
@@ -608,7 +602,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
 	list_add_tail(&op_async->entry, &gb_dev.list_op_async);
 	spin_unlock_irqrestore(&gb_dev.lock, flags);
 
-	do_gettimeofday(&op_async->ts);
+	op_async->ts = ktime_get();
 	op_async->pending = true;
 	atomic_inc(&gb->outstanding_operations);
 	mutex_lock(&gb->mutex);
@@ -842,7 +836,7 @@ static void gb_loopback_reset_stats(struct gb_loopback *gb)
 	/* Should be initialized at least once per transaction set */
 	gb->apbridge_latency_ts = 0;
 	gb->gbphy_latency_ts = 0;
-	memset(&gb->ts, 0, sizeof(struct timeval));
+	gb->ts = ktime_set(0, 0);
 }
 
 static void gb_loopback_update_stats(struct gb_loopback_stats *stats, u32 val)
@@ -925,15 +919,15 @@ static void gb_loopback_calculate_stats(struct gb_loopback *gb, bool error)
 {
 	u64 nlat;
 	u32 lat;
-	struct timeval te;
+	ktime_t te;
 
 	if (!error) {
 		gb->requests_completed++;
 		gb_loopback_calculate_latency_stats(gb);
 	}
 
-	do_gettimeofday(&te);
-	nlat = gb_loopback_calc_latency(&gb->ts, &te);
+	te = ktime_get();
+	nlat = gb_loopback_calc_latency(gb->ts, te);
 	if (nlat >= NSEC_PER_SEC || gb->iteration_count == gb->iteration_max) {
 		lat = gb_loopback_nsec_to_usec_latency(nlat);
 
@@ -1017,8 +1011,8 @@ static int gb_loopback_fn(void *data)
 		size = gb->size;
 		us_wait = gb->us_wait;
 		type = gb->type;
-		if (gb->ts.tv_usec == 0 && gb->ts.tv_sec == 0)
-			do_gettimeofday(&gb->ts);
+		if (ktime_to_ns(gb->ts) == 0)
+			gb->ts = ktime_get();
 		mutex_unlock(&gb->mutex);
 
 		/* Else operations to perform */
-- 
2.9.0

Re: [greybus-dev] [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals

[Viresh Kumar]

On 02-11-17, 15:32, Arnd Bergmann wrote:
> This driver is the only one using the deprecated timeval_to_ns()
> helper. Changing it from do_gettimeofday() to ktime_get() makes
> the code more efficient, more robust against concurrent
> settimeofday(), more accurate and lets us get rid of that helper
> in the future.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/staging/greybus/loopback.c | 42 ++++++++++++++++----------------------
>  1 file changed, 18 insertions(+), 24 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals

[Bryan O'Donoghue]

On 02/11/17 14:32, Arnd Bergmann wrote:
> This driver is the only one using the deprecated timeval_to_ns()
> helper. Changing it from do_gettimeofday() to ktime_get() makes
> the code more efficient, more robust against concurrent
> settimeofday(), more accurate and lets us get rid of that helper
> in the future.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/staging/greybus/loopback.c | 42 ++++++++++++++++----------------------
>   1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 85046fb16aad..3d92638c424b 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -58,7 +58,7 @@ static struct gb_loopback_device gb_dev;
>   struct gb_loopback_async_operation {
>   	struct gb_loopback *gb;
>   	struct gb_operation *operation;
> -	struct timeval ts;
> +	ktime_t ts;
>   	struct timer_list timer;
>   	struct list_head entry;
>   	struct work_struct work;
> @@ -81,7 +81,7 @@ struct gb_loopback {
>   	atomic_t outstanding_operations;
>   
>   	/* Per connection stats */
> -	struct timeval ts;
> +	ktime_t ts;
>   	struct gb_loopback_stats latency;
>   	struct gb_loopback_stats throughput;
>   	struct gb_loopback_stats requests_per_second;
> @@ -375,14 +375,9 @@ static u64 __gb_loopback_calc_latency(u64 t1, u64 t2)
>   		return NSEC_PER_DAY - t2 + t1;
>   }
>   
> -static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)
> +static u64 gb_loopback_calc_latency(ktime_t ts, ktime_t te)
>   {
> -	u64 t1, t2;
> -
> -	t1 = timeval_to_ns(ts);
> -	t2 = timeval_to_ns(te);
> -
> -	return __gb_loopback_calc_latency(t1, t2);
> +	return __gb_loopback_calc_latency(ktime_to_ns(ts), ktime_to_ns(te));
>   }
>   
>   static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
> @@ -390,10 +385,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   				      void *response, int response_size)
>   {
>   	struct gb_operation *operation;
> -	struct timeval ts, te;
> +	ktime_t ts, te;
>   	int ret;
>   
> -	do_gettimeofday(&ts);
> +	ts = ktime_get();
>   	operation = gb_operation_create(gb->connection, type, request_size,
>   					response_size, GFP_KERNEL);
>   	if (!operation)
> @@ -421,10 +416,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   		}
>   	}
>   
> -	do_gettimeofday(&te);
> +	te = ktime_get();
>   
>   	/* Calculate the total time the message took */
> -	gb->elapsed_nsecs = gb_loopback_calc_latency(&ts, &te);
> +	gb->elapsed_nsecs = gb_loopback_calc_latency(ts, te);
>   
>   out_put_operation:
>   	gb_operation_put(operation);
> @@ -492,10 +487,10 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
>   {
>   	struct gb_loopback_async_operation *op_async;
>   	struct gb_loopback *gb;
> -	struct timeval te;
> +	ktime_t te;
>   	bool err = false;
>   
> -	do_gettimeofday(&te);
> +	te = ktime_get();
>   	op_async = gb_loopback_operation_find(operation->id);
>   	if (!op_async)
>   		return;
> @@ -512,8 +507,7 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
>   	}
>   
>   	if (!err)
> -		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
> -							     &te);
> +		gb->elapsed_nsecs = gb_loopback_calc_latency(op_async->ts, te);
>   
>   	if (op_async->pending) {
>   		if (err)
> @@ -608,7 +602,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
>   	list_add_tail(&op_async->entry, &gb_dev.list_op_async);
>   	spin_unlock_irqrestore(&gb_dev.lock, flags);
>   
> -	do_gettimeofday(&op_async->ts);
> +	op_async->ts = ktime_get();
>   	op_async->pending = true;
>   	atomic_inc(&gb->outstanding_operations);
>   	mutex_lock(&gb->mutex);
> @@ -842,7 +836,7 @@ static void gb_loopback_reset_stats(struct gb_loopback *gb)
>   	/* Should be initialized at least once per transaction set */
>   	gb->apbridge_latency_ts = 0;
>   	gb->gbphy_latency_ts = 0;
> -	memset(&gb->ts, 0, sizeof(struct timeval));
> +	gb->ts = ktime_set(0, 0);
>   }
>   
>   static void gb_loopback_update_stats(struct gb_loopback_stats *stats, u32 val)
> @@ -925,15 +919,15 @@ static void gb_loopback_calculate_stats(struct gb_loopback *gb, bool error)
>   {
>   	u64 nlat;
>   	u32 lat;
> -	struct timeval te;
> +	ktime_t te;
>   
>   	if (!error) {
>   		gb->requests_completed++;
>   		gb_loopback_calculate_latency_stats(gb);
>   	}
>   
> -	do_gettimeofday(&te);
> -	nlat = gb_loopback_calc_latency(&gb->ts, &te);
> +	te = ktime_get();
> +	nlat = gb_loopback_calc_latency(gb->ts, te);
>   	if (nlat >= NSEC_PER_SEC || gb->iteration_count == gb->iteration_max) {
>   		lat = gb_loopback_nsec_to_usec_latency(nlat);
>   
> @@ -1017,8 +1011,8 @@ static int gb_loopback_fn(void *data)
>   		size = gb->size;
>   		us_wait = gb->us_wait;
>   		type = gb->type;
> -		if (gb->ts.tv_usec == 0 && gb->ts.tv_sec == 0)
> -			do_gettimeofday(&gb->ts);
> +		if (ktime_to_ns(gb->ts) == 0)
> +			gb->ts = ktime_get();
>   		mutex_unlock(&gb->mutex);
>   
>   		/* Else operations to perform */
> 

This one I'd like to test first.

I'll get back to you

Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals

[Bryan O'Donoghue]

On 02/11/17 14:32, Arnd Bergmann wrote:
> This driver is the only one using the deprecated timeval_to_ns()
> helper. Changing it from do_gettimeofday() to ktime_get() makes
> the code more efficient, more robust against concurrent
> settimeofday(), more accurate and lets us get rid of that helper
> in the future.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/staging/greybus/loopback.c | 42 ++++++++++++++++----------------------
>   1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 85046fb16aad..3d92638c424b 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -58,7 +58,7 @@ static struct gb_loopback_device gb_dev;
>   struct gb_loopback_async_operation {
>   	struct gb_loopback *gb;
>   	struct gb_operation *operation;
> -	struct timeval ts;
> +	ktime_t ts;
>   	struct timer_list timer;
>   	struct list_head entry;
>   	struct work_struct work;
> @@ -81,7 +81,7 @@ struct gb_loopback {
>   	atomic_t outstanding_operations;
>   
>   	/* Per connection stats */
> -	struct timeval ts;
> +	ktime_t ts;
>   	struct gb_loopback_stats latency;
>   	struct gb_loopback_stats throughput;
>   	struct gb_loopback_stats requests_per_second;
> @@ -375,14 +375,9 @@ static u64 __gb_loopback_calc_latency(u64 t1, u64 t2)
>   		return NSEC_PER_DAY - t2 + t1;
>   }
>   
> -static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)
> +static u64 gb_loopback_calc_latency(ktime_t ts, ktime_t te)
>   {
> -	u64 t1, t2;
> -
> -	t1 = timeval_to_ns(ts);
> -	t2 = timeval_to_ns(te);
> -
> -	return __gb_loopback_calc_latency(t1, t2);
> +	return __gb_loopback_calc_latency(ktime_to_ns(ts), ktime_to_ns(te));
>   }
>   
>   static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
> @@ -390,10 +385,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   				      void *response, int response_size)
>   {
>   	struct gb_operation *operation;
> -	struct timeval ts, te;
> +	ktime_t ts, te;
>   	int ret;
>   
> -	do_gettimeofday(&ts);
> +	ts = ktime_get();
>   	operation = gb_operation_create(gb->connection, type, request_size,
>   					response_size, GFP_KERNEL);
>   	if (!operation)
> @@ -421,10 +416,10 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   		}
>   	}
>   
> -	do_gettimeofday(&te);
> +	te = ktime_get();
>   
>   	/* Calculate the total time the message took */
> -	gb->elapsed_nsecs = gb_loopback_calc_latency(&ts, &te);
> +	gb->elapsed_nsecs = gb_loopback_calc_latency(ts, te);
>   
>   out_put_operation:
>   	gb_operation_put(operation);
> @@ -492,10 +487,10 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
>   {
>   	struct gb_loopback_async_operation *op_async;
>   	struct gb_loopback *gb;
> -	struct timeval te;
> +	ktime_t te;
>   	bool err = false;
>   
> -	do_gettimeofday(&te);
> +	te = ktime_get();
>   	op_async = gb_loopback_operation_find(operation->id);
>   	if (!op_async)
>   		return;
> @@ -512,8 +507,7 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
>   	}
>   
>   	if (!err)
> -		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
> -							     &te);
> +		gb->elapsed_nsecs = gb_loopback_calc_latency(op_async->ts, te);
>   
>   	if (op_async->pending) {
>   		if (err)
> @@ -608,7 +602,7 @@ static int gb_loopback_async_operation(struct gb_loopback *gb, int type,
>   	list_add_tail(&op_async->entry, &gb_dev.list_op_async);
>   	spin_unlock_irqrestore(&gb_dev.lock, flags);
>   
> -	do_gettimeofday(&op_async->ts);
> +	op_async->ts = ktime_get();
>   	op_async->pending = true;
>   	atomic_inc(&gb->outstanding_operations);
>   	mutex_lock(&gb->mutex);
> @@ -842,7 +836,7 @@ static void gb_loopback_reset_stats(struct gb_loopback *gb)
>   	/* Should be initialized at least once per transaction set */
>   	gb->apbridge_latency_ts = 0;
>   	gb->gbphy_latency_ts = 0;
> -	memset(&gb->ts, 0, sizeof(struct timeval));
> +	gb->ts = ktime_set(0, 0);
>   }
>   
>   static void gb_loopback_update_stats(struct gb_loopback_stats *stats, u32 val)
> @@ -925,15 +919,15 @@ static void gb_loopback_calculate_stats(struct gb_loopback *gb, bool error)
>   {
>   	u64 nlat;
>   	u32 lat;
> -	struct timeval te;
> +	ktime_t te;
>   
>   	if (!error) {
>   		gb->requests_completed++;
>   		gb_loopback_calculate_latency_stats(gb);
>   	}
>   
> -	do_gettimeofday(&te);
> -	nlat = gb_loopback_calc_latency(&gb->ts, &te);
> +	te = ktime_get();
> +	nlat = gb_loopback_calc_latency(gb->ts, te);
>   	if (nlat >= NSEC_PER_SEC || gb->iteration_count == gb->iteration_max) {
>   		lat = gb_loopback_nsec_to_usec_latency(nlat);
>   
> @@ -1017,8 +1011,8 @@ static int gb_loopback_fn(void *data)
>   		size = gb->size;
>   		us_wait = gb->us_wait;
>   		type = gb->type;
> -		if (gb->ts.tv_usec == 0 && gb->ts.tv_sec == 0)
> -			do_gettimeofday(&gb->ts);
> +		if (ktime_to_ns(gb->ts) == 0)
> +			gb->ts = ktime_get();
>   		mutex_unlock(&gb->mutex);
>   
>   		/* Else operations to perform */
> 

Fine with this change too.

Acked-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts

[Bryan O'Donoghue]

On 02/11/17 14:32, Arnd Bergmann wrote:
> As of commit 8e1d6c336d74 ("greybus: loopback: drop bus aggregate
> calculation"), nothing ever reads from kfifo_ts, so there is no
> reason to write to it or even allocate it any more.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/staging/greybus/loopback.c | 27 +++------------------------
>   1 file changed, 3 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> index 08e255884206..85046fb16aad 100644
> --- a/drivers/staging/greybus/loopback.c
> +++ b/drivers/staging/greybus/loopback.c
> @@ -72,7 +72,6 @@ struct gb_loopback {
>   
>   	struct dentry *file;
>   	struct kfifo kfifo_lat;
> -	struct kfifo kfifo_ts;
>   	struct mutex mutex;
>   	struct task_struct *task;
>   	struct list_head entry;
> @@ -262,7 +261,6 @@ static void gb_loopback_check_attr(struct gb_loopback *gb)
>   			 gb->iteration_max, kfifo_depth);
>   	}
>   	kfifo_reset_out(&gb->kfifo_lat);
> -	kfifo_reset_out(&gb->kfifo_ts);
>   
>   	switch (gb->type) {
>   	case GB_LOOPBACK_TYPE_PING:
> @@ -387,13 +385,6 @@ static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)
>   	return __gb_loopback_calc_latency(t1, t2);
>   }
>   
> -static void gb_loopback_push_latency_ts(struct gb_loopback *gb,
> -					struct timeval *ts, struct timeval *te)
> -{
> -	kfifo_in(&gb->kfifo_ts, (unsigned char *)ts, sizeof(*ts));
> -	kfifo_in(&gb->kfifo_ts, (unsigned char *)te, sizeof(*te));
> -}
> -
>   static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   				      void *request, int request_size,
>   				      void *response, int response_size)
> @@ -433,7 +424,6 @@ static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
>   	do_gettimeofday(&te);
>   
>   	/* Calculate the total time the message took */
> -	gb_loopback_push_latency_ts(gb, &ts, &te);
>   	gb->elapsed_nsecs = gb_loopback_calc_latency(&ts, &te);
>   
>   out_put_operation:
> @@ -521,11 +511,9 @@ static void gb_loopback_async_operation_callback(struct gb_operation *operation)
>   				err = true;
>   	}
>   
> -	if (!err) {
> -		gb_loopback_push_latency_ts(gb, &op_async->ts, &te);
> +	if (!err)
>   		gb->elapsed_nsecs = gb_loopback_calc_latency(&op_async->ts,
>   							     &te);
> -	}
>   
>   	if (op_async->pending) {
>   		if (err)
> @@ -1241,18 +1229,12 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
>   		retval = -ENOMEM;
>   		goto out_conn;
>   	}
> -	if (kfifo_alloc(&gb->kfifo_ts, kfifo_depth * sizeof(struct timeval) * 2,
> -			  GFP_KERNEL)) {
> -		retval = -ENOMEM;
> -		goto out_kfifo0;
> -	}
> -
>   	/* Fork worker thread */
>   	mutex_init(&gb->mutex);
>   	gb->task = kthread_run(gb_loopback_fn, gb, "gb_loopback");
>   	if (IS_ERR(gb->task)) {
>   		retval = PTR_ERR(gb->task);
> -		goto out_kfifo1;
> +		goto out_kfifo;
>   	}
>   
>   	spin_lock_irqsave(&gb_dev.lock, flags);
> @@ -1266,9 +1248,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
>   
>   	return 0;
>   
> -out_kfifo1:
> -	kfifo_free(&gb->kfifo_ts);
> -out_kfifo0:
> +out_kfifo:
>   	kfifo_free(&gb->kfifo_lat);
>   out_conn:
>   	device_unregister(dev);
> @@ -1302,7 +1282,6 @@ static void gb_loopback_disconnect(struct gb_bundle *bundle)
>   		kthread_stop(gb->task);
>   
>   	kfifo_free(&gb->kfifo_lat);
> -	kfifo_free(&gb->kfifo_ts);
>   	gb_connection_latency_tag_disable(gb->connection);
>   	debugfs_remove(gb->file);
>   
> 

This looks right to me

Reviewed-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>

Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts

[Viresh Kumar]

On 02-11-17, 15:32, Arnd Bergmann wrote:
> As of commit 8e1d6c336d74 ("greybus: loopback: drop bus aggregate
> calculation"), nothing ever reads from kfifo_ts, so there is no
> reason to write to it or even allocate it any more.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/staging/greybus/loopback.c | 27 +++------------------------
>  1 file changed, 3 insertions(+), 24 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh