This is an attempt to bring support for modified gcov format in gcc 4.7 to
the kernel. It tries to leverage the existing layout/abstraction, which was
designed keeping in mind that the gcov format could change, but some changes had
to be make. Mostly because the current model does not take into account that
even the core gcov structures, like gcov_info, could change. One part that could
be problematic is the addition of the .init_array section for constructors.
Tested with lcov and seems to be working fine, giving similar results as for the
older format.
Frantisek Hrbata (4):
gcov: move gcov structs definitions to a gcc version specific file
gcov: add support for gcc 4.7 gcov format
gcov: compile specific gcov implementation based on gcc version
kernel: add support for init_array constructors
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/module.h | 2 +
kernel/gcov/Makefile | 5 +-
kernel/gcov/base.c | 32 +-
kernel/gcov/fs.c | 27 +-
kernel/gcov/gcc_3_4.c | 115 +++++++
kernel/gcov/gcc_4_7.c | 612 ++++++++++++++++++++++++++++++++++++++
kernel/gcov/gcov.h | 65 +---
kernel/module.c | 6 +
9 files changed, 784 insertions(+), 81 deletions(-)
create mode 100644 kernel/gcov/gcc_4_7.c
--
1.8.3.1
Since also the gcov structures(gcov_info, gcov_fn_info, gcov_ctr_info) can
change between gcc releases, as shown in gcc 4.7, they cannot be defined in a
common header and need to be moved to a specific gcc implemention file. This
also requires to make the gcov_info structure opaque for the common code and to
introduce simple helpers for accessing data inside gcov_info.
Signed-off-by: Frantisek Hrbata <[email protected]>
---
kernel/gcov/base.c | 26 ++++++------
kernel/gcov/fs.c | 27 ++++++------
kernel/gcov/gcc_3_4.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++
kernel/gcov/gcov.h | 65 +++++-----------------------
4 files changed, 153 insertions(+), 80 deletions(-)
diff --git a/kernel/gcov/base.c b/kernel/gcov/base.c
index 9b22d03..912576a 100644
--- a/kernel/gcov/base.c
+++ b/kernel/gcov/base.c
@@ -20,7 +20,6 @@
#include <linux/mutex.h>
#include "gcov.h"
-static struct gcov_info *gcov_info_head;
static int gcov_events_enabled;
static DEFINE_MUTEX(gcov_lock);
@@ -34,7 +33,7 @@ void __gcov_init(struct gcov_info *info)
mutex_lock(&gcov_lock);
if (gcov_version == 0) {
- gcov_version = info->version;
+ gcov_version = gcov_info_version(info);
/*
* Printing gcc's version magic may prove useful for debugging
* incompatibility reports.
@@ -45,8 +44,7 @@ void __gcov_init(struct gcov_info *info)
* Add new profiling data structure to list and inform event
* listener.
*/
- info->next = gcov_info_head;
- gcov_info_head = info;
+ gcov_info_link(info);
if (gcov_events_enabled)
gcov_event(GCOV_ADD, info);
mutex_unlock(&gcov_lock);
@@ -91,13 +89,15 @@ EXPORT_SYMBOL(__gcov_merge_delta);
*/
void gcov_enable_events(void)
{
- struct gcov_info *info;
+ struct gcov_info *info = NULL;
mutex_lock(&gcov_lock);
gcov_events_enabled = 1;
+
/* Perform event callback for previously registered entries. */
- for (info = gcov_info_head; info; info = info->next)
+ while ((info = gcov_info_next(info)))
gcov_event(GCOV_ADD, info);
+
mutex_unlock(&gcov_lock);
}
@@ -112,25 +112,23 @@ static int gcov_module_notifier(struct notifier_block *nb, unsigned long event,
void *data)
{
struct module *mod = data;
- struct gcov_info *info;
- struct gcov_info *prev;
+ struct gcov_info *info = NULL;
+ struct gcov_info *prev = NULL;
if (event != MODULE_STATE_GOING)
return NOTIFY_OK;
mutex_lock(&gcov_lock);
- prev = NULL;
+
/* Remove entries located in module from linked list. */
- for (info = gcov_info_head; info; info = info->next) {
+ while ((info = gcov_info_next(info))) {
if (within(info, mod->module_core, mod->core_size)) {
- if (prev)
- prev->next = info->next;
- else
- gcov_info_head = info->next;
+ gcov_info_unlink(prev, info);
if (gcov_events_enabled)
gcov_event(GCOV_REMOVE, info);
} else
prev = info;
}
+
mutex_unlock(&gcov_lock);
return NOTIFY_OK;
diff --git a/kernel/gcov/fs.c b/kernel/gcov/fs.c
index 9bd0934..27e12ce 100644
--- a/kernel/gcov/fs.c
+++ b/kernel/gcov/fs.c
@@ -242,7 +242,7 @@ static struct gcov_node *get_node_by_name(const char *name)
list_for_each_entry(node, &all_head, all) {
info = get_node_info(node);
- if (info && (strcmp(info->filename, name) == 0))
+ if (info && (strcmp(gcov_info_filename(info), name) == 0))
return node;
}
@@ -279,7 +279,7 @@ static ssize_t gcov_seq_write(struct file *file, const char __user *addr,
seq = file->private_data;
info = gcov_iter_get_info(seq->private);
mutex_lock(&node_lock);
- node = get_node_by_name(info->filename);
+ node = get_node_by_name(gcov_info_filename(info));
if (node) {
/* Reset counts or remove node for unloaded modules. */
if (node->num_loaded == 0)
@@ -376,8 +376,9 @@ static void add_links(struct gcov_node *node, struct dentry *parent)
if (!node->links)
return;
for (i = 0; i < num; i++) {
- target = get_link_target(get_node_info(node)->filename,
- &gcov_link[i]);
+ target = get_link_target(
+ gcov_info_filename(get_node_info(node)),
+ &gcov_link[i]);
if (!target)
goto out_err;
basename = strrchr(target, '/');
@@ -576,7 +577,7 @@ static void add_node(struct gcov_info *info)
struct gcov_node *parent;
struct gcov_node *node;
- filename = kstrdup(info->filename, GFP_KERNEL);
+ filename = kstrdup(gcov_info_filename(info), GFP_KERNEL);
if (!filename)
return;
parent = &root_node;
@@ -631,7 +632,7 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
loaded_info = kcalloc(num + 1, sizeof(struct gcov_info *), GFP_KERNEL);
if (!loaded_info) {
pr_warning("could not add '%s' (out of memory)\n",
- info->filename);
+ gcov_info_filename(info));
return;
}
memcpy(loaded_info, node->loaded_info,
@@ -645,7 +646,8 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
*/
if (!gcov_info_is_compatible(node->unloaded_info, info)) {
pr_warning("discarding saved data for %s "
- "(incompatible version)\n", info->filename);
+ "(incompatible version)\n",
+ gcov_info_filename(info));
gcov_info_free(node->unloaded_info);
node->unloaded_info = NULL;
}
@@ -656,7 +658,7 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
*/
if (!gcov_info_is_compatible(node->loaded_info[0], info)) {
pr_warning("could not add '%s' (incompatible "
- "version)\n", info->filename);
+ "version)\n", gcov_info_filename(info));
kfree(loaded_info);
return;
}
@@ -692,7 +694,8 @@ static void save_info(struct gcov_node *node, struct gcov_info *info)
node->unloaded_info = gcov_info_dup(info);
if (!node->unloaded_info) {
pr_warning("could not save data for '%s' "
- "(out of memory)\n", info->filename);
+ "(out of memory)\n",
+ gcov_info_filename(info));
}
}
}
@@ -708,7 +711,7 @@ static void remove_info(struct gcov_node *node, struct gcov_info *info)
i = get_info_index(node, info);
if (i < 0) {
pr_warning("could not remove '%s' (not found)\n",
- info->filename);
+ gcov_info_filename(info));
return;
}
if (gcov_persist)
@@ -735,7 +738,7 @@ void gcov_event(enum gcov_action action, struct gcov_info *info)
struct gcov_node *node;
mutex_lock(&node_lock);
- node = get_node_by_name(info->filename);
+ node = get_node_by_name(gcov_info_filename(info));
switch (action) {
case GCOV_ADD:
if (node)
@@ -748,7 +751,7 @@ void gcov_event(enum gcov_action action, struct gcov_info *info)
remove_info(node, info);
else {
pr_warning("could not remove '%s' (not found)\n",
- info->filename);
+ gcov_info_filename(info));
}
break;
}
diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
index ae5bb42..27bc88a 100644
--- a/kernel/gcov/gcc_3_4.c
+++ b/kernel/gcov/gcc_3_4.c
@@ -21,6 +21,121 @@
#include <linux/vmalloc.h>
#include "gcov.h"
+#define GCOV_COUNTERS 5
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @checksum: function checksum
+ * @n_ctrs: number of values per counter type belonging to this function
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ */
+struct gcov_fn_info {
+ unsigned int ident;
+ unsigned int checksum;
+ unsigned int n_ctrs[0];
+};
+
+/**
+ * struct gcov_ctr_info - profiling data per counter type
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ * @merge: merge function for counter values of this type (unused)
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info {
+ unsigned int num;
+ gcov_type *values;
+ void (*merge)(gcov_type *, unsigned int);
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: time stamp
+ * @filename: name of the associated gcov data file
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ * @ctr_mask: mask specifying which counter types are active
+ * @counts: counter data per counter type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info {
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ unsigned int n_functions;
+ const struct gcov_fn_info *functions;
+ unsigned int ctr_mask;
+ struct gcov_ctr_info counts[0];
+};
+
+/**
+ * gcov_info_filename - return info filename
+ * @info: profiling data set
+ */
+const char *gcov_info_filename(struct gcov_info *info)
+{
+ return info->filename;
+}
+
+/**
+ * gcov_info_version - return info version
+ * @info: profiling data set
+ */
+unsigned int gcov_info_version(struct gcov_info *info)
+{
+ return info->version;
+}
+
+/**
+ * gcov_info_next - return next profiling data set
+ * @info: profiling data set
+ *
+ * Returns next gcov_info following @info or first gcov_info in the chain if
+ * @info is %NULL.
+ */
+struct gcov_info *gcov_info_next(struct gcov_info *info)
+{
+ if (!info)
+ return gcov_info_head;
+
+ return info->next;
+}
+
+/**
+ * gcov_info_link - link/add profiling data set to the list
+ * @info: profiling data set
+ */
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+/**
+ * gcov_info_unlink - unlink/remove profiling data set from the list
+ * @prev: previous profiling data set
+ * @info: profiling data set
+ */
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
+{
+ if (prev)
+ prev->next = info->next;
+ else
+ gcov_info_head = info->next;
+}
+
/* Symbolic links to be created for each profiling data file. */
const struct gcov_link gcov_link[] = {
{ OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
diff --git a/kernel/gcov/gcov.h b/kernel/gcov/gcov.h
index 060073e..92c8e22 100644
--- a/kernel/gcov/gcov.h
+++ b/kernel/gcov/gcov.h
@@ -21,7 +21,6 @@
* gcc and need to be kept as close to the original definition as possible to
* remain compatible.
*/
-#define GCOV_COUNTERS 5
#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
@@ -34,60 +33,18 @@ typedef long gcov_type;
typedef long long gcov_type;
#endif
-/**
- * struct gcov_fn_info - profiling meta data per function
- * @ident: object file-unique function identifier
- * @checksum: function checksum
- * @n_ctrs: number of values per counter type belonging to this function
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time.
- */
-struct gcov_fn_info {
- unsigned int ident;
- unsigned int checksum;
- unsigned int n_ctrs[0];
-};
-
-/**
- * struct gcov_ctr_info - profiling data per counter type
- * @num: number of counter values for this type
- * @values: array of counter values for this type
- * @merge: merge function for counter values of this type (unused)
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the values array.
- */
-struct gcov_ctr_info {
- unsigned int num;
- gcov_type *values;
- void (*merge)(gcov_type *, unsigned int);
-};
+/* Opaque gcov_info. The gcov structures can change as for example in gcc 4.7 so
+ * we cannot use full definition here and they need to be placed in gcc specific
+ * implementation of gcov. This also means no direct access to the members in
+ * generic code and usage of the interface below.*/
+struct gcov_info;
-/**
- * struct gcov_info - profiling data per object file
- * @version: gcov version magic indicating the gcc version used for compilation
- * @next: list head for a singly-linked list
- * @stamp: time stamp
- * @filename: name of the associated gcov data file
- * @n_functions: number of instrumented functions
- * @functions: function data
- * @ctr_mask: mask specifying which counter types are active
- * @counts: counter data per counter type
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the next pointer.
- */
-struct gcov_info {
- unsigned int version;
- struct gcov_info *next;
- unsigned int stamp;
- const char *filename;
- unsigned int n_functions;
- const struct gcov_fn_info *functions;
- unsigned int ctr_mask;
- struct gcov_ctr_info counts[0];
-};
+/* Interface to access gcov_info data */
+const char *gcov_info_filename(struct gcov_info *info);
+unsigned int gcov_info_version(struct gcov_info *info);
+struct gcov_info *gcov_info_next(struct gcov_info *info);
+void gcov_info_link(struct gcov_info *info);
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info);
/* Base interface. */
enum gcov_action {
--
1.8.3.1
Compile the correct gcov implementation file for a specific gcc version. In
the future, if another file is added, the conditions will need to be somehow
adjusted to if-elif-else case, but at this point the simple cc-ifversion should
be enough.
Signed-off-by: Frantisek Hrbata <[email protected]>
---
kernel/gcov/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index e97ca59..d57b712 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -1,3 +1,6 @@
ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
-obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
+obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
+
+obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
+obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
--
1.8.3.1
The gcov in-memory format changed in gcc 4.7. The biggest change, which
requires this special implementation, is that gcov_info no longer contains
array of counters for each counter type for all functions and gcov_fn_info is
not used for mapping of function's counters to these arrays(offset). Now each
gcov_fn_info contans it's counters, which makes things a little bit easier.
This is heavily based on the previous gcc_3_4.c implementation.
Signed-off-by: Frantisek Hrbata <[email protected]>
---
kernel/gcov/base.c | 6 +
kernel/gcov/gcc_4_7.c | 612 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 618 insertions(+)
create mode 100644 kernel/gcov/gcc_4_7.c
diff --git a/kernel/gcov/base.c b/kernel/gcov/base.c
index 912576a..f45b75b 100644
--- a/kernel/gcov/base.c
+++ b/kernel/gcov/base.c
@@ -79,6 +79,12 @@ void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
}
EXPORT_SYMBOL(__gcov_merge_delta);
+void __gcov_merge_ior(gcov_type *counters, unsigned int n_counters)
+{
+ /* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_ior);
+
/**
* gcov_enable_events - enable event reporting through gcov_event()
*
diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
new file mode 100644
index 0000000..31bee00
--- /dev/null
+++ b/kernel/gcov/gcc_4_7.c
@@ -0,0 +1,612 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 4.7.
+ *
+ * This file is based heavily on gcc_3_4.c file.
+ *
+ * For a better understanding, refer to gcc source:
+ * gcc/gcov-io.h
+ * libgcc/libgcov.c
+ *
+ * Uses gcc-internal data definitions.
+ */
+
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/seq_file.h>
+#include "gcov.h"
+
+#define GCOV_COUNTERS 8
+#define GCOV_TAG_FUNCTION_LENGTH 3
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_ctr_info - information about counters for a single function
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info {
+ unsigned int num;
+ gcov_type *values;
+};
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @key: comdat key
+ * @ident: unique ident of function
+ * @lineno_checksum: function lineo_checksum
+ * @cfg_checksum: function cfg checksum
+ * @ctrs: instrumented counters
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ *
+ * Information about a single function. This uses the trailing array
+ * idiom. The number of counters is determined from the merge pointer
+ * array in gcov_info. The key is used to detect which of a set of
+ * comdat functions was selected -- it points to the gcov_info object
+ * of the object file containing the selected comdat function.
+ */
+struct gcov_fn_info {
+ const struct gcov_info *key;
+ unsigned int ident;
+ unsigned int lineno_checksum;
+ unsigned int cfg_checksum;
+ struct gcov_ctr_info ctrs[0];
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: uniquifying time stamp
+ * @filename: name of the associated gcov data file
+ * @merge: merge functions (null for unused counter type)
+ * @n_functions: number of instrumented functions
+ * @functions: pointer to pointers to function information
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info {
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
+ unsigned int n_functions;
+ const struct gcov_fn_info *const *functions;
+};
+
+/**
+ * gcov_info_filename - return info filename
+ * @info: profiling data set
+ */
+const char *gcov_info_filename(struct gcov_info *info)
+{
+ return info->filename;
+}
+
+/**
+ * gcov_info_version - return info version
+ * @info: profiling data set
+ */
+unsigned int gcov_info_version(struct gcov_info *info)
+{
+ return info->version;
+}
+
+/**
+ * gcov_info_next - return next profiling data set
+ * @info: profiling data set
+ *
+ * Returns next gcov_info following @info or first gcov_info in the chain if
+ * @info is %NULL.
+ */
+struct gcov_info *gcov_info_next(struct gcov_info *info)
+{
+ if (!info)
+ return gcov_info_head;
+
+ return info->next;
+}
+
+/**
+ * gcov_info_link - link/add profiling data set to the list
+ * @info: profiling data set
+ */
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+/**
+ * gcov_info_unlink - unlink/remove profiling data set from the list
+ * @prev: previous profiling data set
+ * @info: profiling data set
+ */
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
+{
+ if (prev)
+ prev->next = info->next;
+ else
+ gcov_info_head = info->next;
+}
+
+/* Symbolic links to be created for each profiling data file. */
+const struct gcov_link gcov_link[] = {
+ { OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
+ { 0, NULL},
+};
+
+/*
+ * Determine whether a counter is active. Doesn't change at run-time.
+ */
+static int counter_active(struct gcov_info *info, unsigned int type)
+{
+ return info->merge[type] ? 1 : 0;
+}
+
+/* Determine number of active counters. Based on gcc magic. */
+static unsigned int num_counter_active(struct gcov_info *info)
+{
+ unsigned int i;
+ unsigned int result = 0;
+
+ for (i = 0; i < GCOV_COUNTERS; i++) {
+ if (counter_active(info, i))
+ result++;
+ }
+ return result;
+}
+
+/**
+ * gcov_info_reset - reset profiling data to zero
+ * @info: profiling data set
+ */
+void gcov_info_reset(struct gcov_info *info)
+{
+ const struct gcov_ctr_info *ci_ptr;
+ unsigned int fi_idx;
+ unsigned int t_idx;
+
+ for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++)
+ {
+ ci_ptr = info->functions[fi_idx]->ctrs;
+
+ for (t_idx = 0; t_idx < GCOV_COUNTERS; t_idx++) {
+ if (!counter_active(info, t_idx))
+ continue;
+
+ memset(ci_ptr->values, 0,
+ sizeof(gcov_type) * ci_ptr->num);
+ ci_ptr++;
+ }
+ }
+}
+
+/**
+ * gcov_info_is_compatible - check if profiling data can be added
+ * @info1: first profiling data set
+ * @info2: second profiling data set
+ *
+ * Returns non-zero if profiling data can be added, zero otherwise.
+ */
+int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
+{
+ return (info1->stamp == info2->stamp);
+}
+
+/**
+ * gcov_info_add - add up profiling data
+ * @dest: profiling data set to which data is added
+ * @source: profiling data set which is added
+ *
+ * Adds profiling counts of @source to @dest.
+ */
+void gcov_info_add(struct gcov_info *dst, struct gcov_info *src)
+{
+ const struct gcov_ctr_info *dci_ptr;
+ const struct gcov_ctr_info *sci_ptr;
+ unsigned int fi_idx;
+ unsigned int t_idx;
+ unsigned int val_idx;
+
+ for (fi_idx = 0; fi_idx < src->n_functions; fi_idx++)
+ {
+ dci_ptr = dst->functions[fi_idx]->ctrs;
+ sci_ptr = src->functions[fi_idx]->ctrs;
+
+ for (t_idx = 0; t_idx < GCOV_COUNTERS; t_idx++) {
+ if (!counter_active(src, t_idx))
+ continue;
+
+ for (val_idx = 0; val_idx < sci_ptr->num; val_idx++)
+ dci_ptr->values[val_idx] +=
+ sci_ptr->values[val_idx];
+
+ dci_ptr++;
+ sci_ptr++;
+ }
+ }
+}
+
+/**
+ * gcov_info_dup - duplicate profiling data set
+ * @info: profiling data set to duplicate
+ *
+ * Return newly allocated duplicate on success, %NULL on error.
+ */
+struct gcov_info *gcov_info_dup(struct gcov_info *info)
+{
+ struct gcov_info *dup;
+ struct gcov_ctr_info *dci_ptr;
+ struct gcov_ctr_info *sci_ptr;
+ size_t size;
+ unsigned int active;
+ unsigned int fi_idx;
+ unsigned int t_idx;
+
+ dup = kmalloc(sizeof(struct gcov_info), GFP_KERNEL);
+ if (!dup)
+ return NULL;
+
+ *dup = *info;
+ dup->next = NULL;
+ dup->filename = NULL;
+ dup->functions = NULL;
+
+ dup->filename = kstrdup(info->filename, GFP_KERNEL);
+ if (!dup->filename)
+ goto err_free;
+
+ size = sizeof(void *) * info->n_functions;
+ dup->functions = kzalloc(size, GFP_KERNEL);
+ if (!dup->functions)
+ goto err_free;
+
+ active = num_counter_active(info);
+ size = sizeof(struct gcov_fn_info);
+ size += sizeof(struct gcov_ctr_info) * active;
+
+ for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
+ *(struct gcov_fn_info **)&dup->functions[fi_idx] =
+ kzalloc(size, GFP_KERNEL);
+ if (!dup->functions[fi_idx])
+ goto err_free;
+
+ *(struct gcov_fn_info *)&(*(dup->functions[fi_idx])) =
+ *(info->functions[fi_idx]);
+
+ sci_ptr = (struct gcov_ctr_info *)info->functions[fi_idx]->ctrs;
+ dci_ptr = (struct gcov_ctr_info *)dup->functions[fi_idx]->ctrs;
+
+ for (t_idx = 0; t_idx < GCOV_COUNTERS; t_idx++) {
+ if (!counter_active(info, t_idx))
+ continue;
+
+ dci_ptr->values = kmemdup(sci_ptr->values,
+ sizeof(gcov_type) * sci_ptr->num, GFP_KERNEL);
+
+ if (!dci_ptr->values)
+ goto err_free;
+
+ dci_ptr->num = sci_ptr->num;
+
+ sci_ptr++;
+ dci_ptr++;
+ }
+ }
+
+ return dup;
+err_free:
+ gcov_info_free(dup);
+ return NULL;
+}
+
+/**
+ * gcov_info_free - release memory for profiling data set duplicate
+ * @info: profiling data set duplicate to free
+ */
+void gcov_info_free(struct gcov_info *info)
+{
+ unsigned int active;
+ unsigned int fi_idx;
+ unsigned int t_idx;
+ const struct gcov_ctr_info *ci_ptr;
+
+ if (!info->functions)
+ goto free_info;
+
+ active = num_counter_active(info);
+
+ for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
+ if (!info->functions[fi_idx])
+ continue;
+
+ ci_ptr = info->functions[fi_idx]->ctrs;
+
+ for (t_idx = 0; t_idx < GCOV_COUNTERS; t_idx++) {
+ if (!counter_active(info, t_idx))
+ continue;
+
+ if (!ci_ptr->values)
+ continue;
+
+ kfree(ci_ptr->values);
+
+ ci_ptr++;
+ }
+
+ kfree(info->functions[fi_idx]);
+ }
+
+free_info:
+ kfree(info->functions);
+ kfree(info->filename);
+ kfree(info);
+}
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @info: associated profiling data
+ * @record: record type
+ * @function: function number
+ * @type: counter type
+ * @count: index into values array
+ * @num_types: number of counter types
+ * @type_info: helper array to get current counter type
+ */
+struct gcov_iterator {
+ struct gcov_info *info;
+ int record;
+ unsigned int function;
+ unsigned int type;
+ unsigned int count;
+ int num_types;
+ int type_info[0];
+};
+
+static const struct gcov_fn_info *get_func(struct gcov_iterator *iter)
+{
+ return iter->info->functions[iter->function];
+}
+
+static const struct gcov_ctr_info *get_ctr(struct gcov_iterator *iter)
+{
+ return &get_func(iter)->ctrs[iter->type];
+}
+
+static gcov_type get_cnt_type(struct gcov_iterator *iter)
+{
+ return iter->type_info[iter->type];
+}
+
+static gcov_type get_cnt_val(struct gcov_iterator *iter)
+{
+ return get_ctr(iter)->values[iter->count];
+}
+
+/**
+ * gcov_iter_new - allocate and initialize profiling data iterator
+ * @info: profiling data set to be iterated
+ *
+ * Return file iterator on success, %NULL otherwise.
+ */
+struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
+{
+ struct gcov_iterator *iter;
+
+ iter = kzalloc(sizeof(struct gcov_iterator) +
+ num_counter_active(info) * sizeof(int), GFP_KERNEL);
+ if (iter)
+ iter->info = info;
+
+ return iter;
+}
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+void gcov_iter_free(struct gcov_iterator *iter)
+{
+ kfree(iter);
+}
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
+{
+ return iter->info;
+}
+
+/**
+ * gcov_iter_start - reset file iterator to starting position
+ * @iter: file iterator
+ */
+void gcov_iter_start(struct gcov_iterator *iter)
+{
+ int i;
+
+ iter->record = 0;
+ iter->function = 0;
+ iter->type = 0;
+ iter->count = 0;
+ iter->num_types = 0;
+
+ for (i = 0; i < GCOV_COUNTERS; i++) {
+ if (counter_active(iter->info, i))
+ iter->type_info[iter->num_types++] = i;
+ }
+}
+
+/* Mapping of logical record number to actual file content. */
+#define RECORD_FILE_MAGIC 0
+#define RECORD_GCOV_VERSION 1
+#define RECORD_TIME_STAMP 2
+#define RECORD_FUNCTION_TAG 3
+#define RECORD_FUNCTION_TAG_LEN 4
+#define RECORD_FUNCTION_IDENT 5
+#define RECORD_FUNCTION_LINENO_CHECK 6
+#define RECORD_FUNCTION_CFG_CHECK 7
+#define RECORD_COUNT_TAG 8
+#define RECORD_COUNT_LEN 9
+#define RECORD_COUNT 10
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+int gcov_iter_next(struct gcov_iterator *iter)
+{
+ switch (iter->record) {
+ case RECORD_FILE_MAGIC:
+ case RECORD_GCOV_VERSION:
+ case RECORD_FUNCTION_TAG:
+ case RECORD_FUNCTION_TAG_LEN:
+ case RECORD_FUNCTION_IDENT:
+ case RECORD_FUNCTION_LINENO_CHECK:
+ case RECORD_COUNT_TAG:
+ /* Advance to next record */
+ iter->record++;
+ break;
+
+ case RECORD_COUNT:
+ iter->count++;
+ /* fall through */
+ case RECORD_COUNT_LEN:
+ if (iter->count < get_ctr(iter)->num) {
+ iter->record = RECORD_COUNT;
+ break;
+ }
+ iter->count = 0;
+ iter->type++;
+ /* fall through */
+ case RECORD_FUNCTION_CFG_CHECK:
+ if (iter->type < iter->num_types) {
+ iter->record = RECORD_COUNT_TAG;
+ break;
+ }
+ iter->type = 0;
+ iter->function++;
+ /* fall through */
+ case RECORD_TIME_STAMP:
+ if (iter->function < iter->info->n_functions)
+ iter->record = RECORD_FUNCTION_TAG;
+ else
+ iter->record = -1;
+ break;
+ }
+
+ /* Check for EOF. */
+ if (iter->record == -1)
+ return -EINVAL;
+ else
+ return 0;
+}
+
+/**
+ * seq_write_gcov_u32 - write 32 bit number in gcov format to seq_file
+ * @seq: seq_file handle
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file.
+ */
+static int seq_write_gcov_u32(struct seq_file *seq, u32 v)
+{
+ return seq_write(seq, &v, sizeof(v));
+}
+
+/**
+ * seq_write_gcov_u64 - write 64 bit number in gcov format to seq_file
+ * @seq: seq_file handle
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first.
+ */
+static int seq_write_gcov_u64(struct seq_file *seq, u64 v)
+{
+ u32 data[2];
+
+ data[0] = (v & 0xffffffffUL);
+ data[1] = (v >> 32);
+ return seq_write(seq, data, sizeof(data));
+}
+
+/**
+ * gcov_iter_write - write data for current pos to seq_file
+ * @iter: file iterator
+ * @seq: seq_file handle
+ *
+ * Return zero on success, non-zero otherwise.
+ */
+int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
+{
+ switch (iter->record) {
+ case RECORD_FILE_MAGIC:
+ seq_write_gcov_u32(seq, GCOV_DATA_MAGIC);
+ break;
+
+ case RECORD_GCOV_VERSION:
+ seq_write_gcov_u32(seq, iter->info->version);
+ break;
+
+ case RECORD_TIME_STAMP:
+ seq_write_gcov_u32(seq, iter->info->stamp);
+ break;
+
+ case RECORD_FUNCTION_TAG:
+ seq_write_gcov_u32(seq, GCOV_TAG_FUNCTION);
+ break;
+
+ case RECORD_FUNCTION_TAG_LEN:
+ seq_write_gcov_u32(seq, GCOV_TAG_FUNCTION_LENGTH);
+ break;
+
+ case RECORD_FUNCTION_IDENT:
+ seq_write_gcov_u32(seq, get_func(iter)->ident);
+ break;
+
+ case RECORD_FUNCTION_LINENO_CHECK:
+ seq_write_gcov_u32(seq, get_func(iter)->lineno_checksum);
+ break;
+
+ case RECORD_FUNCTION_CFG_CHECK:
+ seq_write_gcov_u32(seq, get_func(iter)->cfg_checksum);
+ break;
+
+ case RECORD_COUNT_TAG:
+ seq_write_gcov_u32(seq,
+ GCOV_TAG_FOR_COUNTER(get_cnt_type(iter)));
+ break;
+
+ case RECORD_COUNT_LEN:
+ seq_write_gcov_u32(seq, get_ctr(iter)->num * 2);
+ break;
+
+ case RECORD_COUNT:
+ seq_write_gcov_u64(seq, get_cnt_val(iter));
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
--
1.8.3.1
This adds the .init_array section as yet another section with constructors. This
is needed because gcc is adding __gcov_init calls to .init_array.
Signed-off-by: Frantisek Hrbata <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/module.h | 2 ++
kernel/module.c | 6 ++++++
3 files changed, 9 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 69732d2..c55d8d9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -468,6 +468,7 @@
#define KERNEL_CTORS() . = ALIGN(8); \
VMLINUX_SYMBOL(__ctors_start) = .; \
*(.ctors) \
+ *(.init_array) \
VMLINUX_SYMBOL(__ctors_end) = .;
#else
#define KERNEL_CTORS()
diff --git a/include/linux/module.h b/include/linux/module.h
index 46f1ea0..89ff829 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -374,6 +374,8 @@ struct module
/* Constructor functions. */
ctor_fn_t *ctors;
unsigned int num_ctors;
+ ctor_fn_t *init_array;
+ unsigned int num_init_array;
#endif
};
#ifndef MODULE_ARCH_INIT
diff --git a/kernel/module.c b/kernel/module.c
index 2069158..581ae89 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
sizeof(*mod->ctors), &mod->num_ctors);
+ mod->init_array = section_objs(info, ".init_array",
+ sizeof(*mod->init_array),
+ &mod->num_init_array);
#endif
#ifdef CONFIG_TRACEPOINTS
@@ -3024,6 +3027,9 @@ static void do_mod_ctors(struct module *mod)
for (i = 0; i < mod->num_ctors; i++)
mod->ctors[i]();
+
+ for (i = 0; i < mod->num_init_array; i++)
+ mod->init_array[i]();
#endif
}
--
1.8.3.1
On 23.08.2013 10:39, Frantisek Hrbata wrote:
> This is an attempt to bring support for modified gcov format in gcc 4.7 to
> the kernel. It tries to leverage the existing layout/abstraction, which was
> designed keeping in mind that the gcov format could change, but some changes had
> to be make. Mostly because the current model does not take into account that
> even the core gcov structures, like gcov_info, could change. One part that could
> be problematic is the addition of the .init_array section for constructors.
It appears that gcc 4.7 support for gcov-kernel is quite important to a
number of people, at least that is what I derive from the fact that I
now know of 3 people who've been working on this support separately from
each other: you, myself (I've been close to posting my own version to
LKML) and Christophe Guillon.
It's apparent now that I made a mistake delaying the discussion of the
effort for too long, but I think your posting the patches opens up a
good opportunity to combine the best of all previous efforts.
Most of your code looks very familiar. There's one feature missing though
that Christophe brought up as a requirement: the ability for gcov-kernel
to cope with kernel modules being compiled with GCC versions implementing
a different gcov format (apparently this can happen in some embedded
setups).
Christophe proposed run-time version checking and a file-ops type function
table which is chosen based on info->version. I found this approach
somewhat intrusive and this would also not have covered the case where a
new GCC versions was used to compile kernel modules for which the base
kernel has no support. I tried to solve this requirement by combining
two changes:
1) make the gcov-format generated by gcov-kernel compile-time configurable
2) separate the gcov-format specific code into a loadable kernel module
This way, the gcov-format specific part of gcov-kernel could be replaced
when working with a different version GCC. I'll post the corresponding
patches as reply in another mail.
Back to your patches: I tested them and they work fine on s390x when
compiled with GCC 4.3.4 and 4.7.2. I'll provide some more specific
comments as replies to your patch-mails.
Regards,
Peter Oberparleiter
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 10:39, Frantisek Hrbata wrote:
> Since also the gcov structures(gcov_info, gcov_fn_info, gcov_ctr_info) can
> change between gcc releases, as shown in gcc 4.7, they cannot be defined in a
> common header and need to be moved to a specific gcc implemention file. This
> also requires to make the gcov_info structure opaque for the common code and to
> introduce simple helpers for accessing data inside gcov_info.
I've taken a similar approach in my version, although I stopped at isolating
the code that handles the linked list. I like your version better since it's
more consistent.
> diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
> index ae5bb42..27bc88a 100644
> --- a/kernel/gcov/gcc_3_4.c
> +++ b/kernel/gcov/gcc_3_4.c
> @@ -21,6 +21,121 @@
> #include <linux/vmalloc.h>
> #include "gcov.h"
>
> +#define GCOV_COUNTERS 5
The value for GCOV_COUNTERS has been changed with GCC 4.3. Before it was 5,
starting with GCC 4.3 the value is 8. While this is not strictly necessary, I'm
wondering if this should be added here to prevent any unwanted side-effects.
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 10:39, Frantisek Hrbata wrote:
> The gcov in-memory format changed in gcc 4.7. The biggest change, which
> requires this special implementation, is that gcov_info no longer contains
> array of counters for each counter type for all functions and gcov_fn_info is
> not used for mapping of function's counters to these arrays(offset). Now each
> gcov_fn_info contans it's counters, which makes things a little bit easier.
By now I've come to the conclusion that I may have over-engineered
gcov_iter_next in the original GCC 3.4 implementation. This became especially
apparent when someone tried to adjust that code to also work with a specific
android GCC version - adding a simple field to the output file format required
major complex changes.
Therefore in my version of the GCC 4.7 support, I opted to replace this logic
with the simpler approach of generating the gcov data file in-memory during
open (gcov_iter_new) and reducing gcov_iter_next/gcov_iter_write to simple
copy-from-buffer functions.
Since I can see the need to change the format code again in the future,
I think it would be easier to simplify this part of the code correspondingly.
I'm adding my version of this part of the implementation as discussion input
below (the relevant part starts at convert_to_gcda()).
---
kernel: gcov: add support for gcc 4.7 gcov format
GCC 4.7 changed the format of gcov related data structures, both
on-disk and in memory. Add support for the new format.
Signed-off-by: Peter Oberparleiter <[email protected]>
---
kernel/gcov/Makefile | 4
kernel/gcov/gcc_4_7.c | 510 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 513 insertions(+), 1 deletion(-)
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -1,3 +1,5 @@
ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
-obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
+obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
+obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
+obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
--- /dev/null
+++ b/kernel/gcov/gcc_4_7.c
@@ -0,0 +1,510 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 4.7. Future versions of gcc may change the gcov
+ * format (as happened before), so all format-specific information needs
+ * to be kept modular and easily exchangeable.
+ *
+ * This file is based on gcc-internal definitions. Functions and data
+ * structures are defined to be compatible with gcc counterparts.
+ * For a better understanding, refer to gcc source: gcc/gcov-io.h.
+ *
+ * Copyright IBM Corp. 2013
+ * Author(s): Peter Oberparleiter <[email protected]>
+ *
+ * Uses gcc-internal data definitions.
+ */
+
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/seq_file.h>
+#include <linux/vmalloc.h>
+#include <linux/types.h>
+#include "gcov.h"
+
+/*
+ * Profiling data types used for gcc 4.7 and above - these are defined by
+ * gcc and need to be kept as close to the original definition as possible to
+ * remain compatible.
+ */
+#define GCOV_COUNTERS 8
+#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
+#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
+#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
+#define GCOV_TAG_FOR_COUNTER(count) \
+ (GCOV_TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
+
+#if BITS_PER_LONG >= 64
+typedef long gcov_type;
+#else
+typedef long long gcov_type;
+#endif
+
+/**
+ * struct gcov_ctr_info - profiling data per function and counter type
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info {
+ unsigned int num;
+ gcov_type *values;
+};
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @key: comdat key
+ * @ident: object file-unique function identifier
+ * @lineno_checksum: function lineno_checksum
+ * @cfg_checksum: function cfg_checksum
+ * @ctrs: counters
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ */
+struct gcov_fn_info {
+ const struct gcov_info *key;
+ unsigned int ident;
+ unsigned int lineno_checksum;
+ unsigned int cfg_checksum;
+ struct gcov_ctr_info ctrs[0];
+};
+
+typedef void (*gcov_merge_fn)(gcov_type *, unsigned int);
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: time stamp
+ * @filename: name of the associated gcov data file
+ * @merge: functions for merging counts per counter type or null for unused
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info {
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ gcov_merge_fn merge[GCOV_COUNTERS];
+ unsigned int n_functions;
+ struct gcov_fn_info **functions;
+};
+
+/* Symbolic links to be created for each profiling data file. */
+const struct gcov_link gcov_link[] = {
+ { OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
+ { 0, NULL},
+};
+
+/*
+ * Determine whether a counter is active. Based on gcc magic. Doesn't change
+ * at run-time.
+ */
+static int counter_active(struct gcov_info *info, unsigned int type)
+{
+ return info->merge[type] != NULL;
+}
+
+/* Determine number of active counters. Based on gcc magic. */
+static unsigned int num_counter_active(struct gcov_info *info)
+{
+ unsigned int i;
+ unsigned int result = 0;
+
+ for (i = 0; i < GCOV_COUNTERS; i++) {
+ if (counter_active(info, i))
+ result++;
+ }
+ return result;
+}
+
+/**
+ * gcov_info_reset - reset profiling data to zero
+ * @info: profiling data set
+ */
+void gcov_info_reset(struct gcov_info *info)
+{
+ unsigned int active = num_counter_active(info);
+ unsigned int f, i;
+ struct gcov_ctr_info *ctr;
+
+ for (f = 0; f < info->n_functions; f++) {
+ for (i = 0; i < active; i++) {
+ ctr = &info->functions[f]->ctrs[i];
+ memset(ctr->values, 0, ctr->num * sizeof(gcov_type));
+ }
+ }
+}
+
+/**
+ * gcov_info_is_compatible - check if profiling data can be added
+ * @info1: first profiling data set
+ * @info2: second profiling data set
+ *
+ * Returns non-zero if profiling data can be added, zero otherwise.
+ */
+int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
+{
+ return (info1->stamp == info2->stamp);
+}
+
+/**
+ * gcov_info_add - add up profiling data
+ * @dest: profiling data set to which data is added
+ * @source: profiling data set which is added
+ *
+ * Adds profiling counts of @source to @dest.
+ */
+void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
+{
+ unsigned int active = num_counter_active(dest);
+ unsigned int f, i, j;
+
+ for (f = 0; f < dest->n_functions; f++) {
+ for (i = 0; i < active; i++) {
+ struct gcov_ctr_info *d_ctr =
+ &dest->functions[f]->ctrs[i];
+ struct gcov_ctr_info *s_ctr =
+ &source->functions[f]->ctrs[i];
+
+ for (j = 0; j < d_ctr->num; j++) {
+ d_ctr->values[j] += s_ctr->values[j];
+ }
+ }
+ }
+}
+
+/**
+ * gcov_info_dup - duplicate profiling data set
+ * @info: profiling data set to duplicate
+ *
+ * Return newly allocated duplicate on success, %NULL on error.
+ */
+struct gcov_info *gcov_info_dup(struct gcov_info *info)
+{
+ struct gcov_info *dup;
+ unsigned int active = num_counter_active(info);
+ unsigned int i, j;
+
+ /* Duplicate gcov_info. */
+ dup = kzalloc(sizeof(struct gcov_info), GFP_KERNEL);
+ if (!dup)
+ return NULL;
+ dup->version = info->version;
+ dup->stamp = info->stamp;
+ dup->n_functions = info->n_functions;
+ memcpy(dup->merge, info->merge, sizeof(dup->merge));
+ /* Duplicate filename. */
+ dup->filename = kstrdup(info->filename, GFP_KERNEL);
+ if (!dup->filename)
+ goto err_free;
+ /* Duplicate table of functions. */
+ dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
+ info->n_functions, GFP_KERNEL);
+ if (!dup->functions)
+ goto err_free;
+ for (i = 0; i < info->n_functions; i++) {
+ struct gcov_fn_info *src_fn = info->functions[i];
+ struct gcov_fn_info *dest_fn;
+
+ /* Duplicate gcov_fn_info. */
+ dup->functions[i] = kzalloc(sizeof(struct gcov_fn_info) +
+ sizeof(struct gcov_ctr_info) *
+ active, GFP_KERNEL);
+ if (!dup->functions[i])
+ goto err_free;
+ dest_fn = dup->functions[i];
+ dest_fn->ident = src_fn->ident;
+ dest_fn->lineno_checksum = src_fn->lineno_checksum;
+ dest_fn->cfg_checksum = src_fn->cfg_checksum;
+
+ for (j = 0; j < active; j++) {
+ struct gcov_ctr_info *src_ctr = &src_fn->ctrs[j];
+ struct gcov_ctr_info *dest_ctr = &dest_fn->ctrs[j];
+ size_t size = sizeof(gcov_type) * src_ctr->num;
+
+ /* Duplicate gcov_ctr_info. */
+ dest_ctr->num = src_ctr->num;
+ dest_ctr->values = vmalloc(size);
+ if (!dest_ctr->values)
+ goto err_free;
+ memcpy(dest_ctr->values, src_ctr->values, size);
+ }
+ }
+ return dup;
+
+err_free:
+ gcov_info_free(dup);
+ return NULL;
+}
+
+/**
+ * gcov_info_free - release memory for profiling data set duplicate
+ * @info: profiling data set duplicate to free
+ */
+void gcov_info_free(struct gcov_info *info)
+{
+ unsigned int active = num_counter_active(info);
+ unsigned int i, j;
+
+ if (info->functions) {
+ for (i = 0; i < info->n_functions; i++) {
+ if (!info->functions[i])
+ continue;
+ for (j = 0; j < active; j++)
+ vfree(info->functions[i]->ctrs[j].values);
+ kfree(info->functions[i]);
+ }
+ }
+ kfree(info->functions);
+ kfree(info->filename);
+ kfree(info);
+}
+
+#define ITER_STRIDE PAGE_SIZE
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @buffer: buffer containing file data
+ * @size: size of buffer
+ * @pos: current position in file
+ */
+struct gcov_iterator {
+ struct gcov_info *info;
+ void *buffer;
+ size_t size;
+ loff_t pos;
+};
+
+/**
+ * store_gcov_u32 - store 32 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
+ * store anything.
+ */
+static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
+{
+ u32 *data;
+
+ if (buffer) {
+ data = buffer + off;
+ *data = v;
+ }
+
+ return sizeof(*data);
+}
+
+/**
+ * store_gcov_u64 - store 64 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
+ * anything.
+ */
+static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
+{
+ u32 *data;
+
+ if (buffer) {
+ data = buffer + off;
+
+ data[0] = (v & 0xffffffffUL);
+ data[1] = (v >> 32);
+ }
+
+ return sizeof(*data) * 2;
+}
+
+/**
+ * get_ctr_type - return type of specified counter
+ * @info: profiling data set
+ * @num: index into list of active counters
+ *
+ * Returns the type of the specified counter.
+ */
+static int get_ctr_type(struct gcov_info *info, unsigned int num)
+{
+ unsigned int type;
+
+ for (type = 0; type < GCOV_COUNTERS; type++) {
+ if (counter_active(info, type)) {
+ if (num == 0)
+ break;
+ num--;
+ }
+ }
+
+ return type;
+}
+
+/**
+ * convert_to_gcda - convert profiling data set to gcda file format
+ * @buffer: the buffer to store file data or %NULL if no data should be stored
+ * @info: profiling data set to be converted
+ *
+ * Returns the number of bytes that were/would have been stored into the buffer.
+ */
+static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
+{
+ unsigned int active = num_counter_active(info);
+ unsigned int i, j, k;
+ size_t pos = 0;
+
+ /* File header. */
+ pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
+ pos += store_gcov_u32(buffer, pos, info->version);
+ pos += store_gcov_u32(buffer, pos, info->stamp);
+
+ for (i = 0; i < info->n_functions; i++) {
+ struct gcov_fn_info *fn = info->functions[i];
+
+ /* Function record. */
+ pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
+ pos += store_gcov_u32(buffer, pos, 3);
+ pos += store_gcov_u32(buffer, pos, fn->ident);
+ pos += store_gcov_u32(buffer, pos, fn->lineno_checksum);
+ pos += store_gcov_u32(buffer, pos, fn->cfg_checksum);
+
+ for (j = 0; j < active; j++) {
+ struct gcov_ctr_info *ctr = &fn->ctrs[j];
+ int type = get_ctr_type(info, j);
+
+ /* Counter record. */
+ pos += store_gcov_u32(buffer, pos,
+ GCOV_TAG_FOR_COUNTER(type));
+ pos += store_gcov_u32(buffer, pos, ctr->num * 2);
+
+ for (k = 0; k < ctr->num; k++) {
+ pos += store_gcov_u64(buffer, pos,
+ ctr->values[k]);
+ }
+ }
+
+
+ }
+
+ return pos;
+}
+
+/**
+ * gcov_iter_new - allocate and initialize profiling data iterator
+ * @info: profiling data set to be iterated
+ *
+ * Return file iterator on success, %NULL otherwise.
+ */
+struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
+{
+ struct gcov_iterator *iter;
+
+ iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
+ if (!iter)
+ goto err_free;
+
+ iter->info = info;
+ /* Dry-run to get the actual buffer size. */
+ iter->size = convert_to_gcda(NULL, info);
+ iter->buffer = vmalloc(iter->size);
+ if (!iter->buffer)
+ goto err_free;
+
+ convert_to_gcda(iter->buffer, info);
+
+ return iter;
+
+err_free:
+ kfree(iter);
+ return NULL;
+}
+
+/**
+ * gcov_iter_free - release memory for iterator
+ * @iter: file iterator to free
+ */
+void gcov_iter_free(struct gcov_iterator *iter)
+{
+ vfree(iter->buffer);
+ kfree(iter);
+}
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
+{
+ return iter->info;
+}
+
+/**
+ * gcov_iter_start - reset file iterator to starting position
+ * @iter: file iterator
+ */
+void gcov_iter_start(struct gcov_iterator *iter)
+{
+ iter->pos = 0;
+}
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+int gcov_iter_next(struct gcov_iterator *iter)
+{
+ if (iter->pos < iter->size)
+ iter->pos += ITER_STRIDE;
+ if (iter->pos >= iter->size)
+ return -EINVAL;
+
+ return 0;
+}
+
+/**
+ * gcov_iter_write - write data for current pos to seq_file
+ * @iter: file iterator
+ * @seq: seq_file handle
+ *
+ * Return zero on success, non-zero otherwise.
+ */
+int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
+{
+ size_t len;
+
+ if (iter->pos >= iter->size)
+ return -EINVAL;
+
+ len = ITER_STRIDE;
+ if (iter->pos + len > iter->size)
+ len = iter->size - iter->pos;
+
+ seq_write(seq, iter->buffer + iter->pos, len);
+
+ return 0;
+}
+
+/**
+ * gcov_info_get_filename - return name of the associated gcov data file
+ * @info: profiling data set
+ */
+const char *gcov_info_get_filename(struct gcov_info *info)
+{
+ return info->filename;
+}
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 10:39, Frantisek Hrbata wrote:
> This adds the .init_array section as yet another section with constructors. This
> is needed because gcc is adding __gcov_init calls to .init_array.
>
> Signed-off-by: Frantisek Hrbata <[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 1 +
> include/linux/module.h | 2 ++
> kernel/module.c | 6 ++++++
> 3 files changed, 9 insertions(+)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 69732d2..c55d8d9 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -468,6 +468,7 @@
> #define KERNEL_CTORS() . = ALIGN(8); \
> VMLINUX_SYMBOL(__ctors_start) = .; \
> *(.ctors) \
> + *(.init_array) \
This is exactly how I (would) have done it.
> VMLINUX_SYMBOL(__ctors_end) = .;
> #else
> #define KERNEL_CTORS()
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 46f1ea0..89ff829 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -374,6 +374,8 @@ struct module
> /* Constructor functions. */
> ctor_fn_t *ctors;
> unsigned int num_ctors;
> + ctor_fn_t *init_array;
> + unsigned int num_init_array;
I think it would be safe to re-use the ctors-related fields here. Each GCC
version will only ever create either of .ctors or .init_array, but never
both. The only case where separate fields would be required, would be when
linking a kernel module from multiple object files compiled with different
versions of GCC.
> #endif
> };
> #ifndef MODULE_ARCH_INIT
> diff --git a/kernel/module.c b/kernel/module.c
> index 2069158..581ae89 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
> #ifdef CONFIG_CONSTRUCTORS
> mod->ctors = section_objs(info, ".ctors",
> sizeof(*mod->ctors), &mod->num_ctors);
When reusing mod->ctors, you could check for mod->ctors == NULL here and
fill in the values from section_objs(".init_array").
> + mod->init_array = section_objs(info, ".init_array",
> + sizeof(*mod->init_array),
> + &mod->num_init_array);
> #endif
>
> #ifdef CONFIG_TRACEPOINTS
> @@ -3024,6 +3027,9 @@ static void do_mod_ctors(struct module *mod)
>
> for (i = 0; i < mod->num_ctors; i++)
> mod->ctors[i]();
> +
> + for (i = 0; i < mod->num_init_array; i++)
> + mod->init_array[i]();
> #endif
> }
>
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 10:39, Frantisek Hrbata wrote:
> Compile the correct gcov implementation file for a specific gcc version. In
> the future, if another file is added, the conditions will need to be somehow
> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
> be enough.
Looks good, though I think this could be merged into the main 4.7 format patch,
since without it, the 4.7 code will never be reached.
Also it is my understanding that there are some distribution-specific versions
of GCC that include the 4.7. gcov format code but report GCC version 4.6. With
the auto-detection code implemented like this, gcov-kernel won't work correctly.
For that purpose I've implemented a configuration option to allow users to
force a specific version of gcov format.
I'm attaching the corresponding patch below:
---
kernel: gcov: make data format configurable
Make the format of the generated gcov data configurable. This may be
required for example for pre-4.7 GCCs that contain the 4.7 gcov data
format changes.
Signed-off-by: Peter Oberparleiter <[email protected]>
---
kernel/gcov/Kconfig | 30 ++++++++++++++++++++++++++++++
kernel/gcov/Makefile | 21 +++++++++++++++++++--
2 files changed, 49 insertions(+), 2 deletions(-)
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -46,4 +46,34 @@ config GCOV_PROFILE_ALL
larger and run slower. Also be sure to exclude files from profiling
which are not linked to the kernel image to prevent linker errors.
+choice
+ prompt "Specify GCOV format"
+ depends on GCOV_KERNEL
+ default GCOV_FORMAT_AUTODETECT
+ ---help---
+ The gcov format is usually determined by the GCC version, but there are
+ exceptions where format changes are integrated in lower-version GCCs.
+ In such a case use this option to adjust the format used in the kernel
+ accordingly.
+
+ If unsure, choose "Autodetect".
+
+config GCOV_FORMAT_AUTODETECT
+ bool "Autodetect"
+ ---help---
+ Select this option to use the format that corresponds to your GCC
+ version.
+
+config GCOV_FORMAT_3_4
+ bool "GCC 3.4 format"
+ ---help---
+ Select this option to use the format defined by GCC 3.4.
+
+config GCOV_FORMAT_4_7
+ bool "GCC 4.7 format"
+ ---help---
+ Select this option to use the format defined by GCC 4.7.
+
+endchoice
+
endmenu
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -1,5 +1,22 @@
ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
+# if-lt
+# Usage VAR := $(call if-lt, $(a), $(b))
+# Returns 1 if (a < b)
+if-lt = $(shell [ $(1) -lt $(2) ] && echo 1)
+
+ifeq ($(CONFIG_GCOV_FORMAT_3_4),y)
+ cc-ver := 0304
+else ifeq ($(CONFIG_GCOV_FORMAT_4_7),y)
+ cc-ver := 0407
+else
+ cc-ver := $(call cc-version)
+endif
+
obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
-obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
-obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
+
+ifeq ($(call if-lt, $(cc-ver), 0407),1)
+ obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
+else
+ obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
+endif
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 17:15, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>> Compile the correct gcov implementation file for a specific gcc version. In
>> the future, if another file is added, the conditions will need to be somehow
>> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
>> be enough.
As promised, I'm also adding the patch that makes the format-specific part
of gcov-kernel a loadable kernel module:
---
kernel: gcov: make format-specific code loadable
Turn the format-specific part of gcov-kernel into a loadable kernel
module. This enables the use of gcov-kernel with kernel modules
that were compiled with a version of GCC that produces a different
gcov format when compared to the version of GCC that was used to
compile the kernel.
Signed-off-by: Peter Oberparleiter <[email protected]>
---
kernel/gcov/Kconfig | 19 +++++++++++++++++--
kernel/gcov/Makefile | 7 ++++---
2 files changed, 21 insertions(+), 5 deletions(-)
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -29,8 +29,23 @@ config GCOV_KERNEL
and:
GCOV_PROFILE := n
- Note that the debugfs filesystem has to be mounted to access
- profiling data.
+ Note that GCOV_KERNEL_FS has to specified as well and the debugfs
+ filesystem has to be mounted to access profiling data.
+
+config GCOV_KERNEL_FS
+ tristate "Provide gcov data files in debugfs"
+ depends on GCOV_KERNEL
+ default y
+ ---help---
+ Make profiling data available in debugfs at /sys/kernel/debug/gcov.
+
+ Say M if you want to enable collecting coverage data for kernel modules
+ which are compiled using a gcc version different from the one that
+ was used to compile the kernel. In that case, re-compile the gcov
+ kernel module with corresponding format support and load that module
+ instead.
+
+ If unsure, say Y.
config GCOV_PROFILE_ALL
bool "Profile entire Kernel"
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -13,10 +13,11 @@ else
cc-ver := $(call cc-version)
endif
-obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
+obj-$(CONFIG_GCOV_KERNEL) += base.o
+obj-$(CONFIG_GCOV_KERNEL_FS) += gcov.o
ifeq ($(call if-lt, $(cc-ver), 0407),1)
- obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
+ gcov-objs += fs.o gcc_3_4.o
else
- obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
+ gcov-objs += fs.o gcc_4_7.o
endif
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On Fri, Aug 23, 2013 at 05:08:07PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> > This is an attempt to bring support for modified gcov format in gcc 4.7 to
> > the kernel. It tries to leverage the existing layout/abstraction, which was
> > designed keeping in mind that the gcov format could change, but some changes had
> > to be make. Mostly because the current model does not take into account that
> > even the core gcov structures, like gcov_info, could change. One part that could
> > be problematic is the addition of the .init_array section for constructors.
>
> It appears that gcc 4.7 support for gcov-kernel is quite important to a
> number of people, at least that is what I derive from the fact that I
> now know of 3 people who've been working on this support separately from
> each other: you, myself (I've been close to posting my own version to
> LKML) and Christophe Guillon.
First, thank you for your quick reply. Second, great, I was worried there is no
interest to have the new format supported, because it's quite some time gcc 4.7
is out.
>
> It's apparent now that I made a mistake delaying the discussion of the
> effort for too long, but I think your posting the patches opens up a
> good opportunity to combine the best of all previous efforts.
To be honest I ran into this problem 14 days ago when I was asked to take a
look. So I cannot say I know everything about gcov format and stuff around it.
I mostly followed your original code after I got some idea of the gcov in-memory
layout. Meaning, I'm glad that there is also yours and Christophe's code and
I for sure welcome the "combine the best of all previous efforts" approach, even
if it means dropping my code :).
>
> Most of your code looks very familiar. There's one feature missing though
> that Christophe brought up as a requirement: the ability for gcov-kernel
> to cope with kernel modules being compiled with GCC versions implementing
> a different gcov format (apparently this can happen in some embedded
> setups).
Here follows quote from the gcc/gcov-io.h file
<quote>
Coverage information is held in two files. A notes file, which is
generated by the compiler, and a data file, which is generated by
the program under test. Both files use a similar structure. We do
not attempt to make these files backwards compatible with previous
versions, as you only need coverage information when developing a
program. We do hold version information, so that mismatches can be
detected, and we use a format that allows tools to skip information
they do not understand or are not interested in.
</quote>
Also from my experience, I would expect that the gcov will be used during
development, meaning re-compilation isn't a big problem here. I for sure can be
missing something and I'm by no means saying it wouldn't be useful feature. Just
that it would complite things a little bit.
>
> Christophe proposed run-time version checking and a file-ops type function
> table which is chosen based on info->version. I found this approach
> somewhat intrusive and this would also not have covered the case where a
> new GCC versions was used to compile kernel modules for which the base
> kernel has no support. I tried to solve this requirement by combining
> two changes:
>
> 1) make the gcov-format generated by gcov-kernel compile-time configurable
> 2) separate the gcov-format specific code into a loadable kernel module
So if I understand it correctly you would separate the input
format(gcov_info) from the output(gcda files). Meaning no matter which gcc
compiled the module you can select the gcda format. And even if the kernel does
not know the new format you can simply compile a module supporting it, instead
of the whole kernel.
Can you please give me an example why this is needed? As I wrote I'm not that
familiar with gcov and I'm probably missing something, but I do not understand
why this(handling several versions at runtime, not only the one used by gcc
during compilation) is useful(note my comment above about the gcov used during
development).
>
> This way, the gcov-format specific part of gcov-kernel could be replaced
> when working with a different version GCC. I'll post the corresponding
> patches as reply in another mail.
Great, I will take a look, but it may take me some time :).
>
> Back to your patches: I tested them and they work fine on s390x when
> compiled with GCC 4.3.4 and 4.7.2. I'll provide some more specific
> comments as replies to your patch-mails.
Many thanks!
>
>
> Regards,
> Peter Oberparleiter
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Fri, Aug 23, 2013 at 05:09:58PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> > Since also the gcov structures(gcov_info, gcov_fn_info, gcov_ctr_info) can
> > change between gcc releases, as shown in gcc 4.7, they cannot be defined in a
> > common header and need to be moved to a specific gcc implemention file. This
> > also requires to make the gcov_info structure opaque for the common code and to
> > introduce simple helpers for accessing data inside gcov_info.
>
> I've taken a similar approach in my version, although I stopped at isolating
> the code that handles the linked list. I like your version better since it's
> more consistent.
:) I also have doubts with the list "abstraction", it isn't very nice. I tried
to keep the changes as simple as possible in the generic code. I'm not sayint it
is the right approach, but your design is pretty good, so I had no urges to
change it more deeper. I'm of course open to any suggestions.
>
> > diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
> > index ae5bb42..27bc88a 100644
> > --- a/kernel/gcov/gcc_3_4.c
> > +++ b/kernel/gcov/gcc_3_4.c
> > @@ -21,6 +21,121 @@
> > #include <linux/vmalloc.h>
> > #include "gcov.h"
> >
> > +#define GCOV_COUNTERS 5
>
> The value for GCOV_COUNTERS has been changed with GCC 4.3. Before it was 5,
> starting with GCC 4.3 the value is 8. While this is not strictly necessary, I'm
> wondering if this should be added here to prevent any unwanted side-effects.
Yes I was thinking about this two. My first idea was to use some define, maybe
in the Makefile during the gcc version check and set the number of counters
properly later based on this define. Something like
#if GCOV_GCC_VERIONS >= 0430
#define GCOV_COUNTERS 8
#elif ...
for the gcc_3_4.c implementation.
But I'm not sure what the new counters are good for and if they are really
needed for the coverage info. This would require deeper understanding what
and how the types of counters are used. At this point a simply did not change the
value for the format before gcc 4.7, because each counter type has a tag and
this should be backward compatible. We only miss the new counters. Again this is
something that probably deserves more attention. Thanks for pointing this out!
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Fri, Aug 23, 2013 at 05:13:31PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> > This adds the .init_array section as yet another section with constructors. This
> > is needed because gcc is adding __gcov_init calls to .init_array.
> >
> > Signed-off-by: Frantisek Hrbata <[email protected]>
> > ---
> > include/asm-generic/vmlinux.lds.h | 1 +
> > include/linux/module.h | 2 ++
> > kernel/module.c | 6 ++++++
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 69732d2..c55d8d9 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -468,6 +468,7 @@
> > #define KERNEL_CTORS() . = ALIGN(8); \
> > VMLINUX_SYMBOL(__ctors_start) = .; \
> > *(.ctors) \
> > + *(.init_array) \
>
> This is exactly how I (would) have done it.
>
> > VMLINUX_SYMBOL(__ctors_end) = .;
> > #else
> > #define KERNEL_CTORS()
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 46f1ea0..89ff829 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -374,6 +374,8 @@ struct module
> > /* Constructor functions. */
> > ctor_fn_t *ctors;
> > unsigned int num_ctors;
> > + ctor_fn_t *init_array;
> > + unsigned int num_init_array;
>
> I think it would be safe to re-use the ctors-related fields here. Each GCC
> version will only ever create either of .ctors or .init_array, but never
> both.
> The only case where separate fields would be required, would be when
> linking a kernel module from multiple object files compiled with different
> versions of GCC.
Ah ok, I was not aware of this.
>
> > #endif
> > };
> > #ifndef MODULE_ARCH_INIT
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 2069158..581ae89 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
> > #ifdef CONFIG_CONSTRUCTORS
> > mod->ctors = section_objs(info, ".ctors",
> > sizeof(*mod->ctors), &mod->num_ctors);
>
> When reusing mod->ctors, you could check for mod->ctors == NULL here and
> fill in the values from section_objs(".init_array").
Yes, in the case that we only have .ctors or .init_array section, this sounds
as the right approach.
>
> > + mod->init_array = section_objs(info, ".init_array",
> > + sizeof(*mod->init_array),
> > + &mod->num_init_array);
> > #endif
> >
> > #ifdef CONFIG_TRACEPOINTS
> > @@ -3024,6 +3027,9 @@ static void do_mod_ctors(struct module *mod)
> >
> > for (i = 0; i < mod->num_ctors; i++)
> > mod->ctors[i]();
> > +
> > + for (i = 0; i < mod->num_init_array; i++)
> > + mod->init_array[i]();
> > #endif
> > }
> >
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Fri, Aug 23, 2013 at 05:12:19PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> > The gcov in-memory format changed in gcc 4.7. The biggest change, which
> > requires this special implementation, is that gcov_info no longer contains
> > array of counters for each counter type for all functions and gcov_fn_info is
> > not used for mapping of function's counters to these arrays(offset). Now each
> > gcov_fn_info contans it's counters, which makes things a little bit easier.
>
> By now I've come to the conclusion that I may have over-engineered
> gcov_iter_next in the original GCC 3.4 implementation. This became especially
> apparent when someone tried to adjust that code to also work with a specific
> android GCC version - adding a simple field to the output file format required
> major complex changes.
>
> Therefore in my version of the GCC 4.7 support, I opted to replace this logic
> with the simpler approach of generating the gcov data file in-memory during
> open (gcov_iter_new) and reducing gcov_iter_next/gcov_iter_write to simple
> copy-from-buffer functions.
Yes, this seems reasonable and much easier approach to me, but we will end up
with three copies of one gcov(gcda) file data in memory: data from gcc, our
buffer and the buffer in seq file. But I guess this is not a big problem,
unless someone opens a huge amount of gcda files in parallel. Generally I
like this idea. This will be also probably faster then writing 1 or 2 ints per
one iter write.
>
> Since I can see the need to change the format code again in the future,
> I think it would be easier to simplify this part of the code correspondingly.
> I'm adding my version of this part of the implementation as discussion input
> below (the relevant part starts at convert_to_gcda()).
I have really only two nits to the code(please see below). I spent some time
checking the new seq/iter implementation and it seems ok to me. Generally I like
how simple this change is done.
>
> ---
> kernel: gcov: add support for gcc 4.7 gcov format
>
> GCC 4.7 changed the format of gcov related data structures, both
> on-disk and in memory. Add support for the new format.
>
> Signed-off-by: Peter Oberparleiter <[email protected]>
> ---
> kernel/gcov/Makefile | 4
> kernel/gcov/gcc_4_7.c | 510 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 513 insertions(+), 1 deletion(-)
>
> --- a/kernel/gcov/Makefile
> +++ b/kernel/gcov/Makefile
> @@ -1,3 +1,5 @@
> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>
> -obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
> +obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
> --- /dev/null
> +++ b/kernel/gcov/gcc_4_7.c
> @@ -0,0 +1,510 @@
> +/*
> + * This code provides functions to handle gcc's profiling data format
> + * introduced with gcc 4.7. Future versions of gcc may change the gcov
> + * format (as happened before), so all format-specific information needs
> + * to be kept modular and easily exchangeable.
> + *
> + * This file is based on gcc-internal definitions. Functions and data
> + * structures are defined to be compatible with gcc counterparts.
> + * For a better understanding, refer to gcc source: gcc/gcov-io.h.
> + *
> + * Copyright IBM Corp. 2013
> + * Author(s): Peter Oberparleiter <[email protected]>
> + *
> + * Uses gcc-internal data definitions.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
> +#include <linux/types.h>
> +#include "gcov.h"
> +
> +/*
> + * Profiling data types used for gcc 4.7 and above - these are defined by
> + * gcc and need to be kept as close to the original definition as possible to
> + * remain compatible.
> + */
> +#define GCOV_COUNTERS 8
> +#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
> +#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
> +#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
> +#define GCOV_TAG_FOR_COUNTER(count) \
> + (GCOV_TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
> +
> +#if BITS_PER_LONG >= 64
> +typedef long gcov_type;
> +#else
> +typedef long long gcov_type;
> +#endif
> +
> +/**
> + * struct gcov_ctr_info - profiling data per function and counter type
> + * @num: number of counter values for this type
> + * @values: array of counter values for this type
> + *
> + * This data is generated by gcc during compilation and doesn't change
> + * at run-time with the exception of the values array.
> + */
> +struct gcov_ctr_info {
> + unsigned int num;
> + gcov_type *values;
> +};
> +
> +/**
> + * struct gcov_fn_info - profiling meta data per function
> + * @key: comdat key
> + * @ident: object file-unique function identifier
> + * @lineno_checksum: function lineno_checksum
> + * @cfg_checksum: function cfg_checksum
> + * @ctrs: counters
> + *
> + * This data is generated by gcc during compilation and doesn't change
> + * at run-time.
> + */
> +struct gcov_fn_info {
> + const struct gcov_info *key;
> + unsigned int ident;
> + unsigned int lineno_checksum;
> + unsigned int cfg_checksum;
> + struct gcov_ctr_info ctrs[0];
> +};
> +
> +typedef void (*gcov_merge_fn)(gcov_type *, unsigned int);
> +
> +/**
> + * struct gcov_info - profiling data per object file
> + * @version: gcov version magic indicating the gcc version used for compilation
> + * @next: list head for a singly-linked list
> + * @stamp: time stamp
> + * @filename: name of the associated gcov data file
> + * @merge: functions for merging counts per counter type or null for unused
> + * @n_functions: number of instrumented functions
> + * @functions: function data
> + *
> + * This data is generated by gcc during compilation and doesn't change
> + * at run-time with the exception of the next pointer.
> + */
> +struct gcov_info {
> + unsigned int version;
> + struct gcov_info *next;
> + unsigned int stamp;
> + const char *filename;
> + gcov_merge_fn merge[GCOV_COUNTERS];
> + unsigned int n_functions;
> + struct gcov_fn_info **functions;
I can see that you removed the const part. I was thinking about it too, because
it requires ugly cast-const-away code in the gcov_info_dup and it is imho not
necessary, but I left the original definition from gcc anyway.
> +};
> +
> +/* Symbolic links to be created for each profiling data file. */
> +const struct gcov_link gcov_link[] = {
> + { OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
> + { 0, NULL},
> +};
> +
> +/*
> + * Determine whether a counter is active. Based on gcc magic. Doesn't change
> + * at run-time.
> + */
> +static int counter_active(struct gcov_info *info, unsigned int type)
> +{
> + return info->merge[type] != NULL;
> +}
> +
> +/* Determine number of active counters. Based on gcc magic. */
> +static unsigned int num_counter_active(struct gcov_info *info)
> +{
> + unsigned int i;
> + unsigned int result = 0;
> +
> + for (i = 0; i < GCOV_COUNTERS; i++) {
> + if (counter_active(info, i))
> + result++;
> + }
> + return result;
> +}
> +
> +/**
> + * gcov_info_reset - reset profiling data to zero
> + * @info: profiling data set
> + */
> +void gcov_info_reset(struct gcov_info *info)
> +{
> + unsigned int active = num_counter_active(info);
> + unsigned int f, i;
> + struct gcov_ctr_info *ctr;
> +
> + for (f = 0; f < info->n_functions; f++) {
> + for (i = 0; i < active; i++) {
> + ctr = &info->functions[f]->ctrs[i];
> + memset(ctr->values, 0, ctr->num * sizeof(gcov_type));
> + }
> + }
> +}
> +
> +/**
> + * gcov_info_is_compatible - check if profiling data can be added
> + * @info1: first profiling data set
> + * @info2: second profiling data set
> + *
> + * Returns non-zero if profiling data can be added, zero otherwise.
> + */
> +int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
> +{
> + return (info1->stamp == info2->stamp);
> +}
> +
> +/**
> + * gcov_info_add - add up profiling data
> + * @dest: profiling data set to which data is added
> + * @source: profiling data set which is added
> + *
> + * Adds profiling counts of @source to @dest.
> + */
> +void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
> +{
> + unsigned int active = num_counter_active(dest);
> + unsigned int f, i, j;
> +
> + for (f = 0; f < dest->n_functions; f++) {
> + for (i = 0; i < active; i++) {
> + struct gcov_ctr_info *d_ctr =
> + &dest->functions[f]->ctrs[i];
> + struct gcov_ctr_info *s_ctr =
> + &source->functions[f]->ctrs[i];
> +
> + for (j = 0; j < d_ctr->num; j++) {
> + d_ctr->values[j] += s_ctr->values[j];
> + }
> + }
> + }
> +}
> +
> +/**
> + * gcov_info_dup - duplicate profiling data set
> + * @info: profiling data set to duplicate
> + *
> + * Return newly allocated duplicate on success, %NULL on error.
> + */
> +struct gcov_info *gcov_info_dup(struct gcov_info *info)
> +{
> + struct gcov_info *dup;
> + unsigned int active = num_counter_active(info);
> + unsigned int i, j;
> +
> + /* Duplicate gcov_info. */
> + dup = kzalloc(sizeof(struct gcov_info), GFP_KERNEL);
> + if (!dup)
> + return NULL;
> + dup->version = info->version;
> + dup->stamp = info->stamp;
> + dup->n_functions = info->n_functions;
> + memcpy(dup->merge, info->merge, sizeof(dup->merge));
> + /* Duplicate filename. */
> + dup->filename = kstrdup(info->filename, GFP_KERNEL);
> + if (!dup->filename)
> + goto err_free;
> + /* Duplicate table of functions. */
> + dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
> + info->n_functions, GFP_KERNEL);
> + if (!dup->functions)
> + goto err_free;
> + for (i = 0; i < info->n_functions; i++) {
> + struct gcov_fn_info *src_fn = info->functions[i];
> + struct gcov_fn_info *dest_fn;
> +
> + /* Duplicate gcov_fn_info. */
> + dup->functions[i] = kzalloc(sizeof(struct gcov_fn_info) +
> + sizeof(struct gcov_ctr_info) *
> + active, GFP_KERNEL);
> + if (!dup->functions[i])
> + goto err_free;
> + dest_fn = dup->functions[i];
> + dest_fn->ident = src_fn->ident;
> + dest_fn->lineno_checksum = src_fn->lineno_checksum;
> + dest_fn->cfg_checksum = src_fn->cfg_checksum;
> +
> + for (j = 0; j < active; j++) {
> + struct gcov_ctr_info *src_ctr = &src_fn->ctrs[j];
> + struct gcov_ctr_info *dest_ctr = &dest_fn->ctrs[j];
> + size_t size = sizeof(gcov_type) * src_ctr->num;
> +
> + /* Duplicate gcov_ctr_info. */
> + dest_ctr->num = src_ctr->num;
> + dest_ctr->values = vmalloc(size);
^^^^^^^
Does the vmalloc make sense here? The counters are allocated per function, so I
expect they will be pretty small compared when there was one "huge" array for
each counter type for all functions per gcov_info. Meaning here we will not
consume a "big" continuous block of phys memory. I'm just currious and maybe I'm
missing something. Anyway this is just a nit.
> + if (!dest_ctr->values)
> + goto err_free;
> + memcpy(dest_ctr->values, src_ctr->values, size);
> + }
> + }
> + return dup;
> +
> +err_free:
> + gcov_info_free(dup);
> + return NULL;
> +}
> +
> +/**
> + * gcov_info_free - release memory for profiling data set duplicate
> + * @info: profiling data set duplicate to free
> + */
> +void gcov_info_free(struct gcov_info *info)
> +{
> + unsigned int active = num_counter_active(info);
> + unsigned int i, j;
> +
> + if (info->functions) {
> + for (i = 0; i < info->n_functions; i++) {
> + if (!info->functions[i])
> + continue;
> + for (j = 0; j < active; j++)
> + vfree(info->functions[i]->ctrs[j].values);
> + kfree(info->functions[i]);
> + }
> + }
> + kfree(info->functions);
> + kfree(info->filename);
> + kfree(info);
> +}
> +
> +#define ITER_STRIDE PAGE_SIZE
> +
> +/**
> + * struct gcov_iterator - specifies current file position in logical records
> + * @buffer: buffer containing file data
> + * @size: size of buffer
> + * @pos: current position in file
> + */
> +struct gcov_iterator {
> + struct gcov_info *info;
> + void *buffer;
> + size_t size;
> + loff_t pos;
> +};
> +
> +/**
> + * store_gcov_u32 - store 32 bit number in gcov format to buffer
> + * @buffer: target buffer or NULL
> + * @off: offset into the buffer
> + * @v: value to be stored
> + *
> + * Number format defined by gcc: numbers are recorded in the 32 bit
> + * unsigned binary form of the endianness of the machine generating the
> + * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
> + * store anything.
> + */
> +static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
> +{
> + u32 *data;
> +
> + if (buffer) {
> + data = buffer + off;
> + *data = v;
> + }
> +
> + return sizeof(*data);
> +}
> +
> +/**
> + * store_gcov_u64 - store 64 bit number in gcov format to buffer
> + * @buffer: target buffer or NULL
> + * @off: offset into the buffer
> + * @v: value to be stored
> + *
> + * Number format defined by gcc: numbers are recorded in the 32 bit
> + * unsigned binary form of the endianness of the machine generating the
> + * file. 64 bit numbers are stored as two 32 bit numbers, the low part
> + * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
> + * anything.
> + */
> +static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
> +{
> + u32 *data;
> +
> + if (buffer) {
> + data = buffer + off;
> +
> + data[0] = (v & 0xffffffffUL);
> + data[1] = (v >> 32);
> + }
> +
> + return sizeof(*data) * 2;
> +}
> +
> +/**
> + * get_ctr_type - return type of specified counter
> + * @info: profiling data set
> + * @num: index into list of active counters
> + *
> + * Returns the type of the specified counter.
> + */
> +static int get_ctr_type(struct gcov_info *info, unsigned int num)
> +{
> + unsigned int type;
> +
> + for (type = 0; type < GCOV_COUNTERS; type++) {
> + if (counter_active(info, type)) {
> + if (num == 0)
> + break;
> + num--;
> + }
> + }
> +
> + return type;
> +}
> +
> +/**
> + * convert_to_gcda - convert profiling data set to gcda file format
> + * @buffer: the buffer to store file data or %NULL if no data should be stored
> + * @info: profiling data set to be converted
> + *
> + * Returns the number of bytes that were/would have been stored into the buffer.
> + */
> +static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
> +{
> + unsigned int active = num_counter_active(info);
> + unsigned int i, j, k;
> + size_t pos = 0;
> +
> + /* File header. */
> + pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
> + pos += store_gcov_u32(buffer, pos, info->version);
> + pos += store_gcov_u32(buffer, pos, info->stamp);
> +
> + for (i = 0; i < info->n_functions; i++) {
> + struct gcov_fn_info *fn = info->functions[i];
> +
> + /* Function record. */
> + pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
> + pos += store_gcov_u32(buffer, pos, 3);
> + pos += store_gcov_u32(buffer, pos, fn->ident);
> + pos += store_gcov_u32(buffer, pos, fn->lineno_checksum);
> + pos += store_gcov_u32(buffer, pos, fn->cfg_checksum);
> +
> + for (j = 0; j < active; j++) {
> + struct gcov_ctr_info *ctr = &fn->ctrs[j];
> + int type = get_ctr_type(info, j);
> +
> + /* Counter record. */
> + pos += store_gcov_u32(buffer, pos,
> + GCOV_TAG_FOR_COUNTER(type));
> + pos += store_gcov_u32(buffer, pos, ctr->num * 2);
> +
> + for (k = 0; k < ctr->num; k++) {
> + pos += store_gcov_u64(buffer, pos,
> + ctr->values[k]);
> + }
> + }
> +
> +
> + }
> +
> + return pos;
> +}
> +
> +/**
> + * gcov_iter_new - allocate and initialize profiling data iterator
> + * @info: profiling data set to be iterated
> + *
> + * Return file iterator on success, %NULL otherwise.
> + */
> +struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
> +{
> + struct gcov_iterator *iter;
> +
> + iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
> + if (!iter)
> + goto err_free;
> +
> + iter->info = info;
> + /* Dry-run to get the actual buffer size. */
> + iter->size = convert_to_gcda(NULL, info);
> + iter->buffer = vmalloc(iter->size);
> + if (!iter->buffer)
> + goto err_free;
> +
> + convert_to_gcda(iter->buffer, info);
> +
> + return iter;
> +
> +err_free:
> + kfree(iter);
> + return NULL;
> +}
> +
> +/**
> + * gcov_iter_free - release memory for iterator
> + * @iter: file iterator to free
> + */
> +void gcov_iter_free(struct gcov_iterator *iter)
> +{
> + vfree(iter->buffer);
> + kfree(iter);
> +}
> +
> +/**
> + * gcov_iter_get_info - return profiling data set for given file iterator
> + * @iter: file iterator
> + */
> +struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
> +{
> + return iter->info;
> +}
> +
> +/**
> + * gcov_iter_start - reset file iterator to starting position
> + * @iter: file iterator
> + */
> +void gcov_iter_start(struct gcov_iterator *iter)
> +{
> + iter->pos = 0;
> +}
> +
> +/**
> + * gcov_iter_next - advance file iterator to next logical record
> + * @iter: file iterator
> + *
> + * Return zero if new position is valid, non-zero if iterator has reached end.
> + */
> +int gcov_iter_next(struct gcov_iterator *iter)
> +{
> + if (iter->pos < iter->size)
> + iter->pos += ITER_STRIDE;
> + if (iter->pos >= iter->size)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> + * gcov_iter_write - write data for current pos to seq_file
> + * @iter: file iterator
> + * @seq: seq_file handle
> + *
> + * Return zero on success, non-zero otherwise.
> + */
> +int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
> +{
> + size_t len;
> +
> + if (iter->pos >= iter->size)
> + return -EINVAL;
> +
> + len = ITER_STRIDE;
> + if (iter->pos + len > iter->size)
> + len = iter->size - iter->pos;
> +
> + seq_write(seq, iter->buffer + iter->pos, len);
> +
> + return 0;
> +}
> +
> +/**
> + * gcov_info_get_filename - return name of the associated gcov data file
> + * @info: profiling data set
> + */
> +const char *gcov_info_get_filename(struct gcov_info *info)
> +{
> + return info->filename;
> +}
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Fri, Aug 23, 2013 at 05:15:19PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> > Compile the correct gcov implementation file for a specific gcc version. In
> > the future, if another file is added, the conditions will need to be somehow
> > adjusted to if-elif-else case, but at this point the simple cc-ifversion should
> > be enough.
>
> Looks good, though I think this could be merged into the main 4.7 format patch,
> since without it, the 4.7 code will never be reached.
Sure, I can merge these two patches.
>
> Also it is my understanding that there are some distribution-specific versions
> of GCC that include the 4.7. gcov format code but report GCC version 4.6. With
> the auto-detection code implemented like this, gcov-kernel won't work correctly.
> For that purpose I've implemented a configuration option to allow users to
> force a specific version of gcov format.
Ah, I was not aware of this inconsistency in versioning. This raises a question
if it would not be better to deal directly with version in the gcov_info
instead of these config options. This would of course mean some kind of gcov
operations callbacks per gcov version(you already mentioned the file
operations approach).
>
> I'm attaching the corresponding patch below:
>
> ---
> kernel: gcov: make data format configurable
>
> Make the format of the generated gcov data configurable. This may be
> required for example for pre-4.7 GCCs that contain the 4.7 gcov data
> format changes.
>
> Signed-off-by: Peter Oberparleiter <[email protected]>
> ---
> kernel/gcov/Kconfig | 30 ++++++++++++++++++++++++++++++
> kernel/gcov/Makefile | 21 +++++++++++++++++++--
> 2 files changed, 49 insertions(+), 2 deletions(-)
>
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -46,4 +46,34 @@ config GCOV_PROFILE_ALL
> larger and run slower. Also be sure to exclude files from profiling
> which are not linked to the kernel image to prevent linker errors.
>
> +choice
> + prompt "Specify GCOV format"
> + depends on GCOV_KERNEL
> + default GCOV_FORMAT_AUTODETECT
> + ---help---
> + The gcov format is usually determined by the GCC version, but there are
> + exceptions where format changes are integrated in lower-version GCCs.
> + In such a case use this option to adjust the format used in the kernel
> + accordingly.
> +
> + If unsure, choose "Autodetect".
> +
> +config GCOV_FORMAT_AUTODETECT
> + bool "Autodetect"
> + ---help---
> + Select this option to use the format that corresponds to your GCC
> + version.
> +
> +config GCOV_FORMAT_3_4
> + bool "GCC 3.4 format"
> + ---help---
> + Select this option to use the format defined by GCC 3.4.
> +
> +config GCOV_FORMAT_4_7
> + bool "GCC 4.7 format"
> + ---help---
> + Select this option to use the format defined by GCC 4.7.
> +
> +endchoice
> +
> endmenu
> --- a/kernel/gcov/Makefile
> +++ b/kernel/gcov/Makefile
> @@ -1,5 +1,22 @@
> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>
> +# if-lt
> +# Usage VAR := $(call if-lt, $(a), $(b))
> +# Returns 1 if (a < b)
> +if-lt = $(shell [ $(1) -lt $(2) ] && echo 1)
> +
> +ifeq ($(CONFIG_GCOV_FORMAT_3_4),y)
> + cc-ver := 0304
> +else ifeq ($(CONFIG_GCOV_FORMAT_4_7),y)
> + cc-ver := 0407
> +else
> + cc-ver := $(call cc-version)
> +endif
> +
> obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
> +
> +ifeq ($(call if-lt, $(cc-ver), 0407),1)
> + obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
> +else
> + obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
> +endif
>
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Fri, Aug 23, 2013 at 05:21:12PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 17:15, Peter Oberparleiter wrote:
> > On 23.08.2013 10:39, Frantisek Hrbata wrote:
> >> Compile the correct gcov implementation file for a specific gcc version. In
> >> the future, if another file is added, the conditions will need to be somehow
> >> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
> >> be enough.
>
> As promised, I'm also adding the patch that makes the format-specific part
> of gcov-kernel a loadable kernel module:
>
> ---
> kernel: gcov: make format-specific code loadable
>
> Turn the format-specific part of gcov-kernel into a loadable kernel
> module. This enables the use of gcov-kernel with kernel modules
> that were compiled with a version of GCC that produces a different
> gcov format when compared to the version of GCC that was used to
> compile the kernel.
If I understand it correctly, this would mean that you will be able to use only
one implementation of gcov format at the time. Meaning you will be able to get
coverage data for module, but not for kernel if it was compiled with different
gcc(gcda format). This is probably ok if you work only on your module, but I'm
not sure this is generally the right approach. In this case I would probably
rather see some support for more gcov formats at the same time(e.g. set of
callback operations per gcov version). Again I'm probably missing something, but
I still cannot see reason why to add such feature. If you want gcov support just
compile your kernel and modules with the same gcc version(gcda format). But if
this is really needed maybe it would be better to consider some parallel support
for more gcov formats based on the gcov_info version.
Would it be possible to add support for the modified gcc 4.7 gcov format and
deal with this later? I can incorporate your changes: iter to use buffer,
.init_array for modules and possibility to explicitly select the gcda format.
In this case we will have at least the basic support in kernel. This is just me
thinking out loud.
Many thanks Peter!
>
> Signed-off-by: Peter Oberparleiter <[email protected]>
> ---
> kernel/gcov/Kconfig | 19 +++++++++++++++++--
> kernel/gcov/Makefile | 7 ++++---
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -29,8 +29,23 @@ config GCOV_KERNEL
> and:
> GCOV_PROFILE := n
>
> - Note that the debugfs filesystem has to be mounted to access
> - profiling data.
> + Note that GCOV_KERNEL_FS has to specified as well and the debugfs
> + filesystem has to be mounted to access profiling data.
> +
> +config GCOV_KERNEL_FS
> + tristate "Provide gcov data files in debugfs"
> + depends on GCOV_KERNEL
> + default y
> + ---help---
> + Make profiling data available in debugfs at /sys/kernel/debug/gcov.
> +
> + Say M if you want to enable collecting coverage data for kernel modules
> + which are compiled using a gcc version different from the one that
> + was used to compile the kernel. In that case, re-compile the gcov
> + kernel module with corresponding format support and load that module
> + instead.
> +
> + If unsure, say Y.
>
> config GCOV_PROFILE_ALL
> bool "Profile entire Kernel"
> --- a/kernel/gcov/Makefile
> +++ b/kernel/gcov/Makefile
> @@ -13,10 +13,11 @@ else
> cc-ver := $(call cc-version)
> endif
>
> -obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
> +obj-$(CONFIG_GCOV_KERNEL) += base.o
> +obj-$(CONFIG_GCOV_KERNEL_FS) += gcov.o
>
> ifeq ($(call if-lt, $(cc-ver), 0407),1)
> - obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
> + gcov-objs += fs.o gcc_3_4.o
> else
> - obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
> + gcov-objs += fs.o gcc_4_7.o
> endif
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Saturday 24 August 2013, Frantisek Hrbata wrote:
> If I understand it correctly, this would mean that you will be able to use only
> one implementation of gcov format at the time. Meaning you will be able to get
> coverage data for module, but not for kernel if it was compiled with different
> gcc(gcda format). This is probably ok if you work only on your module, but I'm
> not sure this is generally the right approach. In this case I would probably
> rather see some support for more gcov formats at the same time(e.g. set of
> callback operations per gcov version). Again I'm probably missing something, but
> I still cannot see reason why to add such feature. If you want gcov support just
> compile your kernel and modules with the same gcc version(gcda format). But if
> this is really needed maybe it would be better to consider some parallel support
> for more gcov formats based on the gcov_info version.
The kernel is always built with exactly one version, including all the modules.
I don't see any reason whatsoever to support externally built modules with gcov,
in particular when they are not built with the system compiler.
Arnd
Hi all
I am interested to enable Gcov on my platform (on a target board). I
have tried to apply the 4 patches submitted by Frantisek Hrbata. But,
the *.gcda is always empty. Note, I am using GCC 4.7.3.
I have followed the steps in Documentation/gcov.txt. I've mounted the
NFS filesystem and it has access to the kernel build directory on host
machine.The path for the kernel source in host and target are same.
Let said my kernel module is under drivers/misc,
drivers/misc/module.ko. Turn on Gcov in drivers/misc/Makefile.
1. mount -t debugfs none /sys/kernel/debug/
2. insmod <PATH>/module.ko
3. Perform some actions to excute code in module.ko
4. rmmod <PATH>/module.ko
5. Check /sys/kernel/debug/gcov/<PATH>/drivers/misc/. There is a soft
link of module.gcno (I check the soft link is working), but
module.gcda is empty.
Any step is incorrect or missing? Or anyone know how to enable GCOV on
target board? Please advice.
I would like to enable GCOV in arch/ code as well.
Thank you very much.
Cheers.
On Sat, Aug 24, 2013 at 12:15 AM, Frantisek Hrbata <[email protected]> wrote:
> On Fri, Aug 23, 2013 at 05:08:07PM +0200, Peter Oberparleiter wrote:
>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>> > This is an attempt to bring support for modified gcov format in gcc 4.7 to
>> > the kernel. It tries to leverage the existing layout/abstraction, which was
>> > designed keeping in mind that the gcov format could change, but some changes had
>> > to be make. Mostly because the current model does not take into account that
>> > even the core gcov structures, like gcov_info, could change. One part that could
>> > be problematic is the addition of the .init_array section for constructors.
>>
>> It appears that gcc 4.7 support for gcov-kernel is quite important to a
>> number of people, at least that is what I derive from the fact that I
>> now know of 3 people who've been working on this support separately from
>> each other: you, myself (I've been close to posting my own version to
>> LKML) and Christophe Guillon.
>
> First, thank you for your quick reply. Second, great, I was worried there is no
> interest to have the new format supported, because it's quite some time gcc 4.7
> is out.
>
>>
>> It's apparent now that I made a mistake delaying the discussion of the
>> effort for too long, but I think your posting the patches opens up a
>> good opportunity to combine the best of all previous efforts.
>
> To be honest I ran into this problem 14 days ago when I was asked to take a
> look. So I cannot say I know everything about gcov format and stuff around it.
> I mostly followed your original code after I got some idea of the gcov in-memory
> layout. Meaning, I'm glad that there is also yours and Christophe's code and
> I for sure welcome the "combine the best of all previous efforts" approach, even
> if it means dropping my code :).
>
>>
>> Most of your code looks very familiar. There's one feature missing though
>> that Christophe brought up as a requirement: the ability for gcov-kernel
>> to cope with kernel modules being compiled with GCC versions implementing
>> a different gcov format (apparently this can happen in some embedded
>> setups).
>
> Here follows quote from the gcc/gcov-io.h file
>
> <quote>
> Coverage information is held in two files. A notes file, which is
> generated by the compiler, and a data file, which is generated by
> the program under test. Both files use a similar structure. We do
> not attempt to make these files backwards compatible with previous
> versions, as you only need coverage information when developing a
> program. We do hold version information, so that mismatches can be
> detected, and we use a format that allows tools to skip information
> they do not understand or are not interested in.
> </quote>
>
> Also from my experience, I would expect that the gcov will be used during
> development, meaning re-compilation isn't a big problem here. I for sure can be
> missing something and I'm by no means saying it wouldn't be useful feature. Just
> that it would complite things a little bit.
>
>>
>> Christophe proposed run-time version checking and a file-ops type function
>> table which is chosen based on info->version. I found this approach
>> somewhat intrusive and this would also not have covered the case where a
>> new GCC versions was used to compile kernel modules for which the base
>> kernel has no support. I tried to solve this requirement by combining
>> two changes:
>>
>> 1) make the gcov-format generated by gcov-kernel compile-time configurable
>> 2) separate the gcov-format specific code into a loadable kernel module
>
> So if I understand it correctly you would separate the input
> format(gcov_info) from the output(gcda files). Meaning no matter which gcc
> compiled the module you can select the gcda format. And even if the kernel does
> not know the new format you can simply compile a module supporting it, instead
> of the whole kernel.
>
> Can you please give me an example why this is needed? As I wrote I'm not that
> familiar with gcov and I'm probably missing something, but I do not understand
> why this(handling several versions at runtime, not only the one used by gcc
> during compilation) is useful(note my comment above about the gcov used during
> development).
>
>>
>> This way, the gcov-format specific part of gcov-kernel could be replaced
>> when working with a different version GCC. I'll post the corresponding
>> patches as reply in another mail.
>
> Great, I will take a look, but it may take me some time :).
>
>>
>> Back to your patches: I tested them and they work fine on s390x when
>> compiled with GCC 4.3.4 and 4.7.2. I'll provide some more specific
>> comments as replies to your patch-mails.
>
> Many thanks!
>
>>
>>
>> Regards,
>> Peter Oberparleiter
>>
>> --
>> Peter Oberparleiter
>> Linux on System z Development - IBM Germany
>>
>
> --
> Frantisek Hrbata
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On 23.08.2013 18:15, Frantisek Hrbata wrote:
> On Fri, Aug 23, 2013 at 05:08:07PM +0200, Peter Oberparleiter wrote:
>> Most of your code looks very familiar. There's one feature missing though
>> that Christophe brought up as a requirement: the ability for gcov-kernel
>> to cope with kernel modules being compiled with GCC versions implementing
>> a different gcov format (apparently this can happen in some embedded
>> setups).
>
> Here follows quote from the gcc/gcov-io.h file
>
> <quote>
> Coverage information is held in two files. A notes file, which is
> generated by the compiler, and a data file, which is generated by
> the program under test. Both files use a similar structure. We do
> not attempt to make these files backwards compatible with previous
> versions, as you only need coverage information when developing a
> program. We do hold version information, so that mismatches can be
> detected, and we use a format that allows tools to skip information
> they do not understand or are not interested in.
> </quote>
>
> Also from my experience, I would expect that the gcov will be used during
> development, meaning re-compilation isn't a big problem here. I for sure can be
> missing something and I'm by no means saying it wouldn't be useful feature. Just
> that it would complite things a little bit.
Here's the info I got from Christophe when we last discussed this feature:
"The main issue is compilation of modules with a different
compiler from the kernel. It is a quite common situation in embedded
Linuxes, as modules can be published far after the core kernel, thus
for our use it is a good feature."
I'd say that if we can support that setup with manageable extra effort,
it would be worth it.
>> Christophe proposed run-time version checking and a file-ops type function
>> table which is chosen based on info->version. I found this approach
>> somewhat intrusive and this would also not have covered the case where a
>> new GCC versions was used to compile kernel modules for which the base
>> kernel has no support. I tried to solve this requirement by combining
>> two changes:
>>
>> 1) make the gcov-format generated by gcov-kernel compile-time configurable
>> 2) separate the gcov-format specific code into a loadable kernel module
>
> So if I understand it correctly you would separate the input
> format(gcov_info) from the output(gcda files). Meaning no matter which gcc
> compiled the module you can select the gcda format. And even if the kernel does
> not know the new format you can simply compile a module supporting it, instead
> of the whole kernel.
Actually my proposal is far simpler:
- the gcov-kernel module supports one GCC format (both gcov_info as well as .gcda)
- the module can be recompiled with different format support
- it will create correct .gcda files for all object files compiled with a
compatible compiler
- for other object files, either the resulting .gcda files are unusable or no
.gcda file would be registered at all (based on info->version differences),
though that would raise the question on how to handle older GCC versions
into which the new gcov format code was integrated
> Can you please give me an example why this is needed? As I wrote I'm not that
> familiar with gcov and I'm probably missing something, but I do not understand
> why this(handling several versions at runtime, not only the one used by gcc
> during compilation) is useful(note my comment above about the gcov used during
> development).
See note above from Christophe: kernel and module are compiled by different parties
at different times using different GCC versions, and apparently the production kernel
has gcov support enabled or they are providing a separate test kernel for that.
Regards,
Peter
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 18:50, Frantisek Hrbata wrote:
> On Fri, Aug 23, 2013 at 05:09:58PM +0200, Peter Oberparleiter wrote:
>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>>> Since also the gcov structures(gcov_info, gcov_fn_info, gcov_ctr_info) can
>>> change between gcc releases, as shown in gcc 4.7, they cannot be defined in a
>>> common header and need to be moved to a specific gcc implemention file. This
>>> also requires to make the gcov_info structure opaque for the common code and to
>>> introduce simple helpers for accessing data inside gcov_info.
>>
>> I've taken a similar approach in my version, although I stopped at isolating
>> the code that handles the linked list. I like your version better since it's
>> more consistent.
>
> :) I also have doubts with the list "abstraction", it isn't very nice. I tried
> to keep the changes as simple as possible in the generic code. I'm not sayint it
> is the right approach, but your design is pretty good, so I had no urges to
> change it more deeper. I'm of course open to any suggestions.
I'd say go for this approach while it works and consider a replacement when it
becomes necessary (because only then do we know what the requirements will be).
>>> diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
>>> index ae5bb42..27bc88a 100644
>>> --- a/kernel/gcov/gcc_3_4.c
>>> +++ b/kernel/gcov/gcc_3_4.c
>>> @@ -21,6 +21,121 @@
>>> #include <linux/vmalloc.h>
>>> #include "gcov.h"
>>>
>>> +#define GCOV_COUNTERS 5
>>
>> The value for GCOV_COUNTERS has been changed with GCC 4.3. Before it was 5,
>> starting with GCC 4.3 the value is 8. While this is not strictly necessary, I'm
>> wondering if this should be added here to prevent any unwanted side-effects.
>
> Yes I was thinking about this two. My first idea was to use some define, maybe
> in the Makefile during the gcc version check and set the number of counters
> properly later based on this define. Something like
>
> #if GCOV_GCC_VERIONS >= 0430
> #define GCOV_COUNTERS 8
> #elif ...
>
> for the gcc_3_4.c implementation.
>
> But I'm not sure what the new counters are good for and if they are really
> needed for the coverage info. This would require deeper understanding what
> and how the types of counters are used. At this point a simply did not change the
> value for the format before gcc 4.7, because each counter type has a tag and
> this should be backward compatible. We only miss the new counters. Again this is
> something that probably deserves more attention. Thanks for pointing this out!
Starting with GCC 4.7 support, GCOV_COUNTERS will have a direct effect on the
size of gcov_info, so an incorrect value will break the format code. The change
I commented on was pre-4.7 code though, so its not that important there. On the
other hand it could help to have a corresponding mechanism in place once
GCOV_COUNTERS changes again. Maybe something like a macro to determine if GCC
is below a certain level.
Regards,
Peter
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 23.08.2013 23:00, Frantisek Hrbata wrote:
> On Fri, Aug 23, 2013 at 05:12:19PM +0200, Peter Oberparleiter wrote:
>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>>> The gcov in-memory format changed in gcc 4.7. The biggest change, which
>>> requires this special implementation, is that gcov_info no longer contains
>>> array of counters for each counter type for all functions and gcov_fn_info is
>>> not used for mapping of function's counters to these arrays(offset). Now each
>>> gcov_fn_info contans it's counters, which makes things a little bit easier.
>>
>> By now I've come to the conclusion that I may have over-engineered
>> gcov_iter_next in the original GCC 3.4 implementation. This became especially
>> apparent when someone tried to adjust that code to also work with a specific
>> android GCC version - adding a simple field to the output file format required
>> major complex changes.
>>
>> Therefore in my version of the GCC 4.7 support, I opted to replace this logic
>> with the simpler approach of generating the gcov data file in-memory during
>> open (gcov_iter_new) and reducing gcov_iter_next/gcov_iter_write to simple
>> copy-from-buffer functions.
>
> Yes, this seems reasonable and much easier approach to me, but we will end up
> with three copies of one gcov(gcda) file data in memory: data from gcc, our
> buffer and the buffer in seq file. But I guess this is not a big problem,
> unless someone opens a huge amount of gcda files in parallel. Generally I
> like this idea. This will be also probably faster then writing 1 or 2 ints per
> one iter write.
You're right about the data being in multiple buffers for the time that a .gcda
file is opened, but as you mentioned, this shouldn't be a problem since usually
only one .gcda file is open at a time. Also the reduction in code complexity is
in my opinion well worth it.
>> Since I can see the need to change the format code again in the future,
>> I think it would be easier to simplify this part of the code correspondingly.
>> I'm adding my version of this part of the implementation as discussion input
>> below (the relevant part starts at convert_to_gcda()).
>
> I have really only two nits to the code(please see below). I spent some time
> checking the new seq/iter implementation and it seems ok to me. Generally I like
> how simple this change is done.
>
>>
>> ---
>> kernel: gcov: add support for gcc 4.7 gcov format
>>
>> GCC 4.7 changed the format of gcov related data structures, both
>> on-disk and in memory. Add support for the new format.
>>
>> Signed-off-by: Peter Oberparleiter <[email protected]>
>> ---
>> kernel/gcov/Makefile | 4
>> kernel/gcov/gcc_4_7.c | 510 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 513 insertions(+), 1 deletion(-)
>>
>> --- a/kernel/gcov/Makefile
>> +++ b/kernel/gcov/Makefile
>> @@ -1,3 +1,5 @@
>> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>>
>> -obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
>> +obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
>> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
>> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
>> --- /dev/null
>> +++ b/kernel/gcov/gcc_4_7.c
>> @@ -0,0 +1,510 @@
>> +/*
>> + * This code provides functions to handle gcc's profiling data format
>> + * introduced with gcc 4.7. Future versions of gcc may change the gcov
>> + * format (as happened before), so all format-specific information needs
>> + * to be kept modular and easily exchangeable.
>> + *
>> + * This file is based on gcc-internal definitions. Functions and data
>> + * structures are defined to be compatible with gcc counterparts.
>> + * For a better understanding, refer to gcc source: gcc/gcov-io.h.
>> + *
>> + * Copyright IBM Corp. 2013
>> + * Author(s): Peter Oberparleiter <[email protected]>
>> + *
>> + * Uses gcc-internal data definitions.
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/slab.h>
>> +#include <linux/string.h>
>> +#include <linux/seq_file.h>
>> +#include <linux/vmalloc.h>
>> +#include <linux/types.h>
>> +#include "gcov.h"
>> +
>> +/*
>> + * Profiling data types used for gcc 4.7 and above - these are defined by
>> + * gcc and need to be kept as close to the original definition as possible to
>> + * remain compatible.
>> + */
>> +#define GCOV_COUNTERS 8
>> +#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
>> +#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
>> +#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
>> +#define GCOV_TAG_FOR_COUNTER(count) \
>> + (GCOV_TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
>> +
>> +#if BITS_PER_LONG >= 64
>> +typedef long gcov_type;
>> +#else
>> +typedef long long gcov_type;
>> +#endif
>> +
>> +/**
>> + * struct gcov_ctr_info - profiling data per function and counter type
>> + * @num: number of counter values for this type
>> + * @values: array of counter values for this type
>> + *
>> + * This data is generated by gcc during compilation and doesn't change
>> + * at run-time with the exception of the values array.
>> + */
>> +struct gcov_ctr_info {
>> + unsigned int num;
>> + gcov_type *values;
>> +};
>> +
>> +/**
>> + * struct gcov_fn_info - profiling meta data per function
>> + * @key: comdat key
>> + * @ident: object file-unique function identifier
>> + * @lineno_checksum: function lineno_checksum
>> + * @cfg_checksum: function cfg_checksum
>> + * @ctrs: counters
>> + *
>> + * This data is generated by gcc during compilation and doesn't change
>> + * at run-time.
>> + */
>> +struct gcov_fn_info {
>> + const struct gcov_info *key;
>> + unsigned int ident;
>> + unsigned int lineno_checksum;
>> + unsigned int cfg_checksum;
>> + struct gcov_ctr_info ctrs[0];
>> +};
>> +
>> +typedef void (*gcov_merge_fn)(gcov_type *, unsigned int);
>> +
>> +/**
>> + * struct gcov_info - profiling data per object file
>> + * @version: gcov version magic indicating the gcc version used for compilation
>> + * @next: list head for a singly-linked list
>> + * @stamp: time stamp
>> + * @filename: name of the associated gcov data file
>> + * @merge: functions for merging counts per counter type or null for unused
>> + * @n_functions: number of instrumented functions
>> + * @functions: function data
>> + *
>> + * This data is generated by gcc during compilation and doesn't change
>> + * at run-time with the exception of the next pointer.
>> + */
>> +struct gcov_info {
>> + unsigned int version;
>> + struct gcov_info *next;
>> + unsigned int stamp;
>> + const char *filename;
>> + gcov_merge_fn merge[GCOV_COUNTERS];
>> + unsigned int n_functions;
>> + struct gcov_fn_info **functions;
>
> I can see that you removed the const part. I was thinking about it too, because
> it requires ugly cast-const-away code in the gcov_info_dup and it is imho not
> necessary, but I left the original definition from gcc anyway.
The kernel's usage of gcov_info is slightly different from the GCC's internal
one, so a slight divergence should be ok from my point of view, especially
considering that the outcome (dropping a const declaration) is the same.
>
>> +};
>> +
>> +/* Symbolic links to be created for each profiling data file. */
>> +const struct gcov_link gcov_link[] = {
>> + { OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
>> + { 0, NULL},
>> +};
>> +
>> +/*
>> + * Determine whether a counter is active. Based on gcc magic. Doesn't change
>> + * at run-time.
>> + */
>> +static int counter_active(struct gcov_info *info, unsigned int type)
>> +{
>> + return info->merge[type] != NULL;
>> +}
>> +
>> +/* Determine number of active counters. Based on gcc magic. */
>> +static unsigned int num_counter_active(struct gcov_info *info)
>> +{
>> + unsigned int i;
>> + unsigned int result = 0;
>> +
>> + for (i = 0; i < GCOV_COUNTERS; i++) {
>> + if (counter_active(info, i))
>> + result++;
>> + }
>> + return result;
>> +}
>> +
>> +/**
>> + * gcov_info_reset - reset profiling data to zero
>> + * @info: profiling data set
>> + */
>> +void gcov_info_reset(struct gcov_info *info)
>> +{
>> + unsigned int active = num_counter_active(info);
>> + unsigned int f, i;
>> + struct gcov_ctr_info *ctr;
>> +
>> + for (f = 0; f < info->n_functions; f++) {
>> + for (i = 0; i < active; i++) {
>> + ctr = &info->functions[f]->ctrs[i];
>> + memset(ctr->values, 0, ctr->num * sizeof(gcov_type));
>> + }
>> + }
>> +}
>> +
>> +/**
>> + * gcov_info_is_compatible - check if profiling data can be added
>> + * @info1: first profiling data set
>> + * @info2: second profiling data set
>> + *
>> + * Returns non-zero if profiling data can be added, zero otherwise.
>> + */
>> +int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
>> +{
>> + return (info1->stamp == info2->stamp);
>> +}
>> +
>> +/**
>> + * gcov_info_add - add up profiling data
>> + * @dest: profiling data set to which data is added
>> + * @source: profiling data set which is added
>> + *
>> + * Adds profiling counts of @source to @dest.
>> + */
>> +void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
>> +{
>> + unsigned int active = num_counter_active(dest);
>> + unsigned int f, i, j;
>> +
>> + for (f = 0; f < dest->n_functions; f++) {
>> + for (i = 0; i < active; i++) {
>> + struct gcov_ctr_info *d_ctr =
>> + &dest->functions[f]->ctrs[i];
>> + struct gcov_ctr_info *s_ctr =
>> + &source->functions[f]->ctrs[i];
>> +
>> + for (j = 0; j < d_ctr->num; j++) {
>> + d_ctr->values[j] += s_ctr->values[j];
>> + }
>> + }
>> + }
>> +}
>> +
>> +/**
>> + * gcov_info_dup - duplicate profiling data set
>> + * @info: profiling data set to duplicate
>> + *
>> + * Return newly allocated duplicate on success, %NULL on error.
>> + */
>> +struct gcov_info *gcov_info_dup(struct gcov_info *info)
>> +{
>> + struct gcov_info *dup;
>> + unsigned int active = num_counter_active(info);
>> + unsigned int i, j;
>> +
>> + /* Duplicate gcov_info. */
>> + dup = kzalloc(sizeof(struct gcov_info), GFP_KERNEL);
>> + if (!dup)
>> + return NULL;
>> + dup->version = info->version;
>> + dup->stamp = info->stamp;
>> + dup->n_functions = info->n_functions;
>> + memcpy(dup->merge, info->merge, sizeof(dup->merge));
>> + /* Duplicate filename. */
>> + dup->filename = kstrdup(info->filename, GFP_KERNEL);
>> + if (!dup->filename)
>> + goto err_free;
>> + /* Duplicate table of functions. */
>> + dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
>> + info->n_functions, GFP_KERNEL);
>> + if (!dup->functions)
>> + goto err_free;
>> + for (i = 0; i < info->n_functions; i++) {
>> + struct gcov_fn_info *src_fn = info->functions[i];
>> + struct gcov_fn_info *dest_fn;
>> +
>> + /* Duplicate gcov_fn_info. */
>> + dup->functions[i] = kzalloc(sizeof(struct gcov_fn_info) +
>> + sizeof(struct gcov_ctr_info) *
>> + active, GFP_KERNEL);
>> + if (!dup->functions[i])
>> + goto err_free;
>> + dest_fn = dup->functions[i];
>> + dest_fn->ident = src_fn->ident;
>> + dest_fn->lineno_checksum = src_fn->lineno_checksum;
>> + dest_fn->cfg_checksum = src_fn->cfg_checksum;
>> +
>> + for (j = 0; j < active; j++) {
>> + struct gcov_ctr_info *src_ctr = &src_fn->ctrs[j];
>> + struct gcov_ctr_info *dest_ctr = &dest_fn->ctrs[j];
>> + size_t size = sizeof(gcov_type) * src_ctr->num;
>> +
>> + /* Duplicate gcov_ctr_info. */
>> + dest_ctr->num = src_ctr->num;
>> + dest_ctr->values = vmalloc(size);
> ^^^^^^^
> Does the vmalloc make sense here? The counters are allocated per function, so I
> expect they will be pretty small compared when there was one "huge" array for
> each counter type for all functions per gcov_info. Meaning here we will not
> consume a "big" continuous block of phys memory. I'm just currious and maybe I'm
> missing something. Anyway this is just a nit.
Good point. Previously the sizes to be allocated here could be as large as 64k.
Now the largest I see for 3.10 on an s390x system is ~3k. On the other hand
since we're not on a performance critical path, staying with vmalloc might just
be safer.
>> + if (!dest_ctr->values)
>> + goto err_free;
>> + memcpy(dest_ctr->values, src_ctr->values, size);
>> + }
>> + }
>> + return dup;
>> +
>> +err_free:
>> + gcov_info_free(dup);
>> + return NULL;
>> +}
>> +
>> +/**
>> + * gcov_info_free - release memory for profiling data set duplicate
>> + * @info: profiling data set duplicate to free
>> + */
>> +void gcov_info_free(struct gcov_info *info)
>> +{
>> + unsigned int active = num_counter_active(info);
>> + unsigned int i, j;
>> +
>> + if (info->functions) {
>> + for (i = 0; i < info->n_functions; i++) {
>> + if (!info->functions[i])
>> + continue;
>> + for (j = 0; j < active; j++)
>> + vfree(info->functions[i]->ctrs[j].values);
>> + kfree(info->functions[i]);
>> + }
>> + }
>> + kfree(info->functions);
>> + kfree(info->filename);
>> + kfree(info);
>> +}
>> +
>> +#define ITER_STRIDE PAGE_SIZE
>> +
>> +/**
>> + * struct gcov_iterator - specifies current file position in logical records
>> + * @buffer: buffer containing file data
>> + * @size: size of buffer
>> + * @pos: current position in file
>> + */
>> +struct gcov_iterator {
>> + struct gcov_info *info;
>> + void *buffer;
>> + size_t size;
>> + loff_t pos;
>> +};
>> +
>> +/**
>> + * store_gcov_u32 - store 32 bit number in gcov format to buffer
>> + * @buffer: target buffer or NULL
>> + * @off: offset into the buffer
>> + * @v: value to be stored
>> + *
>> + * Number format defined by gcc: numbers are recorded in the 32 bit
>> + * unsigned binary form of the endianness of the machine generating the
>> + * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
>> + * store anything.
>> + */
>> +static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
>> +{
>> + u32 *data;
>> +
>> + if (buffer) {
>> + data = buffer + off;
>> + *data = v;
>> + }
>> +
>> + return sizeof(*data);
>> +}
>> +
>> +/**
>> + * store_gcov_u64 - store 64 bit number in gcov format to buffer
>> + * @buffer: target buffer or NULL
>> + * @off: offset into the buffer
>> + * @v: value to be stored
>> + *
>> + * Number format defined by gcc: numbers are recorded in the 32 bit
>> + * unsigned binary form of the endianness of the machine generating the
>> + * file. 64 bit numbers are stored as two 32 bit numbers, the low part
>> + * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
>> + * anything.
>> + */
>> +static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
>> +{
>> + u32 *data;
>> +
>> + if (buffer) {
>> + data = buffer + off;
>> +
>> + data[0] = (v & 0xffffffffUL);
>> + data[1] = (v >> 32);
>> + }
>> +
>> + return sizeof(*data) * 2;
>> +}
>> +
>> +/**
>> + * get_ctr_type - return type of specified counter
>> + * @info: profiling data set
>> + * @num: index into list of active counters
>> + *
>> + * Returns the type of the specified counter.
>> + */
>> +static int get_ctr_type(struct gcov_info *info, unsigned int num)
>> +{
>> + unsigned int type;
>> +
>> + for (type = 0; type < GCOV_COUNTERS; type++) {
>> + if (counter_active(info, type)) {
>> + if (num == 0)
>> + break;
>> + num--;
>> + }
>> + }
>> +
>> + return type;
>> +}
>> +
>> +/**
>> + * convert_to_gcda - convert profiling data set to gcda file format
>> + * @buffer: the buffer to store file data or %NULL if no data should be stored
>> + * @info: profiling data set to be converted
>> + *
>> + * Returns the number of bytes that were/would have been stored into the buffer.
>> + */
>> +static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
>> +{
>> + unsigned int active = num_counter_active(info);
>> + unsigned int i, j, k;
>> + size_t pos = 0;
>> +
>> + /* File header. */
>> + pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
>> + pos += store_gcov_u32(buffer, pos, info->version);
>> + pos += store_gcov_u32(buffer, pos, info->stamp);
>> +
>> + for (i = 0; i < info->n_functions; i++) {
>> + struct gcov_fn_info *fn = info->functions[i];
>> +
>> + /* Function record. */
>> + pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
>> + pos += store_gcov_u32(buffer, pos, 3);
>> + pos += store_gcov_u32(buffer, pos, fn->ident);
>> + pos += store_gcov_u32(buffer, pos, fn->lineno_checksum);
>> + pos += store_gcov_u32(buffer, pos, fn->cfg_checksum);
>> +
>> + for (j = 0; j < active; j++) {
>> + struct gcov_ctr_info *ctr = &fn->ctrs[j];
>> + int type = get_ctr_type(info, j);
>> +
>> + /* Counter record. */
>> + pos += store_gcov_u32(buffer, pos,
>> + GCOV_TAG_FOR_COUNTER(type));
>> + pos += store_gcov_u32(buffer, pos, ctr->num * 2);
>> +
>> + for (k = 0; k < ctr->num; k++) {
>> + pos += store_gcov_u64(buffer, pos,
>> + ctr->values[k]);
>> + }
>> + }
>> +
>> +
>> + }
>> +
>> + return pos;
>> +}
>> +
>> +/**
>> + * gcov_iter_new - allocate and initialize profiling data iterator
>> + * @info: profiling data set to be iterated
>> + *
>> + * Return file iterator on success, %NULL otherwise.
>> + */
>> +struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
>> +{
>> + struct gcov_iterator *iter;
>> +
>> + iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
>> + if (!iter)
>> + goto err_free;
>> +
>> + iter->info = info;
>> + /* Dry-run to get the actual buffer size. */
>> + iter->size = convert_to_gcda(NULL, info);
>> + iter->buffer = vmalloc(iter->size);
>> + if (!iter->buffer)
>> + goto err_free;
>> +
>> + convert_to_gcda(iter->buffer, info);
>> +
>> + return iter;
>> +
>> +err_free:
>> + kfree(iter);
>> + return NULL;
>> +}
>> +
>> +/**
>> + * gcov_iter_free - release memory for iterator
>> + * @iter: file iterator to free
>> + */
>> +void gcov_iter_free(struct gcov_iterator *iter)
>> +{
>> + vfree(iter->buffer);
>> + kfree(iter);
>> +}
>> +
>> +/**
>> + * gcov_iter_get_info - return profiling data set for given file iterator
>> + * @iter: file iterator
>> + */
>> +struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
>> +{
>> + return iter->info;
>> +}
>> +
>> +/**
>> + * gcov_iter_start - reset file iterator to starting position
>> + * @iter: file iterator
>> + */
>> +void gcov_iter_start(struct gcov_iterator *iter)
>> +{
>> + iter->pos = 0;
>> +}
>> +
>> +/**
>> + * gcov_iter_next - advance file iterator to next logical record
>> + * @iter: file iterator
>> + *
>> + * Return zero if new position is valid, non-zero if iterator has reached end.
>> + */
>> +int gcov_iter_next(struct gcov_iterator *iter)
>> +{
>> + if (iter->pos < iter->size)
>> + iter->pos += ITER_STRIDE;
>> + if (iter->pos >= iter->size)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * gcov_iter_write - write data for current pos to seq_file
>> + * @iter: file iterator
>> + * @seq: seq_file handle
>> + *
>> + * Return zero on success, non-zero otherwise.
>> + */
>> +int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
>> +{
>> + size_t len;
>> +
>> + if (iter->pos >= iter->size)
>> + return -EINVAL;
>> +
>> + len = ITER_STRIDE;
>> + if (iter->pos + len > iter->size)
>> + len = iter->size - iter->pos;
>> +
>> + seq_write(seq, iter->buffer + iter->pos, len);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * gcov_info_get_filename - return name of the associated gcov data file
>> + * @info: profiling data set
>> + */
>> +const char *gcov_info_get_filename(struct gcov_info *info)
>> +{
>> + return info->filename;
>> +}
>>
>> --
>> Peter Oberparleiter
>> Linux on System z Development - IBM Germany
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 24.08.2013 21:12, Frantisek Hrbata wrote:
> On Fri, Aug 23, 2013 at 05:15:19PM +0200, Peter Oberparleiter wrote:
>> Also it is my understanding that there are some distribution-specific versions
>> of GCC that include the 4.7. gcov format code but report GCC version 4.6. With
>> the auto-detection code implemented like this, gcov-kernel won't work correctly.
>> For that purpose I've implemented a configuration option to allow users to
>> force a specific version of gcov format.
>
> Ah, I was not aware of this inconsistency in versioning. This raises a question
> if it would not be better to deal directly with version in the gcov_info
> instead of these config options. This would of course mean some kind of gcov
> operations callbacks per gcov version(you already mentioned the file
> operations approach).
Using the version information from gcov_info would make the support easier to use,
but I see 2 problems with this approach:
1. How do you decide which version applies to any given gcov_info struct?
Since gcov_info is opaque this would require a compatibility check inside
the version specific code. And each version specific code assumes a different
layout for gcov_info, so in an (unlikely) worst case scenario, the compatibility
check might fail (false positive or access beyond end of data structure) due to
these differences
2. There's no easy way to "force" compatibility, for example in the case
mentioned above where a GCC 4.6 produces GCC 4.7 gcov format data.
>> I'm attaching the corresponding patch below:
>>
>> ---
>> kernel: gcov: make data format configurable
>>
>> Make the format of the generated gcov data configurable. This may be
>> required for example for pre-4.7 GCCs that contain the 4.7 gcov data
>> format changes.
>>
>> Signed-off-by: Peter Oberparleiter <[email protected]>
>> ---
>> kernel/gcov/Kconfig | 30 ++++++++++++++++++++++++++++++
>> kernel/gcov/Makefile | 21 +++++++++++++++++++--
>> 2 files changed, 49 insertions(+), 2 deletions(-)
>>
>> --- a/kernel/gcov/Kconfig
>> +++ b/kernel/gcov/Kconfig
>> @@ -46,4 +46,34 @@ config GCOV_PROFILE_ALL
>> larger and run slower. Also be sure to exclude files from profiling
>> which are not linked to the kernel image to prevent linker errors.
>>
>> +choice
>> + prompt "Specify GCOV format"
>> + depends on GCOV_KERNEL
>> + default GCOV_FORMAT_AUTODETECT
>> + ---help---
>> + The gcov format is usually determined by the GCC version, but there are
>> + exceptions where format changes are integrated in lower-version GCCs.
>> + In such a case use this option to adjust the format used in the kernel
>> + accordingly.
>> +
>> + If unsure, choose "Autodetect".
>> +
>> +config GCOV_FORMAT_AUTODETECT
>> + bool "Autodetect"
>> + ---help---
>> + Select this option to use the format that corresponds to your GCC
>> + version.
>> +
>> +config GCOV_FORMAT_3_4
>> + bool "GCC 3.4 format"
>> + ---help---
>> + Select this option to use the format defined by GCC 3.4.
>> +
>> +config GCOV_FORMAT_4_7
>> + bool "GCC 4.7 format"
>> + ---help---
>> + Select this option to use the format defined by GCC 4.7.
>> +
>> +endchoice
>> +
>> endmenu
>> --- a/kernel/gcov/Makefile
>> +++ b/kernel/gcov/Makefile
>> @@ -1,5 +1,22 @@
>> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
>>
>> +# if-lt
>> +# Usage VAR := $(call if-lt, $(a), $(b))
>> +# Returns 1 if (a < b)
>> +if-lt = $(shell [ $(1) -lt $(2) ] && echo 1)
>> +
>> +ifeq ($(CONFIG_GCOV_FORMAT_3_4),y)
>> + cc-ver := 0304
>> +else ifeq ($(CONFIG_GCOV_FORMAT_4_7),y)
>> + cc-ver := 0407
>> +else
>> + cc-ver := $(call cc-version)
>> +endif
>> +
>> obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
>> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
>> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
>> +
>> +ifeq ($(call if-lt, $(cc-ver), 0407),1)
>> + obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
>> +else
>> + obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
>> +endif
>>
>>
>> --
>> Peter Oberparleiter
>> Linux on System z Development - IBM Germany
>>
>
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 24.08.2013 21:44, Frantisek Hrbata wrote:
> On Fri, Aug 23, 2013 at 05:21:12PM +0200, Peter Oberparleiter wrote:
>> On 23.08.2013 17:15, Peter Oberparleiter wrote:
>>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>>>> Compile the correct gcov implementation file for a specific gcc version. In
>>>> the future, if another file is added, the conditions will need to be somehow
>>>> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
>>>> be enough.
>>
>> As promised, I'm also adding the patch that makes the format-specific part
>> of gcov-kernel a loadable kernel module:
>>
>> ---
>> kernel: gcov: make format-specific code loadable
>>
>> Turn the format-specific part of gcov-kernel into a loadable kernel
>> module. This enables the use of gcov-kernel with kernel modules
>> that were compiled with a version of GCC that produces a different
>> gcov format when compared to the version of GCC that was used to
>> compile the kernel.
>
> If I understand it correctly, this would mean that you will be able to use only
> one implementation of gcov format at the time. Meaning you will be able to get
> coverage data for module, but not for kernel if it was compiled with different
> gcc(gcda format). This is probably ok if you work only on your module, but I'm
> not sure this is generally the right approach. In this case I would probably
> rather see some support for more gcov formats at the same time(e.g. set of
> callback operations per gcov version). Again I'm probably missing something, but
> I still cannot see reason why to add such feature. If you want gcov support just
> compile your kernel and modules with the same gcc version(gcda format). But if
> this is really needed maybe it would be better to consider some parallel support
> for more gcov formats based on the gcov_info version.
The callback approach has other drawbacks (see previous mail).
> Would it be possible to add support for the modified gcc 4.7 gcov format and
> deal with this later? I can incorporate your changes: iter to use buffer,
> .init_array for modules and possibility to explicitly select the gcda format.
> In this case we will have at least the basic support in kernel. This is just me
> thinking out loud.
I think that's an approach I can live with. Maybe the need for a multi-version
support will surface again later in a more refined form, but until then there
should be no reason to delay base GCC 4.7 support any further.
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On 26.08.2013 13:39, LF.Tan wrote:
> Hi all
>
> I am interested to enable Gcov on my platform (on a target board). I
> have tried to apply the 4 patches submitted by Frantisek Hrbata. But,
> the *.gcda is always empty. Note, I am using GCC 4.7.3.
>
> I have followed the steps in Documentation/gcov.txt. I've mounted the
> NFS filesystem and it has access to the kernel build directory on host
> machine.The path for the kernel source in host and target are same.
>
> Let said my kernel module is under drivers/misc,
> drivers/misc/module.ko. Turn on Gcov in drivers/misc/Makefile.
>
> 1. mount -t debugfs none /sys/kernel/debug/
> 2. insmod <PATH>/module.ko
> 3. Perform some actions to excute code in module.ko
> 4. rmmod <PATH>/module.ko
> 5. Check /sys/kernel/debug/gcov/<PATH>/drivers/misc/. There is a soft
> link of module.gcno (I check the soft link is working), but
> module.gcda is empty.
Are you sure they are empty? .gcda files report a file size of 0 as many
virtual files do, but you can read data from them nevertheless. You could
try running "cat" on them to copy their contents for example.
> Any step is incorrect or missing? Or anyone know how to enable GCOV on
> target board? Please advice.
> I would like to enable GCOV in arch/ code as well.
> Thank you very much.
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
Hi Peter
I can get the gcda for the kernel loadable module now. Yes, the
virtual file is with size 0 and I saw the file is empty with 'vi'
command. But, it has data with 'cat' or 'hexdump' command.
Thanks.
On Mon, Aug 26, 2013 at 10:19 PM, Peter Oberparleiter
<[email protected]> wrote:
> On 26.08.2013 13:39, LF.Tan wrote:
>> Hi all
>>
>> I am interested to enable Gcov on my platform (on a target board). I
>> have tried to apply the 4 patches submitted by Frantisek Hrbata. But,
>> the *.gcda is always empty. Note, I am using GCC 4.7.3.
>>
>> I have followed the steps in Documentation/gcov.txt. I've mounted the
>> NFS filesystem and it has access to the kernel build directory on host
>> machine.The path for the kernel source in host and target are same.
>>
>> Let said my kernel module is under drivers/misc,
>> drivers/misc/module.ko. Turn on Gcov in drivers/misc/Makefile.
>>
>> 1. mount -t debugfs none /sys/kernel/debug/
>> 2. insmod <PATH>/module.ko
>> 3. Perform some actions to excute code in module.ko
>> 4. rmmod <PATH>/module.ko
>> 5. Check /sys/kernel/debug/gcov/<PATH>/drivers/misc/. There is a soft
>> link of module.gcno (I check the soft link is working), but
>> module.gcda is empty.
>
> Are you sure they are empty? .gcda files report a file size of 0 as many
> virtual files do, but you can read data from them nevertheless. You could
> try running "cat" on them to copy their contents for example.
>
>> Any step is incorrect or missing? Or anyone know how to enable GCOV on
>> target board? Please advice.
>> I would like to enable GCOV in arch/ code as well.
>> Thank you very much.
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
On Mon, Aug 26, 2013 at 02:56:04PM +0200, Peter Oberparleiter wrote:
> On 24.08.2013 21:12, Frantisek Hrbata wrote:
> > On Fri, Aug 23, 2013 at 05:15:19PM +0200, Peter Oberparleiter wrote:
> >> Also it is my understanding that there are some distribution-specific versions
> >> of GCC that include the 4.7. gcov format code but report GCC version 4.6. With
> >> the auto-detection code implemented like this, gcov-kernel won't work correctly.
> >> For that purpose I've implemented a configuration option to allow users to
> >> force a specific version of gcov format.
> >
> > Ah, I was not aware of this inconsistency in versioning. This raises a question
> > if it would not be better to deal directly with version in the gcov_info
> > instead of these config options. This would of course mean some kind of gcov
> > operations callbacks per gcov version(you already mentioned the file
> > operations approach).
>
> Using the version information from gcov_info would make the support easier to use,
> but I see 2 problems with this approach:
>
> 1. How do you decide which version applies to any given gcov_info struct?
> Since gcov_info is opaque this would require a compatibility check inside
> the version specific code. And each version specific code assumes a different
> layout for gcov_info, so in an (unlikely) worst case scenario, the compatibility
> check might fail (false positive or access beyond end of data structure) due to
> these differences
>
> 2. There's no easy way to "force" compatibility, for example in the case
> mentioned above where a GCC 4.6 produces GCC 4.7 gcov format data.
Yes, you are right of course. Problem imho is that user-space program is always
linked with the correct libgcov. Meaning the gcov structures and the code
dumping the gcda are tied and known(compatible) at the compilation(link) time.
So there is no need to deal with different gcov_info layouts. After a little bit
more thinking the "per gcov_info version callbacks" does not seem as the right
way to go anymore. At this point I think the easier and probably the safest way
would be the module approach as you originally proposed. Anyway I would leave
this aside for now and we will see if someone requests this in the future.
For now I would propose to simply support only one gcov version at the time.
Meaning if you want to use gcov, compile your kernel and modules with the same
gcc version. And add the possibility to explicitly select gcov version(3.4 or
4.7) during configuration as you proposed in your patch.
IMHO this should be a good start :)
>
> >> I'm attaching the corresponding patch below:
> >>
> >> ---
> >> kernel: gcov: make data format configurable
> >>
> >> Make the format of the generated gcov data configurable. This may be
> >> required for example for pre-4.7 GCCs that contain the 4.7 gcov data
> >> format changes.
> >>
> >> Signed-off-by: Peter Oberparleiter <[email protected]>
> >> ---
> >> kernel/gcov/Kconfig | 30 ++++++++++++++++++++++++++++++
> >> kernel/gcov/Makefile | 21 +++++++++++++++++++--
> >> 2 files changed, 49 insertions(+), 2 deletions(-)
> >>
> >> --- a/kernel/gcov/Kconfig
> >> +++ b/kernel/gcov/Kconfig
> >> @@ -46,4 +46,34 @@ config GCOV_PROFILE_ALL
> >> larger and run slower. Also be sure to exclude files from profiling
> >> which are not linked to the kernel image to prevent linker errors.
> >>
> >> +choice
> >> + prompt "Specify GCOV format"
> >> + depends on GCOV_KERNEL
> >> + default GCOV_FORMAT_AUTODETECT
> >> + ---help---
> >> + The gcov format is usually determined by the GCC version, but there are
> >> + exceptions where format changes are integrated in lower-version GCCs.
> >> + In such a case use this option to adjust the format used in the kernel
> >> + accordingly.
> >> +
> >> + If unsure, choose "Autodetect".
> >> +
> >> +config GCOV_FORMAT_AUTODETECT
> >> + bool "Autodetect"
> >> + ---help---
> >> + Select this option to use the format that corresponds to your GCC
> >> + version.
> >> +
> >> +config GCOV_FORMAT_3_4
> >> + bool "GCC 3.4 format"
> >> + ---help---
> >> + Select this option to use the format defined by GCC 3.4.
> >> +
> >> +config GCOV_FORMAT_4_7
> >> + bool "GCC 4.7 format"
> >> + ---help---
> >> + Select this option to use the format defined by GCC 4.7.
> >> +
> >> +endchoice
> >> +
> >> endmenu
> >> --- a/kernel/gcov/Makefile
> >> +++ b/kernel/gcov/Makefile
> >> @@ -1,5 +1,22 @@
> >> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
> >>
> >> +# if-lt
> >> +# Usage VAR := $(call if-lt, $(a), $(b))
> >> +# Returns 1 if (a < b)
> >> +if-lt = $(shell [ $(1) -lt $(2) ] && echo 1)
> >> +
> >> +ifeq ($(CONFIG_GCOV_FORMAT_3_4),y)
> >> + cc-ver := 0304
> >> +else ifeq ($(CONFIG_GCOV_FORMAT_4_7),y)
> >> + cc-ver := 0407
> >> +else
> >> + cc-ver := $(call cc-version)
> >> +endif
> >> +
> >> obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
> >> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
> >> -obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
> >> +
> >> +ifeq ($(call if-lt, $(cc-ver), 0407),1)
> >> + obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
> >> +else
> >> + obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
> >> +endif
> >>
> >>
> >> --
> >> Peter Oberparleiter
> >> Linux on System z Development - IBM Germany
> >>
> >
>
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Mon, Aug 26, 2013 at 04:14:07PM +0200, Peter Oberparleiter wrote:
> On 24.08.2013 21:44, Frantisek Hrbata wrote:
> > On Fri, Aug 23, 2013 at 05:21:12PM +0200, Peter Oberparleiter wrote:
> >> On 23.08.2013 17:15, Peter Oberparleiter wrote:
> >>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> >>>> Compile the correct gcov implementation file for a specific gcc version. In
> >>>> the future, if another file is added, the conditions will need to be somehow
> >>>> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
> >>>> be enough.
> >>
> >> As promised, I'm also adding the patch that makes the format-specific part
> >> of gcov-kernel a loadable kernel module:
> >>
> >> ---
> >> kernel: gcov: make format-specific code loadable
> >>
> >> Turn the format-specific part of gcov-kernel into a loadable kernel
> >> module. This enables the use of gcov-kernel with kernel modules
> >> that were compiled with a version of GCC that produces a different
> >> gcov format when compared to the version of GCC that was used to
> >> compile the kernel.
> >
> > If I understand it correctly, this would mean that you will be able to use only
> > one implementation of gcov format at the time. Meaning you will be able to get
> > coverage data for module, but not for kernel if it was compiled with different
> > gcc(gcda format). This is probably ok if you work only on your module, but I'm
> > not sure this is generally the right approach. In this case I would probably
> > rather see some support for more gcov formats at the same time(e.g. set of
> > callback operations per gcov version). Again I'm probably missing something, but
> > I still cannot see reason why to add such feature. If you want gcov support just
> > compile your kernel and modules with the same gcc version(gcda format). But if
> > this is really needed maybe it would be better to consider some parallel support
> > for more gcov formats based on the gcov_info version.
>
> The callback approach has other drawbacks (see previous mail).
Agreed, I did not realized these problems for the first time when I was thinking
about the callback approach.
>
> > Would it be possible to add support for the modified gcc 4.7 gcov format and
> > deal with this later? I can incorporate your changes: iter to use buffer,
> > .init_array for modules and possibility to explicitly select the gcda format.
> > In this case we will have at least the basic support in kernel. This is just me
> > thinking out loud.
>
> I think that's an approach I can live with. Maybe the need for a multi-version
> support will surface again later in a more refined form, but until then there
> should be no reason to delay base GCC 4.7 support any further.
Great. I can incorporate the changes you proposed and post V2. Or do you prefer
to post it by your self? Based on the info and patches you already provided I
guess you already have something ready. Simply what suits you best :).
Many thanks Peter
>
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On Mon, Aug 26, 2013 at 02:45:10PM +0200, Peter Oberparleiter wrote:
> On 23.08.2013 23:00, Frantisek Hrbata wrote:
> > On Fri, Aug 23, 2013 at 05:12:19PM +0200, Peter Oberparleiter wrote:
> >> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> >>> The gcov in-memory format changed in gcc 4.7. The biggest change, which
> >>> requires this special implementation, is that gcov_info no longer contains
> >>> array of counters for each counter type for all functions and gcov_fn_info is
> >>> not used for mapping of function's counters to these arrays(offset). Now each
> >>> gcov_fn_info contans it's counters, which makes things a little bit easier.
> >>
> >> By now I've come to the conclusion that I may have over-engineered
> >> gcov_iter_next in the original GCC 3.4 implementation. This became especially
> >> apparent when someone tried to adjust that code to also work with a specific
> >> android GCC version - adding a simple field to the output file format required
> >> major complex changes.
> >>
> >> Therefore in my version of the GCC 4.7 support, I opted to replace this logic
> >> with the simpler approach of generating the gcov data file in-memory during
> >> open (gcov_iter_new) and reducing gcov_iter_next/gcov_iter_write to simple
> >> copy-from-buffer functions.
> >
> > Yes, this seems reasonable and much easier approach to me, but we will end up
> > with three copies of one gcov(gcda) file data in memory: data from gcc, our
> > buffer and the buffer in seq file. But I guess this is not a big problem,
> > unless someone opens a huge amount of gcda files in parallel. Generally I
> > like this idea. This will be also probably faster then writing 1 or 2 ints per
> > one iter write.
>
> You're right about the data being in multiple buffers for the time that a .gcda
> file is opened, but as you mentioned, this shouldn't be a problem since usually
> only one .gcda file is open at a time. Also the reduction in code complexity is
> in my opinion well worth it.
agreed
>
> >> Since I can see the need to change the format code again in the future,
> >> I think it would be easier to simplify this part of the code correspondingly.
> >> I'm adding my version of this part of the implementation as discussion input
> >> below (the relevant part starts at convert_to_gcda()).
> >
> > I have really only two nits to the code(please see below). I spent some time
> > checking the new seq/iter implementation and it seems ok to me. Generally I like
> > how simple this change is done.
> >
> >>
> >> ---
> >> kernel: gcov: add support for gcc 4.7 gcov format
> >>
> >> GCC 4.7 changed the format of gcov related data structures, both
> >> on-disk and in memory. Add support for the new format.
> >>
> >> Signed-off-by: Peter Oberparleiter <[email protected]>
> >> ---
> >> kernel/gcov/Makefile | 4
> >> kernel/gcov/gcc_4_7.c | 510 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 513 insertions(+), 1 deletion(-)
> >>
> >> --- a/kernel/gcov/Makefile
> >> +++ b/kernel/gcov/Makefile
> >> @@ -1,3 +1,5 @@
> >> ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
> >>
> >> -obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
> >> +obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
> >> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -lt, 0407, gcc_3_4.o)
> >> +obj-$(CONFIG_GCOV_KERNEL) += $(call cc-ifversion, -ge, 0407, gcc_4_7.o)
> >> --- /dev/null
> >> +++ b/kernel/gcov/gcc_4_7.c
> >> @@ -0,0 +1,510 @@
> >> +/*
> >> + * This code provides functions to handle gcc's profiling data format
> >> + * introduced with gcc 4.7. Future versions of gcc may change the gcov
> >> + * format (as happened before), so all format-specific information needs
> >> + * to be kept modular and easily exchangeable.
> >> + *
> >> + * This file is based on gcc-internal definitions. Functions and data
> >> + * structures are defined to be compatible with gcc counterparts.
> >> + * For a better understanding, refer to gcc source: gcc/gcov-io.h.
> >> + *
> >> + * Copyright IBM Corp. 2013
> >> + * Author(s): Peter Oberparleiter <[email protected]>
> >> + *
> >> + * Uses gcc-internal data definitions.
> >> + */
> >> +
> >> +#include <linux/errno.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/string.h>
> >> +#include <linux/seq_file.h>
> >> +#include <linux/vmalloc.h>
> >> +#include <linux/types.h>
> >> +#include "gcov.h"
> >> +
> >> +/*
> >> + * Profiling data types used for gcc 4.7 and above - these are defined by
> >> + * gcc and need to be kept as close to the original definition as possible to
> >> + * remain compatible.
> >> + */
> >> +#define GCOV_COUNTERS 8
> >> +#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
> >> +#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
> >> +#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
> >> +#define GCOV_TAG_FOR_COUNTER(count) \
> >> + (GCOV_TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
> >> +
> >> +#if BITS_PER_LONG >= 64
> >> +typedef long gcov_type;
> >> +#else
> >> +typedef long long gcov_type;
> >> +#endif
> >> +
> >> +/**
> >> + * struct gcov_ctr_info - profiling data per function and counter type
> >> + * @num: number of counter values for this type
> >> + * @values: array of counter values for this type
> >> + *
> >> + * This data is generated by gcc during compilation and doesn't change
> >> + * at run-time with the exception of the values array.
> >> + */
> >> +struct gcov_ctr_info {
> >> + unsigned int num;
> >> + gcov_type *values;
> >> +};
> >> +
> >> +/**
> >> + * struct gcov_fn_info - profiling meta data per function
> >> + * @key: comdat key
> >> + * @ident: object file-unique function identifier
> >> + * @lineno_checksum: function lineno_checksum
> >> + * @cfg_checksum: function cfg_checksum
> >> + * @ctrs: counters
> >> + *
> >> + * This data is generated by gcc during compilation and doesn't change
> >> + * at run-time.
> >> + */
> >> +struct gcov_fn_info {
> >> + const struct gcov_info *key;
> >> + unsigned int ident;
> >> + unsigned int lineno_checksum;
> >> + unsigned int cfg_checksum;
> >> + struct gcov_ctr_info ctrs[0];
> >> +};
> >> +
> >> +typedef void (*gcov_merge_fn)(gcov_type *, unsigned int);
> >> +
> >> +/**
> >> + * struct gcov_info - profiling data per object file
> >> + * @version: gcov version magic indicating the gcc version used for compilation
> >> + * @next: list head for a singly-linked list
> >> + * @stamp: time stamp
> >> + * @filename: name of the associated gcov data file
> >> + * @merge: functions for merging counts per counter type or null for unused
> >> + * @n_functions: number of instrumented functions
> >> + * @functions: function data
> >> + *
> >> + * This data is generated by gcc during compilation and doesn't change
> >> + * at run-time with the exception of the next pointer.
> >> + */
> >> +struct gcov_info {
> >> + unsigned int version;
> >> + struct gcov_info *next;
> >> + unsigned int stamp;
> >> + const char *filename;
> >> + gcov_merge_fn merge[GCOV_COUNTERS];
> >> + unsigned int n_functions;
> >> + struct gcov_fn_info **functions;
> >
> > I can see that you removed the const part. I was thinking about it too, because
> > it requires ugly cast-const-away code in the gcov_info_dup and it is imho not
> > necessary, but I left the original definition from gcc anyway.
>
> The kernel's usage of gcov_info is slightly different from the GCC's internal
> one, so a slight divergence should be ok from my point of view, especially
> considering that the outcome (dropping a const declaration) is the same.
agreed
>
> >
> >> +};
> >> +
> >> +/* Symbolic links to be created for each profiling data file. */
> >> +const struct gcov_link gcov_link[] = {
> >> + { OBJ_TREE, "gcno" }, /* Link to .gcno file in $(objtree). */
> >> + { 0, NULL},
> >> +};
> >> +
> >> +/*
> >> + * Determine whether a counter is active. Based on gcc magic. Doesn't change
> >> + * at run-time.
> >> + */
> >> +static int counter_active(struct gcov_info *info, unsigned int type)
> >> +{
> >> + return info->merge[type] != NULL;
> >> +}
> >> +
> >> +/* Determine number of active counters. Based on gcc magic. */
> >> +static unsigned int num_counter_active(struct gcov_info *info)
> >> +{
> >> + unsigned int i;
> >> + unsigned int result = 0;
> >> +
> >> + for (i = 0; i < GCOV_COUNTERS; i++) {
> >> + if (counter_active(info, i))
> >> + result++;
> >> + }
> >> + return result;
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_reset - reset profiling data to zero
> >> + * @info: profiling data set
> >> + */
> >> +void gcov_info_reset(struct gcov_info *info)
> >> +{
> >> + unsigned int active = num_counter_active(info);
> >> + unsigned int f, i;
> >> + struct gcov_ctr_info *ctr;
> >> +
> >> + for (f = 0; f < info->n_functions; f++) {
> >> + for (i = 0; i < active; i++) {
> >> + ctr = &info->functions[f]->ctrs[i];
> >> + memset(ctr->values, 0, ctr->num * sizeof(gcov_type));
> >> + }
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_is_compatible - check if profiling data can be added
> >> + * @info1: first profiling data set
> >> + * @info2: second profiling data set
> >> + *
> >> + * Returns non-zero if profiling data can be added, zero otherwise.
> >> + */
> >> +int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
> >> +{
> >> + return (info1->stamp == info2->stamp);
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_add - add up profiling data
> >> + * @dest: profiling data set to which data is added
> >> + * @source: profiling data set which is added
> >> + *
> >> + * Adds profiling counts of @source to @dest.
> >> + */
> >> +void gcov_info_add(struct gcov_info *dest, struct gcov_info *source)
> >> +{
> >> + unsigned int active = num_counter_active(dest);
> >> + unsigned int f, i, j;
> >> +
> >> + for (f = 0; f < dest->n_functions; f++) {
> >> + for (i = 0; i < active; i++) {
> >> + struct gcov_ctr_info *d_ctr =
> >> + &dest->functions[f]->ctrs[i];
> >> + struct gcov_ctr_info *s_ctr =
> >> + &source->functions[f]->ctrs[i];
> >> +
> >> + for (j = 0; j < d_ctr->num; j++) {
> >> + d_ctr->values[j] += s_ctr->values[j];
> >> + }
> >> + }
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_dup - duplicate profiling data set
> >> + * @info: profiling data set to duplicate
> >> + *
> >> + * Return newly allocated duplicate on success, %NULL on error.
> >> + */
> >> +struct gcov_info *gcov_info_dup(struct gcov_info *info)
> >> +{
> >> + struct gcov_info *dup;
> >> + unsigned int active = num_counter_active(info);
> >> + unsigned int i, j;
> >> +
> >> + /* Duplicate gcov_info. */
> >> + dup = kzalloc(sizeof(struct gcov_info), GFP_KERNEL);
> >> + if (!dup)
> >> + return NULL;
> >> + dup->version = info->version;
> >> + dup->stamp = info->stamp;
> >> + dup->n_functions = info->n_functions;
> >> + memcpy(dup->merge, info->merge, sizeof(dup->merge));
> >> + /* Duplicate filename. */
> >> + dup->filename = kstrdup(info->filename, GFP_KERNEL);
> >> + if (!dup->filename)
> >> + goto err_free;
> >> + /* Duplicate table of functions. */
> >> + dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
> >> + info->n_functions, GFP_KERNEL);
> >> + if (!dup->functions)
> >> + goto err_free;
> >> + for (i = 0; i < info->n_functions; i++) {
> >> + struct gcov_fn_info *src_fn = info->functions[i];
> >> + struct gcov_fn_info *dest_fn;
> >> +
> >> + /* Duplicate gcov_fn_info. */
> >> + dup->functions[i] = kzalloc(sizeof(struct gcov_fn_info) +
> >> + sizeof(struct gcov_ctr_info) *
> >> + active, GFP_KERNEL);
> >> + if (!dup->functions[i])
> >> + goto err_free;
> >> + dest_fn = dup->functions[i];
> >> + dest_fn->ident = src_fn->ident;
> >> + dest_fn->lineno_checksum = src_fn->lineno_checksum;
> >> + dest_fn->cfg_checksum = src_fn->cfg_checksum;
> >> +
> >> + for (j = 0; j < active; j++) {
> >> + struct gcov_ctr_info *src_ctr = &src_fn->ctrs[j];
> >> + struct gcov_ctr_info *dest_ctr = &dest_fn->ctrs[j];
> >> + size_t size = sizeof(gcov_type) * src_ctr->num;
> >> +
> >> + /* Duplicate gcov_ctr_info. */
> >> + dest_ctr->num = src_ctr->num;
> >> + dest_ctr->values = vmalloc(size);
> > ^^^^^^^
> > Does the vmalloc make sense here? The counters are allocated per function, so I
> > expect they will be pretty small compared when there was one "huge" array for
> > each counter type for all functions per gcov_info. Meaning here we will not
> > consume a "big" continuous block of phys memory. I'm just currious and maybe I'm
> > missing something. Anyway this is just a nit.
>
> Good point. Previously the sizes to be allocated here could be as large as 64k.
> Now the largest I see for 3.10 on an s390x system is ~3k. On the other hand
> since we're not on a performance critical path, staying with vmalloc might just
> be safer.
agreed, this makes sense
>
> >> + if (!dest_ctr->values)
> >> + goto err_free;
> >> + memcpy(dest_ctr->values, src_ctr->values, size);
> >> + }
> >> + }
> >> + return dup;
> >> +
> >> +err_free:
> >> + gcov_info_free(dup);
> >> + return NULL;
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_free - release memory for profiling data set duplicate
> >> + * @info: profiling data set duplicate to free
> >> + */
> >> +void gcov_info_free(struct gcov_info *info)
> >> +{
> >> + unsigned int active = num_counter_active(info);
> >> + unsigned int i, j;
> >> +
> >> + if (info->functions) {
> >> + for (i = 0; i < info->n_functions; i++) {
> >> + if (!info->functions[i])
> >> + continue;
> >> + for (j = 0; j < active; j++)
> >> + vfree(info->functions[i]->ctrs[j].values);
> >> + kfree(info->functions[i]);
> >> + }
> >> + }
> >> + kfree(info->functions);
> >> + kfree(info->filename);
> >> + kfree(info);
> >> +}
> >> +
> >> +#define ITER_STRIDE PAGE_SIZE
> >> +
> >> +/**
> >> + * struct gcov_iterator - specifies current file position in logical records
> >> + * @buffer: buffer containing file data
> >> + * @size: size of buffer
> >> + * @pos: current position in file
> >> + */
> >> +struct gcov_iterator {
> >> + struct gcov_info *info;
> >> + void *buffer;
> >> + size_t size;
> >> + loff_t pos;
> >> +};
> >> +
> >> +/**
> >> + * store_gcov_u32 - store 32 bit number in gcov format to buffer
> >> + * @buffer: target buffer or NULL
> >> + * @off: offset into the buffer
> >> + * @v: value to be stored
> >> + *
> >> + * Number format defined by gcc: numbers are recorded in the 32 bit
> >> + * unsigned binary form of the endianness of the machine generating the
> >> + * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
> >> + * store anything.
> >> + */
> >> +static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
> >> +{
> >> + u32 *data;
> >> +
> >> + if (buffer) {
> >> + data = buffer + off;
> >> + *data = v;
> >> + }
> >> +
> >> + return sizeof(*data);
> >> +}
> >> +
> >> +/**
> >> + * store_gcov_u64 - store 64 bit number in gcov format to buffer
> >> + * @buffer: target buffer or NULL
> >> + * @off: offset into the buffer
> >> + * @v: value to be stored
> >> + *
> >> + * Number format defined by gcc: numbers are recorded in the 32 bit
> >> + * unsigned binary form of the endianness of the machine generating the
> >> + * file. 64 bit numbers are stored as two 32 bit numbers, the low part
> >> + * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
> >> + * anything.
> >> + */
> >> +static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
> >> +{
> >> + u32 *data;
> >> +
> >> + if (buffer) {
> >> + data = buffer + off;
> >> +
> >> + data[0] = (v & 0xffffffffUL);
> >> + data[1] = (v >> 32);
> >> + }
> >> +
> >> + return sizeof(*data) * 2;
> >> +}
> >> +
> >> +/**
> >> + * get_ctr_type - return type of specified counter
> >> + * @info: profiling data set
> >> + * @num: index into list of active counters
> >> + *
> >> + * Returns the type of the specified counter.
> >> + */
> >> +static int get_ctr_type(struct gcov_info *info, unsigned int num)
> >> +{
> >> + unsigned int type;
> >> +
> >> + for (type = 0; type < GCOV_COUNTERS; type++) {
> >> + if (counter_active(info, type)) {
> >> + if (num == 0)
> >> + break;
> >> + num--;
> >> + }
> >> + }
> >> +
> >> + return type;
> >> +}
> >> +
> >> +/**
> >> + * convert_to_gcda - convert profiling data set to gcda file format
> >> + * @buffer: the buffer to store file data or %NULL if no data should be stored
> >> + * @info: profiling data set to be converted
> >> + *
> >> + * Returns the number of bytes that were/would have been stored into the buffer.
> >> + */
> >> +static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
> >> +{
> >> + unsigned int active = num_counter_active(info);
> >> + unsigned int i, j, k;
> >> + size_t pos = 0;
> >> +
> >> + /* File header. */
> >> + pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
> >> + pos += store_gcov_u32(buffer, pos, info->version);
> >> + pos += store_gcov_u32(buffer, pos, info->stamp);
> >> +
> >> + for (i = 0; i < info->n_functions; i++) {
> >> + struct gcov_fn_info *fn = info->functions[i];
> >> +
> >> + /* Function record. */
> >> + pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
> >> + pos += store_gcov_u32(buffer, pos, 3);
> >> + pos += store_gcov_u32(buffer, pos, fn->ident);
> >> + pos += store_gcov_u32(buffer, pos, fn->lineno_checksum);
> >> + pos += store_gcov_u32(buffer, pos, fn->cfg_checksum);
> >> +
> >> + for (j = 0; j < active; j++) {
> >> + struct gcov_ctr_info *ctr = &fn->ctrs[j];
> >> + int type = get_ctr_type(info, j);
> >> +
> >> + /* Counter record. */
> >> + pos += store_gcov_u32(buffer, pos,
> >> + GCOV_TAG_FOR_COUNTER(type));
> >> + pos += store_gcov_u32(buffer, pos, ctr->num * 2);
> >> +
> >> + for (k = 0; k < ctr->num; k++) {
> >> + pos += store_gcov_u64(buffer, pos,
> >> + ctr->values[k]);
> >> + }
> >> + }
> >> +
> >> +
> >> + }
> >> +
> >> + return pos;
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_new - allocate and initialize profiling data iterator
> >> + * @info: profiling data set to be iterated
> >> + *
> >> + * Return file iterator on success, %NULL otherwise.
> >> + */
> >> +struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
> >> +{
> >> + struct gcov_iterator *iter;
> >> +
> >> + iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
> >> + if (!iter)
> >> + goto err_free;
> >> +
> >> + iter->info = info;
> >> + /* Dry-run to get the actual buffer size. */
> >> + iter->size = convert_to_gcda(NULL, info);
> >> + iter->buffer = vmalloc(iter->size);
> >> + if (!iter->buffer)
> >> + goto err_free;
> >> +
> >> + convert_to_gcda(iter->buffer, info);
> >> +
> >> + return iter;
> >> +
> >> +err_free:
> >> + kfree(iter);
> >> + return NULL;
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_free - release memory for iterator
> >> + * @iter: file iterator to free
> >> + */
> >> +void gcov_iter_free(struct gcov_iterator *iter)
> >> +{
> >> + vfree(iter->buffer);
> >> + kfree(iter);
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_get_info - return profiling data set for given file iterator
> >> + * @iter: file iterator
> >> + */
> >> +struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
> >> +{
> >> + return iter->info;
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_start - reset file iterator to starting position
> >> + * @iter: file iterator
> >> + */
> >> +void gcov_iter_start(struct gcov_iterator *iter)
> >> +{
> >> + iter->pos = 0;
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_next - advance file iterator to next logical record
> >> + * @iter: file iterator
> >> + *
> >> + * Return zero if new position is valid, non-zero if iterator has reached end.
> >> + */
> >> +int gcov_iter_next(struct gcov_iterator *iter)
> >> +{
> >> + if (iter->pos < iter->size)
> >> + iter->pos += ITER_STRIDE;
> >> + if (iter->pos >= iter->size)
> >> + return -EINVAL;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * gcov_iter_write - write data for current pos to seq_file
> >> + * @iter: file iterator
> >> + * @seq: seq_file handle
> >> + *
> >> + * Return zero on success, non-zero otherwise.
> >> + */
> >> +int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
> >> +{
> >> + size_t len;
> >> +
> >> + if (iter->pos >= iter->size)
> >> + return -EINVAL;
> >> +
> >> + len = ITER_STRIDE;
> >> + if (iter->pos + len > iter->size)
> >> + len = iter->size - iter->pos;
> >> +
> >> + seq_write(seq, iter->buffer + iter->pos, len);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * gcov_info_get_filename - return name of the associated gcov data file
> >> + * @info: profiling data set
> >> + */
> >> +const char *gcov_info_get_filename(struct gcov_info *info)
> >> +{
> >> + return info->filename;
> >> +}
> >>
> >> --
> >> Peter Oberparleiter
> >> Linux on System z Development - IBM Germany
>
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata
On 27.08.2013 15:34, Frantisek Hrbata wrote:
> On Mon, Aug 26, 2013 at 04:14:07PM +0200, Peter Oberparleiter wrote:
>> On 24.08.2013 21:44, Frantisek Hrbata wrote:
>>> On Fri, Aug 23, 2013 at 05:21:12PM +0200, Peter Oberparleiter wrote:
>>>> On 23.08.2013 17:15, Peter Oberparleiter wrote:
>>>>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
>>>>>> Compile the correct gcov implementation file for a specific gcc version. In
>>>>>> the future, if another file is added, the conditions will need to be somehow
>>>>>> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
>>>>>> be enough.
>>>>
>>>> As promised, I'm also adding the patch that makes the format-specific part
>>>> of gcov-kernel a loadable kernel module:
>>>>
>>>> ---
>>>> kernel: gcov: make format-specific code loadable
>>>>
>>>> Turn the format-specific part of gcov-kernel into a loadable kernel
>>>> module. This enables the use of gcov-kernel with kernel modules
>>>> that were compiled with a version of GCC that produces a different
>>>> gcov format when compared to the version of GCC that was used to
>>>> compile the kernel.
>>>
>>> If I understand it correctly, this would mean that you will be able to use only
>>> one implementation of gcov format at the time. Meaning you will be able to get
>>> coverage data for module, but not for kernel if it was compiled with different
>>> gcc(gcda format). This is probably ok if you work only on your module, but I'm
>>> not sure this is generally the right approach. In this case I would probably
>>> rather see some support for more gcov formats at the same time(e.g. set of
>>> callback operations per gcov version). Again I'm probably missing something, but
>>> I still cannot see reason why to add such feature. If you want gcov support just
>>> compile your kernel and modules with the same gcc version(gcda format). But if
>>> this is really needed maybe it would be better to consider some parallel support
>>> for more gcov formats based on the gcov_info version.
>>
>> The callback approach has other drawbacks (see previous mail).
>
> Agreed, I did not realized these problems for the first time when I was thinking
> about the callback approach.
>
>>
>>> Would it be possible to add support for the modified gcc 4.7 gcov format and
>>> deal with this later? I can incorporate your changes: iter to use buffer,
>>> .init_array for modules and possibility to explicitly select the gcda format.
>>> In this case we will have at least the basic support in kernel. This is just me
>>> thinking out loud.
>>
>> I think that's an approach I can live with. Maybe the need for a multi-version
>> support will surface again later in a more refined form, but until then there
>> should be no reason to delay base GCC 4.7 support any further.
>
> Great. I can incorporate the changes you proposed and post V2. Or do you prefer
> to post it by your self? Based on the info and patches you already provided I
> guess you already have something ready. Simply what suits you best :).
If it's not too much hassle, I'd prefer that you post V2 of your patches. Also
I guess it would make sense to put Andrew Morton on CC since the initial
gcov-patches were picked up by him.
--
Peter Oberparleiter
Linux on System z Development - IBM Germany
On Wed, Aug 28, 2013 at 03:46:05PM +0200, Peter Oberparleiter wrote:
> On 27.08.2013 15:34, Frantisek Hrbata wrote:
> > On Mon, Aug 26, 2013 at 04:14:07PM +0200, Peter Oberparleiter wrote:
> >> On 24.08.2013 21:44, Frantisek Hrbata wrote:
> >>> On Fri, Aug 23, 2013 at 05:21:12PM +0200, Peter Oberparleiter wrote:
> >>>> On 23.08.2013 17:15, Peter Oberparleiter wrote:
> >>>>> On 23.08.2013 10:39, Frantisek Hrbata wrote:
> >>>>>> Compile the correct gcov implementation file for a specific gcc version. In
> >>>>>> the future, if another file is added, the conditions will need to be somehow
> >>>>>> adjusted to if-elif-else case, but at this point the simple cc-ifversion should
> >>>>>> be enough.
> >>>>
> >>>> As promised, I'm also adding the patch that makes the format-specific part
> >>>> of gcov-kernel a loadable kernel module:
> >>>>
> >>>> ---
> >>>> kernel: gcov: make format-specific code loadable
> >>>>
> >>>> Turn the format-specific part of gcov-kernel into a loadable kernel
> >>>> module. This enables the use of gcov-kernel with kernel modules
> >>>> that were compiled with a version of GCC that produces a different
> >>>> gcov format when compared to the version of GCC that was used to
> >>>> compile the kernel.
> >>>
> >>> If I understand it correctly, this would mean that you will be able to use only
> >>> one implementation of gcov format at the time. Meaning you will be able to get
> >>> coverage data for module, but not for kernel if it was compiled with different
> >>> gcc(gcda format). This is probably ok if you work only on your module, but I'm
> >>> not sure this is generally the right approach. In this case I would probably
> >>> rather see some support for more gcov formats at the same time(e.g. set of
> >>> callback operations per gcov version). Again I'm probably missing something, but
> >>> I still cannot see reason why to add such feature. If you want gcov support just
> >>> compile your kernel and modules with the same gcc version(gcda format). But if
> >>> this is really needed maybe it would be better to consider some parallel support
> >>> for more gcov formats based on the gcov_info version.
> >>
> >> The callback approach has other drawbacks (see previous mail).
> >
> > Agreed, I did not realized these problems for the first time when I was thinking
> > about the callback approach.
> >
> >>
> >>> Would it be possible to add support for the modified gcc 4.7 gcov format and
> >>> deal with this later? I can incorporate your changes: iter to use buffer,
> >>> .init_array for modules and possibility to explicitly select the gcda format.
> >>> In this case we will have at least the basic support in kernel. This is just me
> >>> thinking out loud.
> >>
> >> I think that's an approach I can live with. Maybe the need for a multi-version
> >> support will surface again later in a more refined form, but until then there
> >> should be no reason to delay base GCC 4.7 support any further.
> >
> > Great. I can incorporate the changes you proposed and post V2. Or do you prefer
> > to post it by your self? Based on the info and patches you already provided I
> > guess you already have something ready. Simply what suits you best :).
>
> If it's not too much hassle, I'd prefer that you post V2 of your patches. Also
> I guess it would make sense to put Andrew Morton on CC since the initial
> gcov-patches were picked up by him.
Ok, no problem. I will prepare v2. Again many thanks Peter for your help and
input. It's really appreciated.
>
> --
> Peter Oberparleiter
> Linux on System z Development - IBM Germany
>
--
Frantisek Hrbata