[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <14417f49-1a82-59c4-8e12-59e862368f29@nexus-software.ie>
Date: Sat, 4 Nov 2017 16:50:30 +0000
From: Bryan O'Donoghue <pure.logic@...us-software.ie>
To: Arnd Bergmann <arnd@...db.de>, Johan Hovold <johan@...nel.org>,
Alex Elder <elder@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Gioh Kim <gi-oh.kim@...fitbricks.com>,
Abdul Rauf <abdulraufmujahid@...il.com>,
Arushi Singhal <arushisinghal19971997@...il.com>,
greybus-dev@...ts.linaro.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time
intervals
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@...db.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@...us-software.ie>
Powered by blists - more mailing lists