trace-cmd: make records refcounted

If a callback wants to hold on to a record,
it can then increase the refcount and keep
it. This is most likely to happen with the
upcoming python plugin.

Since the underlying data pages are already
refcounted, we can just add a refcount to
struct record (rather than having the error
checking with the 'locked' member).

Acked-by: Darren Hart <dvhltc@us.ibm.com>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

5 files changed, 37 insertions, 20 deletions

diff --git a/parse-events.h b/parse-events.h
index 7d1c42e..7f79619 100644
--- a/parse-events.h
+++ b/parse-events.h
@@ -43,7 +43,7 @@ struct record {
         int                     size;           /* size of data */
         void                    *data;
         int                     cpu;
-        int                     locked;         /* Do not allow freeing */
+        int                     ref_count;
         void                    *private;
 };
 
diff --git a/trace-ftrace.c b/trace-ftrace.c
index af9ac8d..7cd5032 100644
--- a/trace-ftrace.c
+++ b/trace-ftrace.c
@@ -255,8 +255,13 @@ fgraph_ent_handler(struct trace_seq *s, struct record *record,
                 return trace_seq_putc(s, '!');
 
         rec = tracecmd_peek_data(tracecmd_curr_thread_handle, cpu);
-        if (rec)
-                rec = get_return_for_leaf(s, cpu, pid, val, rec);
+        if (rec) {
+                struct record *tmp;
+
+                tmp = get_return_for_leaf(s, cpu, pid, val, rec);
+                free_record(rec);
+                rec = tmp;
+        }
 
         if (rec) {
                 /*
diff --git a/trace-input.c b/trace-input.c
index c39004a..9db96c7 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -660,8 +660,13 @@ void free_record(struct record *record)
         if (!record)
                 return;
 
-        if (record->locked)
-                die("freeing record when it is locked!");
+        if (!record->ref_count)
+                die("record ref count is zero!");
+
+        record->ref_count--;
+        if (record->ref_count)
+                return;
+
         record->data = NULL;
 
         __free_record(record);
@@ -675,7 +680,8 @@ static void free_next(struct tracecmd_input *handle, int cpu)
                 return;
 
         handle->cpu_data[cpu].next = NULL;
-        __free_record(record);
+
+        free_record(record);
 }
 
 /*
@@ -870,6 +876,7 @@ read_old_format(struct tracecmd_input *handle, void **ptr, int cpu)
                 return NULL;
         memset(data, 0, sizeof(*data));
 
+        data->ref_count = 1;
         data->ts = handle->cpu_data[cpu].timestamp;
         data->size = length;
         data->data = *ptr;
@@ -902,11 +909,12 @@ peek_event(struct tracecmd_input *handle, unsigned long long offset,
         free_next(handle, cpu);
 
         do {
-                free_record(record);
                 record = tracecmd_peek_data(handle, cpu);
                 if (record && (record->offset + record->record_size) > offset)
                         break;
-                record = tracecmd_read_data(handle, cpu);
+                free_record(record);
+
+                free_next(handle, cpu);
         } while (record);
 
         return record;
@@ -919,8 +927,10 @@ read_event(struct tracecmd_input *handle, unsigned long long offset,
         struct record *record;
 
         record = peek_event(handle, offset, cpu);
-        if (record)
+        if (record) {
+                free_record(record);
                 record = tracecmd_read_data(handle, cpu);
+        }
         return record;
 }
 
@@ -1301,7 +1311,7 @@ int tracecmd_set_cursor(struct tracecmd_input *handle,
         if (get_page(handle, cpu, page_offset) < 0)
                 return -1;
 
-        peek_event(handle, offset, cpu);
+        free_record(peek_event(handle, offset, cpu));
 
         return 0;
 }
@@ -1429,6 +1439,7 @@ tracecmd_translate_data(struct tracecmd_input *handle,
                 return NULL;
         memset(record, 0, sizeof(*record));
 
+        record->ref_count = 1;
         record->data = ptr;
         type_len = translate_data(handle, &record->data, &record->ts, &record->size);
         switch (type_len) {
@@ -1451,8 +1462,6 @@ tracecmd_translate_data(struct tracecmd_input *handle,
  *
  * This returns the record at the current location of the CPU
  * iterator. It does not increment the CPU iterator.
- *
- * NOTE: Do not free the record returned, it is stored in the @handle.
  */
 struct record *
 tracecmd_peek_data(struct tracecmd_input *handle, int cpu)
@@ -1479,8 +1488,10 @@ tracecmd_peek_data(struct tracecmd_input *handle, int cpu)
                 if (!record->data)
                         die("Something freed the record");
 
-                if (handle->cpu_data[cpu].timestamp == record->ts)
+                if (handle->cpu_data[cpu].timestamp == record->ts) {
+                        record->ref_count++;
                         return record;
+                }
 
                 /*
                  * The timestamp changed, which means the cached
@@ -1553,6 +1564,7 @@ read_again:
         record->data = ptr;
         record->offset = handle->cpu_data[cpu].offset + index;
         record->missed_events = missed_events;
+        record->ref_count = 2; /* will be returned and stored in page */
 
         ptr += length;
 
@@ -1561,7 +1573,6 @@ read_again:
 
         record->record_size = handle->cpu_data[cpu].index - index;
         record->private = page;
-        record->locked = 1;
         page->ref_count++;
 
         return record;
@@ -1585,7 +1596,7 @@ tracecmd_read_data(struct tracecmd_input *handle, int cpu)
         record = tracecmd_peek_data(handle, cpu);
         handle->cpu_data[cpu].next = NULL;
         if (record)
-                record->locked = 0;
+                record->ref_count--;
 
         return record;
 }
@@ -1627,6 +1638,7 @@ tracecmd_read_next_data(struct tracecmd_input *handle, int *rec_cpu)
                         ts = record->ts;
                         next = cpu;
                 }
