Skip to contentSkip to conversation

trace: Use 64-bit timekeeping

PATCH Superseded Tina Ruchandani 2 participants 2 messages Jan 27, 2015

Subsystems (2)
Tracing Tracing ring buffer

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Tina Ruchandani

author
#0 lore raw

PATCH v3 1/1 trace: Use 64-bit timekeeping

The ring_buffer_producer uses 'struct timeval' to measure its start and end times. 'struct timeval' on 32-bit systems will have its tv_sec value overflow in year 2038 and beyond. This patch replaces struct timeval with 'ktime_t' which uses 64-bit representation for seconds

Suggested-by Arnd Bergmann <[email protected]>
Signed-off-by Tina Ruchandani <[email protected]>

---
Changes in v3:
- Use a more efficient way to compute condition for exiting loop.
- Fix variable naming - all caps is only for macros.
Changes in v2:
- Use ktime_t instead of timespec64 for efficiency reasons.

---
 kernel/trace/ring_buffer_benchmark.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
kernel/trace/ring_buffer_benchmark.c +8 -9
index 3f9e328..0a50abb 100644
@@ -7,7 +7,7 @@
 #include <linux/completion.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
-#include <linux/time.h>
+#include <linux/ktime.h>
 #include <asm/local.h>
 
 struct rb_page {

@@ -212,8 +212,7 @@ static void ring_buffer_consumer(void)
 
 static void ring_buffer_producer(void)
 {
-	struct timeval start_tv;
-	struct timeval end_tv;
+	ktime_t start_time, end_time, timeout;
 	unsigned long long time;
 	unsigned long long entries;
 	unsigned long long overruns;

@@ -227,7 +226,9 @@ static void ring_buffer_producer(void)
 	 * make the system stall)
 	 */
 	trace_printk("Starting ring buffer hammer\n");
-	do_gettimeofday(&start_tv);
+	start_time = ktime_get();
+	timeout = ktime_add_ns(start_time,
+			       ((long long) RUN_TIME) * NSEC_PER_SEC);
 	do {
 		struct ring_buffer_event *event;
 		int *entry;

@@ -244,7 +245,7 @@ static void ring_buffer_producer(void)
 				ring_buffer_unlock_commit(buffer, event);
 			}
 		}
-		do_gettimeofday(&end_tv);
+		end_time = ktime_get();
 
 		cnt++;
 		if (consumer && !(cnt % wakeup_interval))

@@ -264,7 +265,7 @@ static void ring_buffer_producer(void)
 			cond_resched();
 #endif
 
-	} while (end_tv.tv_sec < (start_tv.tv_sec + RUN_TIME) && !kill_test);
+	} while (ktime_before(end_time, timeout) && !kill_test);
 	trace_printk("End ring buffer hammer\n");
 
 	if (consumer) {

@@ -280,9 +281,7 @@ static void ring_buffer_producer(void)
 		wait_for_completion(&read_done);
 	}
 
-	time = end_tv.tv_sec - start_tv.tv_sec;
-	time *= USEC_PER_SEC;
-	time += (long long)((long)end_tv.tv_usec - (long)start_tv.tv_usec);
+	time = ktime_us_delta(end_time, start_time);
 
 	entries = ring_buffer_entries(buffer);
 	overruns = ring_buffer_overruns(buffer);
Signature · 1 line
2.2.0.rc0.207.ga3a616c
maintainer
#1 lore raw
Quoted 5 lines · On Tue, 27 Jan 2015 21:25:38 +0530 Tina Ruchandani <[email protected]>…

The ring_buffer_producer uses 'struct timeval' to measure its start and end times. 'struct timeval' on 32-bit systems will have its tv_sec value overflow in year 2038 and beyond. This patch replaces struct timeval with 'ktime_t' which uses 64-bit representation for seconds

Doesn't ktime_t actually use 64-bit representation for nanoseconds?

Quoted 38 lines
Suggested-by Arnd Bergmann <[email protected]>
Signed-off-by Tina Ruchandani <[email protected]>

---
Changes in v3:
- Use a more efficient way to compute condition for exiting loop.
- Fix variable naming - all caps is only for macros.
Changes in v2:
- Use ktime_t instead of timespec64 for efficiency reasons.

---
 kernel/trace/ring_buffer_benchmark.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
kernel/trace/ring_buffer_benchmark.c +2 -3
index 3f9e328..0a50abb 100644
@@ -7,7 +7,7 @@
 #include <linux/completion.h>
 #include <linux/kthread.h>
 #include <linux/module.h>
-#include <linux/time.h>
+#include <linux/ktime.h>
 #include <asm/local.h>
 
 struct rb_page {

@@ -212,8 +212,7 @@ static void ring_buffer_consumer(void)
 
 static void ring_buffer_producer(void)
 {
-	struct timeval start_tv;
-	struct timeval end_tv;
+	ktime_t start_time, end_time, timeout;
 	unsigned long long time;
 	unsigned long long entries;
 	unsigned long long overruns;

kernel/trace/ring_buffer_benchmark.c static void ring_buffer_producer(void) · line 231

Quoted 7 lines
kernel/trace/ring_buffer_benchmark.c +3 -1
@@ -227,7 +226,9 @@ static void ring_buffer_producer(void)
 	 * make the system stall)
 	 */
 	trace_printk("Starting ring buffer hammer\n");
-	do_gettimeofday(&start_tv);
+	start_time = ktime_get();
+	timeout = ktime_add_ns(start_time,
+			       ((long long) RUN_TIME) * NSEC_PER_SEC);

Instead of the (long long) typecast, could you define RUN_TIME as 10ULL.

Makes the code a bit cleaner, and we can put that on one line.

Rest looks fine.

-- Steve

Quoted 32 lines
kernel/trace/ring_buffer_benchmark.c +3 -5
 	do {
 		struct ring_buffer_event *event;
 		int *entry;

@@ -244,7 +245,7 @@ static void ring_buffer_producer(void)
 				ring_buffer_unlock_commit(buffer, event);
 			}
 		}
-		do_gettimeofday(&end_tv);
+		end_time = ktime_get();
 
 		cnt++;
 		if (consumer && !(cnt % wakeup_interval))

@@ -264,7 +265,7 @@ static void ring_buffer_producer(void)
 			cond_resched();
 #endif
 
-	} while (end_tv.tv_sec < (start_tv.tv_sec + RUN_TIME) && !kill_test);
+	} while (ktime_before(end_time, timeout) && !kill_test);
 	trace_printk("End ring buffer hammer\n");
 
 	if (consumer) {

@@ -280,9 +281,7 @@ static void ring_buffer_producer(void)
 		wait_for_completion(&read_done);
 	}
 
-	time = end_tv.tv_sec - start_tv.tv_sec;
-	time *= USEC_PER_SEC;
-	time += (long long)((long)end_tv.tv_usec - (long)start_tv.tv_usec);
+	time = ktime_us_delta(end_time, start_time);
 
 	entries = ring_buffer_entries(buffer);
 	overruns = ring_buffer_overruns(buffer);