On Wed, 16 Aug 2023 21:32:52 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> Changes since v2: https://lore.kernel.org/all/20230811053940.1408424-1-rostedt@xxxxxxxxxxx/
The diff between this version and the last version:
diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index 315b66adb7ee..8e5a6451f399 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -67,11 +67,11 @@ union traceeval_data {
struct traceeval_type;
struct traceeval;
-/* struct traceeval_dynamic release function signature */
+/* release function callback on traceeval_data */
typedef void (*traceeval_data_release_fn)(const struct traceeval_type *type,
union traceeval_data *data);
-/* struct traceeval_dynamic compare function signature */
+/* compare function callback to compare traceeval_data */
typedef int (*traceeval_data_cmp_fn)(struct traceeval *teval,
const struct traceeval_type *type,
const union traceeval_data *A,
@@ -160,6 +160,9 @@ int traceeval_insert(struct traceeval *teval,
const union traceeval_data *keys,
const union traceeval_data *vals);
+int traceeval_remove(struct traceeval *teval,
+ const union traceeval_data *keys);
+
int traceeval_query(struct traceeval *teval, const union traceeval_data *keys,
const union traceeval_data **results);
diff --git a/samples/task-eval.c b/samples/task-eval.c
index e0ef2d0bcb30..66d0c40dc0c8 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -215,7 +215,9 @@ static void update_process(struct task_data *tdata, const char *comm,
return;
case STOP:
ret = traceeval_query(tdata->teval_processes.start, keys, &results);
- if (ret <= 0)
+ if (ret < 0)
+ pdie("Could not query start process");
+ if (ret == 0)
return;
if (!results[0].number_64)
break;
@@ -264,7 +266,9 @@ get_process_data(struct task_data *tdata, const char *comm)
int ret;
ret = traceeval_query(tdata->teval_processes_data, keys, &results);
- if (ret <= 0)
+ if (ret < 0)
+ pdie("Could not query process data");
+ if (ret == 0)
return NULL;
data = results[0].pointer;
@@ -289,6 +293,8 @@ void set_process_data(struct task_data *tdata, const char *comm, void *data)
ret = traceeval_query(tdata->teval_processes_data, keys, &results);
if (ret > 0)
goto out; /* It already exists ? */
+ if (ret < 0)
+ pdie("Could not query process data");
new_vals[0].pointer = data;
ret = traceeval_insert(tdata->teval_processes_data, keys, new_vals);
@@ -328,13 +334,17 @@ static void update_cpu(struct teval_pair *teval_pair, int cpu,
if (results[0].number_64)
break;
}
+ if (ret < 0)
+ pdie("Could not query cpu start data");
ret = traceeval_insert(teval_pair->start, keys, vals);
if (ret < 0)
pdie("Could not start CPU");
break;
case STOP:
ret = traceeval_query(teval_pair->start, keys, &results);
- if (ret <= 0)
+ if (ret < 0)
+ pdie("Could not query cpu stop data");
+ if (ret == 0)
return;
if (!results[0].number_64)
@@ -400,7 +410,9 @@ static void update_thread(struct process_data *pdata, int tid,
return;
case STOP:
ret = traceeval_query(pdata->teval_threads.start, keys, &results);
- if (ret <= 0)
+ if (ret < 0)
+ pdie("Could not query thread start");
+ if (ret == 0)
return;
new_vals[0].number_64 = ts - results[0].number_64;
@@ -825,6 +837,8 @@ static void display_processes(struct traceeval *teval,
const char *comm = keys[0].cstring;
ret = traceeval_query(teval_data, keys, &results);
+ if (ret < 0)
+ pdie("Could not query iterator");
if (ret < 1)
continue; /* ?? */
diff --git a/src/eval-local.h b/src/eval-local.h
index c3ea920ed2e8..5c3918f17cc1 100644
--- a/src/eval-local.h
+++ b/src/eval-local.h
@@ -6,7 +6,7 @@
#define __hidden __attribute__((visibility ("hidden")))
-#define offset_of(type, field) (&(((type *)(NULL))->field))
+#define offset_of(type, field) ((size_t)(&(((type *)(NULL))->field)))
#define container_of(ptr, type, field) \
(type *)((void *)(ptr) - (void *)offset_of(type, field))
diff --git a/src/hash.c b/src/hash.c
index 221145efcbb9..82962fbba8d8 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -78,7 +78,7 @@ __hidden struct hash_item *hash_iter_next(struct hash_iter *iter)
struct hash_table *hash = container_of(iter, struct hash_table, iter);
struct hash_item *item;
- if (iter->current_bucket > HASH_SIZE(hash->bits))
+ if (iter->current_bucket >= HASH_SIZE(hash->bits))
return NULL;
item = iter->next_item;
diff --git a/src/histograms.c b/src/histograms.c
index cd68d4c834af..f35d1b2e583d 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -12,7 +12,6 @@
#include <stdio.h>
#include <traceeval-hist.h>
-
#include "eval-local.h"
/*
@@ -37,9 +36,9 @@ static void print_err(const char *fmt, ...)
* -1 for the other way around, and -2 on error.
*/
static int compare_traceeval_data(struct traceeval *teval,
+ struct traceeval_type *type,
const union traceeval_data *orig,
- const union traceeval_data *copy,
- struct traceeval_type *type)
+ const union traceeval_data *copy)
{
int i;
@@ -92,16 +91,16 @@ static int compare_traceeval_data(struct traceeval *teval,
* Return 1 if @orig and @copy are the same, 0 if not, and -1 on error.
*/
static int compare_traceeval_data_set(struct traceeval *teval,
+ struct traceeval_type *defs,
union traceeval_data *orig,
- const union traceeval_data *copy,
- struct traceeval_type *defs, size_t size)
+ const union traceeval_data *copy, size_t size)
{
int check;
size_t i;
/* compare data arrays */
for (i = 0; i < size; i++) {
- if ((check = compare_traceeval_data(teval, orig + i, copy + i, defs + i)))
+ if ((check = compare_traceeval_data(teval, defs + i, orig + i, copy + i)))
return check == -2 ? -1 : 0;
}
@@ -294,12 +293,12 @@ struct traceeval *traceeval_init(struct traceeval_type *keys,
ret = check_keys(keys);
if (ret < 0)
- goto fail;
+ goto fail_release;
if (vals) {
ret = check_vals(vals);
if (ret < 0)
- goto fail;
+ goto fail_release;
}
/* alloc key types */
@@ -463,7 +462,7 @@ static unsigned make_hash(struct traceeval *teval, const union traceeval_data *k
key += hash_number(val);
}
- return key & HASH_MASK(bits);
+ return key;
}
/*
@@ -489,8 +488,8 @@ static int get_entry(struct traceeval *teval, const union traceeval_data *keys,
for (iter = hash_iter_bucket(hist, key); (item = hash_iter_bucket_next(iter)); ) {
entry = container_of(item, struct entry, hash);
- check = compare_traceeval_data_set(teval, entry->keys, keys,
- teval->key_types, teval->nr_key_types);
+ check = compare_traceeval_data_set(teval, teval->key_types,
+ entry->keys, keys, teval->nr_key_types);
if (check)
break;
}
@@ -596,7 +595,7 @@ static int copy_traceeval_data(struct traceeval_type *type,
/*
* Free @data with respect to @size and @type.
*
- * Does not call release() if a copy() exists.
+ * Does not call the release() callback if a copy() exists.
*/
static void data_release(size_t size, union traceeval_data *data,
struct traceeval_type *type)
@@ -776,7 +775,7 @@ fail_entry:
*
* Frees the old vals field of @entry, unless an error occurs.
*
- * Return 1 on success, -1 on error.
+ * Return 0 on success, -1 on error.
*/
static int update_entry(struct traceeval *teval, struct entry *entry,
const union traceeval_data *vals)
@@ -789,7 +788,7 @@ static int update_entry(struct traceeval *teval, struct entry *entry,
size_t i;
if (!size)
- return 1;
+ return 0;
for (i = 0; i < size; i++) {
old[i] = copy[i];
@@ -798,11 +797,16 @@ static int update_entry(struct traceeval *teval, struct entry *entry,
vals + i, copy + i))
goto fail;
}
-
+ data_release(size, old, types);
return 0;
-
-fail:
- data_release(i, old, types);
+ fail:
+ /* Free the new values that were added */
+ data_release(i, copy, types);
+ /* Put back the old values */
+ for (i--; i >= 0; i--) {
+ copy_traceeval_data(types + i, NULL,
+ copy + i, old + i);
+ }
return -1;
}
@@ -930,6 +934,35 @@ int traceeval_insert(struct traceeval *teval,
return update_entry(teval, entry, vals);
}
+/**
+ * traceeval_remove - remove an item from the traceeval descriptor
+ * @teval: The descriptor to insert into
+ * @keys: The list of keys that defines what is being removed
+ *
+ * This is the opposite of traceeval_insert(). Instead of inserting
+ * an item into the traceeval historgram, it removes it.
+ *
+ * Returns 1 if it found and removed an item,
+ * 0 if it did not find an time matching @keys
+ * -1 if there was an error.
+ */
+int traceeval_remove(struct traceeval *teval,
+ const union traceeval_data *keys)
+{
+ struct hash_table *hist = teval->hist;
+ struct entry *entry;
+ int check;
+
+ entry = NULL;
+ check = get_entry(teval, keys, &entry);
+
+ if (check < 1)
+ return check;
+
+ hash_remove(hist, &entry->hash);
+ return 1;
+}
+
/**
* traceeval_iterator_put - release a given iterator
* @iter: The iterartor to release
@@ -942,7 +975,9 @@ void traceeval_iterator_put(struct traceeval_iterator *iter)
if (!iter)
return;
+ free(iter->direction);
free(iter->entries);
+ free(iter->sort);
free(iter);
}
@@ -1058,6 +1093,7 @@ int traceeval_iterator_sort(struct traceeval_iterator *iter, const char *sort_fi
bool *direction = iter->direction;
struct traceeval_type **sort = iter->sort;
struct traceeval_type *type;
+ int num_levels = level + 1;
type = find_sort_type(iter->teval, sort_field);
if (!type)
@@ -1074,21 +1110,21 @@ int traceeval_iterator_sort(struct traceeval_iterator *iter, const char *sort_fi
break;
}
- if ((level + 1) > iter->nr_sort) {
- sort = realloc(sort, sizeof(*sort) * (level + 1));
+ if (num_levels > iter->nr_sort) {
+ sort = realloc(sort, sizeof(*sort) * num_levels);
if (!sort)
return -1;
iter->sort = sort;
- direction = realloc(direction, sizeof(*direction) * (level + 1));
+ direction = realloc(direction, sizeof(*direction) * num_levels);
if (!direction)
return -1;
iter->direction = direction;
/* Make sure the newly allocated contain NULL */
- for (int i = iter->nr_sort; i < (level + 1); i++)
+ for (int i = iter->nr_sort; i < num_levels; i++)
sort[i] = NULL;
iter->nr_sort = level + 1;
@@ -1123,7 +1159,7 @@ static int iter_cmp(const void *A, const void *B, void *data)
dataB = &b->vals[type->index];
}
- ret = compare_traceeval_data(teval, dataA, dataB, type);
+ ret = compare_traceeval_data(teval, type, dataA, dataB);
if (ret)
return iter->direction[i] ? ret : ret * -1;
@@ -1182,6 +1218,7 @@ int traceeval_iterator_sort_custom(struct traceeval_iterator *iter,
iter_custom_cmp, &cust_data);
iter->needs_sort = false;
+ iter->next = 0;
return 0;
}
-- Steve
|