+                free_record(record);
         }
 
         if (next >= 0) {
@@ -1647,13 +1659,12 @@ tracecmd_read_next_data(struct tracecmd_input *handle, int *rec_cpu)
  * @record is the first record, NULL is returned. The cursor is set
  * as if the previous record was read by tracecmd_read_data().
  *
- * @record can not be NULL, otherwise NULL is returned.
+ * @record can not be NULL, otherwise NULL is returned; the
+ * record ownership goes to this function.
  *
  * Note, this is not that fast of an algorithm, since it needs
  * to build the timestamp for the record.
  *
- * Note 2: This may free any record allocoted with tracecmd_peek_data().
- *
  * The record returned must be freed with free_record().
  */
 struct record *
@@ -1674,7 +1685,7 @@ tracecmd_read_prev(struct tracecmd_input *handle, struct record *record)
         page_offset = calc_page_offset(handle, offset);
         index = offset - page_offset;
 
-        /* Note, the record passed in could have been a peek */
+        free_record(record);
         free_next(handle, cpu);
 
         /* Reset the cursor */
diff --git a/trace-plot-cpu.c b/trace-plot-cpu.c
index 50d720b..fdfbdf7 100644
--- a/trace-plot-cpu.c
+++ b/trace-plot-cpu.c
@@ -137,7 +137,7 @@ static int cpu_plot_display_last_event(struct graph_info *ginfo,
         record = tracecmd_peek_data(ginfo->handle, cpu);
         if (record)
                 offset = record->offset;
-        /* Don't need to free a peek */
+        free_record(record);
 
         tracecmd_set_cpu_to_timestamp(ginfo->handle, cpu, time);
 
diff --git a/trace-plot-task.c b/trace-plot-task.c
index 0c9f8e0..71fa731 100644
--- a/trace-plot-task.c
+++ b/trace-plot-task.c
@@ -183,6 +183,7 @@ static struct offset_cache *save_offsets(struct graph_info *ginfo)
                 record = tracecmd_peek_data(ginfo->handle, cpu);
                 if (record)
                         offsets->offsets[cpu] = record->offset;
+                free_record(record);
         }
 
         return offsets;