* [PATCH 1/2] staging: greybus: remove unused kfifo_ts
@ 2017-11-02 14:32 Arnd Bergmann
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Arnd Bergmann @ 2017-11-02 14:32 UTC (permalink / raw)
To: Bryan O'Donoghue, Johan Hovold, Alex Elder, Greg Kroah-Hartman
Cc: Arnd Bergmann, Viresh Kumar, Gioh Kim, Arushi Singhal,
Abdul Rauf, greybus-dev, devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals
2017-11-02 14:32 [PATCH 1/2] staging: greybus: remove unused kfifo_ts Arnd Bergmann
@ 2017-11-02 14:32 ` Arnd Bergmann
2017-11-03 3:59 ` [greybus-dev] " Viresh Kumar
` (2 more replies)
2017-11-03 10:28 ` [PATCH 1/2] staging: greybus: remove unused kfifo_ts Bryan O'Donoghue
2017-11-03 10:44 ` Viresh Kumar
2 siblings, 3 replies; 7+ messages in thread
From: Arnd Bergmann @ 2017-11-02 14:32 UTC (permalink / raw)
To: Bryan O'Donoghue, Johan Hovold, Alex Elder, Greg Kroah-Hartman
Cc: Arnd Bergmann, Gioh Kim, Abdul Rauf, Arushi Singhal, greybus-dev,
devel, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [greybus-dev] [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
@ 2017-11-03 3:59 ` Viresh Kumar
2017-11-03 10:28 ` Bryan O'Donoghue
2017-11-04 16:50 ` Bryan O'Donoghue
2 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2017-11-03 3:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bryan O'Donoghue, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Arushi Singhal, devel, Abdul Rauf,
linux-kernel, greybus-dev, Gioh Kim
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts
2017-11-02 14:32 [PATCH 1/2] staging: greybus: remove unused kfifo_ts Arnd Bergmann
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
@ 2017-11-03 10:28 ` Bryan O'Donoghue
2017-11-03 10:44 ` Viresh Kumar
2 siblings, 0 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2017-11-03 10:28 UTC (permalink / raw)
To: Arnd Bergmann, Johan Hovold, Alex Elder, Greg Kroah-Hartman
Cc: Viresh Kumar, Gioh Kim, Arushi Singhal, Abdul Rauf, greybus-dev,
devel, linux-kernel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
2017-11-03 3:59 ` [greybus-dev] " Viresh Kumar
@ 2017-11-03 10:28 ` Bryan O'Donoghue
2017-11-04 16:50 ` Bryan O'Donoghue
2 siblings, 0 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2017-11-03 10:28 UTC (permalink / raw)
To: Arnd Bergmann, Johan Hovold, Alex Elder, Greg Kroah-Hartman
Cc: Gioh Kim, Abdul Rauf, Arushi Singhal, greybus-dev, devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts
2017-11-02 14:32 [PATCH 1/2] staging: greybus: remove unused kfifo_ts Arnd Bergmann
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
2017-11-03 10:28 ` [PATCH 1/2] staging: greybus: remove unused kfifo_ts Bryan O'Donoghue
@ 2017-11-03 10:44 ` Viresh Kumar
2 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2017-11-03 10:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bryan O'Donoghue, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Gioh Kim, Arushi Singhal, Abdul Rauf,
greybus-dev, devel, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
2017-11-03 3:59 ` [greybus-dev] " Viresh Kumar
2017-11-03 10:28 ` Bryan O'Donoghue
@ 2017-11-04 16:50 ` Bryan O'Donoghue
2 siblings, 0 replies; 7+ messages in thread
From: Bryan O'Donoghue @ 2017-11-04 16:50 UTC (permalink / raw)
To: Arnd Bergmann, Johan Hovold, Alex Elder, Greg Kroah-Hartman
Cc: Gioh Kim, Abdul Rauf, Arushi Singhal, greybus-dev, devel, linux-kernel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-04 16:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 14:32 [PATCH 1/2] staging: greybus: remove unused kfifo_ts Arnd Bergmann
2017-11-02 14:32 ` [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals Arnd Bergmann
2017-11-03 3:59 ` [greybus-dev] " Viresh Kumar
2017-11-03 10:28 ` Bryan O'Donoghue
2017-11-04 16:50 ` Bryan O'Donoghue
2017-11-03 10:28 ` [PATCH 1/2] staging: greybus: remove unused kfifo_ts Bryan O'Donoghue
2017-11-03 10:44 ` Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).