From 8ae601e8fedbbc9b1a1514796bb608e942660bd9 Mon Sep 17 00:00:00 2001 From: Harsh Prateek Bora Date: Fri, 20 Jul 2012 18:52:12 +0530 Subject: [PATCH 1/6] trace: rename TraceRecordHeader to TraceLogHeader The TraceRecordHeader is really the header for the entire trace log file. It's not per-record header so make this obvious by renaming it. Signed-off-by: Harsh Prateek Bora Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index b700ea317c..5d929399ca 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -70,7 +70,7 @@ typedef struct { uint64_t header_event_id; /* HEADER_EVENT_ID */ uint64_t header_magic; /* HEADER_MAGIC */ uint64_t header_version; /* HEADER_VERSION */ -} TraceRecordHeader; +} TraceLogHeader; static void read_from_buffer(unsigned int idx, void *dataptr, size_t size); @@ -295,7 +295,7 @@ void st_set_trace_file_enabled(bool enable) flush_trace_file(true); if (enable) { - static const TraceRecordHeader header = { + static const TraceLogHeader header = { .header_event_id = HEADER_EVENT_ID, .header_magic = HEADER_MAGIC, /* Older log readers will check for version at next location */ From 83d35d3e9909614e952021a92dbb39421977affc Mon Sep 17 00:00:00 2001 From: Harsh Prateek Bora Date: Fri, 20 Jul 2012 18:52:13 +0530 Subject: [PATCH 2/6] trace: remove unnecessary write_to_buffer() typecasting The buffer argument is void* so it is not necessary to cast. Signed-off-by: Harsh Prateek Bora Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index 5d929399ca..a0e0f051d7 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -235,9 +235,9 @@ int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasi rec->next_tbuf_idx = new_idx % TRACE_BUF_LEN; rec_off = idx; - rec_off = write_to_buffer(rec_off, (uint8_t*)&event, sizeof(event)); - rec_off = write_to_buffer(rec_off, (uint8_t*)×tamp_ns, sizeof(timestamp_ns)); - rec_off = write_to_buffer(rec_off, (uint8_t*)&rec_len, sizeof(rec_len)); + rec_off = write_to_buffer(rec_off, &event, sizeof(event)); + rec_off = write_to_buffer(rec_off, ×tamp_ns, sizeof(timestamp_ns)); + rec_off = write_to_buffer(rec_off, &rec_len, sizeof(rec_len)); rec->tbuf_idx = idx; rec->rec_off = (idx + sizeof(TraceRecord)) % TRACE_BUF_LEN; From fd82f015a1cc2df5972f06b75b350f65da68f9cf Mon Sep 17 00:00:00 2001 From: Harsh Prateek Bora Date: Fri, 20 Jul 2012 18:52:14 +0530 Subject: [PATCH 3/6] trace: drop unused TraceBufferRecord->next_tbuf_idx field Signed-off-by: Harsh Prateek Bora Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 2 -- trace/simple.h | 1 - 2 files changed, 3 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index a0e0f051d7..4fed07f330 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -231,8 +231,6 @@ int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasi } idx = old_idx % TRACE_BUF_LEN; - /* To check later if threshold crossed */ - rec->next_tbuf_idx = new_idx % TRACE_BUF_LEN; rec_off = idx; rec_off = write_to_buffer(rec_off, &event, sizeof(event)); diff --git a/trace/simple.h b/trace/simple.h index 7e521c1e1f..2ab96a8147 100644 --- a/trace/simple.h +++ b/trace/simple.h @@ -29,7 +29,6 @@ void st_flush_trace_buffer(void); typedef struct { unsigned int tbuf_idx; - unsigned int next_tbuf_idx; unsigned int rec_off; } TraceBufferRecord; From db8894f2090629a2e12b1e7324c4c5c53367e048 Mon Sep 17 00:00:00 2001 From: Harsh Prateek Bora Date: Fri, 20 Jul 2012 18:52:15 +0530 Subject: [PATCH 4/6] trace: avoid pointer aliasing in trace_record_finish() Declaring a TraceRecord on the stack works fine. No need for a uint8_t array and pointer aliasing. Signed-off-by: Harsh Prateek Bora Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index 4fed07f330..8e175ece73 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -269,12 +269,11 @@ static unsigned int write_to_buffer(unsigned int idx, void *dataptr, size_t size void trace_record_finish(TraceBufferRecord *rec) { - uint8_t temp_rec[sizeof(TraceRecord)]; - TraceRecord *record = (TraceRecord *) temp_rec; - read_from_buffer(rec->tbuf_idx, temp_rec, sizeof(TraceRecord)); + TraceRecord record; + read_from_buffer(rec->tbuf_idx, &record, sizeof(TraceRecord)); smp_wmb(); /* write barrier before marking as valid */ - record->event |= TRACE_RECORD_VALID; - write_to_buffer(rec->tbuf_idx, temp_rec, sizeof(TraceRecord)); + record.event |= TRACE_RECORD_VALID; + write_to_buffer(rec->tbuf_idx, &record, sizeof(TraceRecord)); if ((trace_idx - writeout_idx) > TRACE_BUF_FLUSH_THRESHOLD) { flush_trace_file(false); From 964d0a7b2bab935d48d3b2c4d6ab9b0efc74ce8b Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Mon, 13 Aug 2012 21:50:56 +0200 Subject: [PATCH 5/6] trace/simple: Fix compiler warning for 32 bit hosts gcc complains when a 32 bit pointer is casted to a 64 bit integer. Cc: Stefan Hajnoczi Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- scripts/tracetool/backend/simple.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py index c7e47d6d72..e4b4a7f05d 100644 --- a/scripts/tracetool/backend/simple.py +++ b/scripts/tracetool/backend/simple.py @@ -79,7 +79,7 @@ def c(events): ) # pointer var (not string) elif type_.endswith('*'): - out(' trace_record_write_u64(&rec, (uint64_t)(uint64_t *)%(name)s);', + out(' trace_record_write_u64(&rec, (uintptr_t)(uint64_t *)%(name)s);', name = name, ) # primitive data type From 4552e41025af4694c55854448c3ae4d95e72c7f6 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Mon, 13 Aug 2012 21:51:16 +0200 Subject: [PATCH 6/6] trace/simple: Replace asprintf by g_strdup_printf asprintf is not available for all hosts. g_strdup_printf is more portable and simplifies the code because if does not need error handling. The static variable does not need an explicit assignment to be NULL. Cc: Stefan Hajnoczi Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- trace/simple.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index 8e175ece73..d83681b227 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -55,7 +55,7 @@ static unsigned int trace_idx; static unsigned int writeout_idx; static uint64_t dropped_events; static FILE *trace_fp; -static char *trace_file_name = NULL; +static char *trace_file_name; /* * Trace buffer entry */ typedef struct { @@ -329,18 +329,12 @@ bool st_set_trace_file(const char *file) { st_set_trace_file_enabled(false); - free(trace_file_name); + g_free(trace_file_name); if (!file) { - if (asprintf(&trace_file_name, CONFIG_TRACE_FILE, getpid()) < 0) { - trace_file_name = NULL; - return false; - } + trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid()); } else { - if (asprintf(&trace_file_name, "%s", file) < 0) { - trace_file_name = NULL; - return false; - } + trace_file_name = g_strdup_printf("%s", file); } st_set_trace_file_enabled(true);