PGO profile data is exported from the kernel by
creating debugfs files: pgo/<module>.profraw for each module.
Modules are register into the profiler via module notifier callback
similiar to gcov/base.c. Note that if module does not have __llvm_prf_xxx
sections required the module ignored.
Also there is no "reset" support for yet for these files.
Signed-off-by: Jarmo Tiitto <[email protected]>
---
kernel/pgo/Makefile | 2 +-
kernel/pgo/fs.c | 54 ++++--
kernel/pgo/fs_mod.c | 415 ++++++++++++++++++++++++++++++++++++++++
kernel/pgo/instrument.c | 12 +-
kernel/pgo/pgo.h | 11 +-
5 files changed, 466 insertions(+), 28 deletions(-)
create mode 100644 kernel/pgo/fs_mod.c
diff --git a/kernel/pgo/Makefile b/kernel/pgo/Makefile
index 41e27cefd9a4..662b7dfdfbe9 100644
--- a/kernel/pgo/Makefile
+++ b/kernel/pgo/Makefile
@@ -2,4 +2,4 @@
GCOV_PROFILE := n
PGO_PROFILE := n
-obj-y += fs.o instrument.o
+obj-y += fs.o fs_mod.o instrument.o
diff --git a/kernel/pgo/fs.c b/kernel/pgo/fs.c
index 575142735273..5471d270a5bb 100644
--- a/kernel/pgo/fs.c
+++ b/kernel/pgo/fs.c
@@ -227,15 +227,15 @@ static unsigned long prf_buffer_size(void)
* Serialize the profiling data into a format LLVM's tools can understand.
* Note: caller *must* hold pgo_lock.
*/
-static int prf_serialize(struct prf_private_data *p)
+static int prf_serialize(struct prf_private_data *p, unsigned long *buf_size)
{
int err = 0;
void *buffer;
- p->size = prf_buffer_size();
- p->buffer = vzalloc(p->size);
+ /* re-check buffer size */
+ *buf_size = prf_buffer_size();
- if (!p->buffer) {
+ if (p->size < *buf_size || !p->buffer) {
err = -ENOMEM;
goto out;
}
@@ -262,7 +262,8 @@ static int prf_open(struct inode *inode, struct file *file)
{
struct prf_private_data *data;
unsigned long flags;
- int err;
+ unsigned long buf_size;
+ int err = 0;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data) {
@@ -270,18 +271,41 @@ static int prf_open(struct inode *inode, struct file *file)
goto out;
}
+ /* estimate amount of memory needed:
+ * can't vzalloc() while prf_lock() is held:
+ * CONFIG_DEBUG_ATOMIC_SLEEP complains.
+ * So first get buffer size, release the lock,
+ * vzalloc(), re-lock and try serialize.
+ */
flags = prf_lock();
+ buf_size = prf_buffer_size();
- err = prf_serialize(data);
- if (unlikely(err)) {
- kfree(data);
- goto out_unlock;
- }
+ do {
+ prf_unlock(flags);
- file->private_data = data;
+ /* resize buffer */
+ if (data->size < buf_size && data->buffer) {
+ vfree(data->buffer);
+ data->buffer = NULL;
+ }
+
+ if (!data->buffer) {
+ data->size = buf_size;
+ data->buffer = vzalloc(data->size);
-out_unlock:
+ if (!data->buffer) {
+ err = -ENOMEM;
+ goto out;
+ }
+ }
+ /* try serialize */
+ flags = prf_lock();
+ } while (prf_serialize(data, &buf_size));
prf_unlock(flags);
+
+ data->size = buf_size;
+ file->private_data = data;
+
out:
return err;
}
@@ -363,6 +387,8 @@ static const struct file_operations prf_reset_fops = {
/* Create debugfs entries. */
static int __init pgo_init(void)
{
+ pr_notice("Clang PGO profile data available.");
+
directory = debugfs_create_dir("pgo", NULL);
if (!directory)
goto err_remove;
@@ -375,6 +401,8 @@ static int __init pgo_init(void)
&prf_reset_fops))
goto err_remove;
+ prf_modules_init();
+
return 0;
err_remove:
@@ -385,6 +413,8 @@ static int __init pgo_init(void)
/* Remove debugfs entries. */
static void __exit pgo_exit(void)
{
+ prf_modules_exit();
+
debugfs_remove_recursive(directory);
}
diff --git a/kernel/pgo/fs_mod.c b/kernel/pgo/fs_mod.c
new file mode 100644
index 000000000000..0808d44227f1
--- /dev/null
+++ b/kernel/pgo/fs_mod.c
@@ -0,0 +1,415 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Jarmo Tiitto
+ *
+ * Author:
+ * Jarmo Tiitto <[email protected]>
+ *
+ * Based on the clang PGO kernel patch by:
+ * Sami Tolvanen <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#define pr_fmt(fmt) "pgo: " fmt
+
+#include <linux/kernel.h>
+#include <linux/debugfs.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include "pgo.h"
+
+/*
+ * PGO profile data reporting for modules:
+ * We maintain one debugfs pgo/<module>.profraw file per module.
+ */
+
+
+DEFINE_MUTEX(prf_mod_lock);
+LIST_HEAD(prf_mod_list);
+
+struct prf_mod_data {
+ void *buffer;
+ unsigned long size;
+};
+
+/* these are trivial, but each one differs a bit */
+static inline unsigned long prf_mod_data_size(struct module *mod)
+{
+ return mod->prf_data_size;
+}
+
+static inline unsigned long prf_mod_data_count(struct module *mod)
+{
+ return mod->prf_data_size / sizeof(struct llvm_prf_data);
+}
+
+static inline unsigned long prf_mod_cnts_size(struct module *mod)
+{
+ return mod->prf_cnts_num * sizeof(mod->prf_cnts[0]);
+}
+
+static inline unsigned long prf_mod_cnts_count(struct module *mod)
+{
+ return mod->prf_cnts_num;
+}
+
+static inline unsigned long prf_mod_names_size(struct module *mod)
+{
+ return mod->prf_names_num * sizeof(mod->prf_names[0]);
+}
+
+static inline unsigned long prf_mod_names_count(struct module *mod)
+{
+ return mod->prf_names_num;
+}
+
+static inline unsigned long prf_mod_vnds_size(struct module *mod)
+{
+ return mod->prf_vnds_size;
+}
+
+static inline unsigned long prf_mod_vnds_count(struct module *mod)
+{
+ return mod->prf_vnds_size / sizeof(struct llvm_prf_value_node);
+}
+
+/*
+ * Raw profile data format:
+ *
+ * - llvm_prf_header
+ * - __llvm_prf_data
+ * - __llvm_prf_cnts
+ * - __llvm_prf_names
+ * - zero padding to 8 bytes
+ * - for each llvm_prf_data in __llvm_prf_data:
+ * - llvm_prf_value_data
+ * - llvm_prf_value_record + site count array
+ * - llvm_prf_value_node_data
+ * ...
+ * ...
+ * ...
+ */
+
+static void prf_module_fill_header(struct module *mod, void **buffer)
+{
+ struct llvm_prf_header *header = *(struct llvm_prf_header **)buffer;
+
+#ifdef CONFIG_64BIT
+ header->magic = LLVM_INSTR_PROF_RAW_MAGIC_64;
+#else
+ header->magic = LLVM_INSTR_PROF_RAW_MAGIC_32;
+#endif
+ header->version = LLVM_VARIANT_MASK_IR_PROF | LLVM_INSTR_PROF_RAW_VERSION;
+ header->data_size = prf_mod_data_count(mod);
+ header->padding_bytes_before_counters = 0;
+ header->counters_size = prf_mod_cnts_count(mod);
+ header->padding_bytes_after_counters = 0;
+ header->names_size = prf_mod_names_count(mod);
+ header->counters_delta = (u64)mod->prf_cnts;
+ header->names_delta = (u64)mod->prf_names;
+ header->value_kind_last = LLVM_INSTR_PROF_IPVK_LAST;
+
+ *buffer += sizeof(*header);
+}
+
+/*
+ * Copy the source into the buffer, incrementing the pointer into buffer in the
+ * process.
+ */
+static void prf_copy_to_buffer(void **buffer, const void *src, unsigned long size)
+{
+ memcpy(*buffer, src, size);
+ *buffer += size;
+}
+
+/* extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds) */
+
+static u32 prf_module_get_value_size(struct module *mod)
+{
+ u32 size = 0;
+ struct llvm_prf_data *p;
+ struct llvm_prf_data *start = mod->prf_data;
+ struct llvm_prf_data *end = start + prf_mod_data_count(mod);
+
+ for (p = start; p < end; p++)
+ size += __prf_get_value_size(p, NULL);
+
+ return size;
+}
+
+/* Serialize the profiling's value.
+ * extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer)
+ */
+
+static void prf_module_serialize_values(struct module *mod, void **buffer)
+{
+ struct llvm_prf_data *p;
+ struct llvm_prf_data *start = mod->prf_data;
+ struct llvm_prf_data *end = start + prf_mod_data_count(mod);
+
+ for (p = start; p < end; p++)
+ prf_serialize_value(p, buffer);
+}
+
+static inline unsigned long prf_get_padding(unsigned long size)
+{
+ return 7 & (sizeof(u64) - size % sizeof(u64));
+}
+
+static unsigned long prf_module_buffer_size(struct module *mod)
+{
+ return sizeof(struct llvm_prf_header) +
+ prf_mod_data_size(mod) +
+ prf_mod_cnts_size(mod) +
+ prf_mod_names_size(mod) +
+ prf_get_padding(prf_mod_names_size(mod)) +
+ prf_module_get_value_size(mod);
+}
+
+/*
+ * Serialize the profiling data into a format LLVM's tools can understand.
+ * Note: caller *must* hold pgo_lock and hold reference to the module.
+ */
+static int prf_module_serialize(struct module *mod, struct prf_mod_data *p, unsigned long *buf_size)
+{
+ int err = 0;
+ void *buffer;
+
+ /* re-check buffer size */
+ *buf_size = prf_module_buffer_size(mod);
+
+ if (p->size < *buf_size || !p->buffer) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ buffer = p->buffer;
+
+ prf_module_fill_header(mod, &buffer);
+ prf_copy_to_buffer(&buffer, mod->prf_data, prf_mod_data_size(mod));
+ prf_copy_to_buffer(&buffer, mod->prf_cnts, prf_mod_cnts_size(mod));
+ prf_copy_to_buffer(&buffer, mod->prf_names, prf_mod_names_size(mod));
+ buffer += prf_get_padding(prf_mod_names_size(mod));
+
+ prf_module_serialize_values(mod, &buffer);
+
+out:
+ return err;
+}
+
+/* open() implementation for module PGO. */
+static int prf_module_open(struct inode *inode, struct file *file)
+{
+ struct prf_mod_private_data *data;
+ struct prf_mod_data *pinfo;
+ struct module *mod;
+ unsigned long flags;
+ unsigned long buf_size = 0;
+ int err = 0;
+
+ mutex_lock(&prf_mod_lock);
+ data = inode->i_private; /* see: pgo_module_notifier() */
+
+ BUG_ON(!data);
+
+ /* grab the module */
+ mod = READ_ONCE(data->mod);
+ if (mod && try_module_get(mod)) {
+ // Is it live?
+ if (mod->state != MODULE_STATE_LIVE) {
+ err = -EAGAIN;
+ goto put_unlock;
+ }
+
+ pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
+ if (!pinfo) {
+ err = -ENOMEM;
+ goto put_unlock;
+ }
+
+ mutex_unlock(&prf_mod_lock);
+
+ /* estimate amount of memory needed:
+ * can't vzalloc() while prf_lock() is held
+ * and prf_module_buffer_size() only works while it is held..
+ */
+ flags = prf_lock();
+ buf_size = prf_module_buffer_size(mod);
+ do {
+ prf_unlock(flags);
+
+ /* resize buffer */
+ if (pinfo->size < buf_size && pinfo->buffer) {
+ vfree(pinfo->buffer);
+ pinfo->buffer = NULL;
+ }
+
+ if (!pinfo->buffer) {
+ pinfo->size = buf_size;
+ pinfo->buffer = vzalloc(pinfo->size);
+
+ if (!pinfo->buffer) {
+ err = -ENOMEM;
+ goto out;
+ }
+ }
+
+ /* try serialize */
+ flags = prf_lock();
+
+ } while (prf_module_serialize(mod, pinfo, &buf_size));
+
+ prf_unlock(flags);
+
+ /* success! */
+ pinfo->size = buf_size;
+ file->private_data = pinfo;
+
+ module_put(mod);
+ return err;
+ }
+
+put_unlock:
+ module_put(mod);
+ mutex_unlock(&prf_mod_lock);
+out:
+ return err;
+}
+
+/* read() implementation for PGO. */
+static ssize_t prf_module_read(struct file *file, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ struct prf_mod_data *pinfo = file->private_data;
+
+ BUG_ON(!pinfo);
+
+ return simple_read_from_buffer(buf, count, ppos, pinfo->buffer,
+ pinfo->size);
+}
+
+/* release() implementation for PGO. Release resources allocated by open(). */
+static int prf_module_release(struct inode *inode, struct file *file)
+{
+ struct prf_mod_data *pinfo = file->private_data;
+
+ if (pinfo) {
+ vfree(pinfo->buffer);
+ kfree(pinfo);
+ file->private_data = 0;
+ }
+ return 0;
+}
+
+static const struct file_operations prf_mod_fops = {
+ .owner = THIS_MODULE,
+ .open = prf_module_open,
+ .read = prf_module_read,
+ .llseek = default_llseek,
+ .release = prf_module_release
+};
+
+static void prf_module_free(struct rcu_head *rcu)
+{
+ struct prf_mod_private_data *data;
+
+ data = container_of(rcu, struct prf_mod_private_data, rcu);
+
+ debugfs_remove(data->file);
+
+ kfree(data);
+}
+
+static int pgo_module_notifier(struct notifier_block *nb, unsigned long event,
+ void *pdata)
+{
+ struct module *mod = pdata;
+ struct prf_mod_private_data *data;
+ char fsname[MODULE_NAME_LEN + 9]; // +strlen(".profraw")
+
+ if (event == MODULE_STATE_LIVE) {
+ /* does the module have profiling info? */
+ if (mod->prf_data
+ && mod->prf_cnts
+ && mod->prf_names
+ && mod->prf_vnds) {
+ /* add module prf_mod_private_data entry */
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+
+ fsname[0] = 0;
+ snprintf(fsname, sizeof(fsname), "%s.profraw", mod->name);
+
+ mutex_lock(&prf_mod_lock);
+
+ data->file = debugfs_create_file(fsname, 0600, directory, data, &prf_mod_fops);
+ if (!data->file) {
+ pr_err("Failed setup module pgo: %s", fsname);
+ kfree(data);
+ mutex_unlock(&prf_mod_lock);
+ return NOTIFY_OK;
+ }
+
+ WRITE_ONCE(data->mod, mod);
+
+ list_add_tail_rcu(&data->link, &prf_mod_list);
+ mutex_unlock(&prf_mod_lock);
+ }
+ }
+ if (event == MODULE_STATE_GOING) {
+ /* remove module from the list */
+ rcu_read_lock();
+ list_for_each_entry_rcu(data, &prf_mod_list, link) {
+ if (strcmp(data->mod->name, mod->name) == 0) {
+
+ mutex_lock(&prf_mod_lock);
+ /* remofe from profiled modules */
+ list_del_rcu(&data->link);
+ /* mark it stale */
+ WRITE_ONCE(data->mod, NULL);
+ mutex_unlock(&prf_mod_lock);
+ call_rcu(&data->rcu, prf_module_free);
+ break;
+ }
+ }
+ rcu_read_unlock();
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block pgo_module_nb = {
+ .notifier_call = pgo_module_notifier
+};
+
+void prf_modules_init(void)
+{
+ register_module_notifier(&pgo_module_nb);
+}
+
+void prf_modules_exit(void)
+{
+ struct prf_mod_private_data *p;
+
+ /* unsubscribe the notifier and do cleanup. */
+ unregister_module_notifier(&pgo_module_nb);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(p, &prf_mod_list, link) {
+ /* delete nodes */
+ list_del(&p->link);
+ WRITE_ONCE(p->mod, NULL);
+ call_rcu(&p->rcu, prf_module_free);
+ }
+ rcu_read_unlock();
+}
\ No newline at end of file
diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
index 464b3bc77431..98cfa11a7b76 100644
--- a/kernel/pgo/instrument.c
+++ b/kernel/pgo/instrument.c
@@ -139,11 +139,11 @@ EXPORT_SYMBOL(__llvm_profile_instrument_target);
/* Counts the number of times a range of targets values are seen. */
void __llvm_profile_instrument_range(u64 target_value, void *data,
- u32 index, s64 precise_start,
- s64 precise_last, s64 large_value);
+ u32 index, s64 precise_start,
+ s64 precise_last, s64 large_value);
void __llvm_profile_instrument_range(u64 target_value, void *data,
- u32 index, s64 precise_start,
- s64 precise_last, s64 large_value)
+ u32 index, s64 precise_start,
+ s64 precise_last, s64 large_value)
{
if (large_value != S64_MIN && (s64)target_value >= large_value)
target_value = large_value;
@@ -176,9 +176,9 @@ static u64 inst_prof_get_range_rep_value(u64 value)
* defined in compiler-rt/include/profile/InstrProfData.inc.
*/
void __llvm_profile_instrument_memop(u64 target_value, void *data,
- u32 counter_index);
+ u32 counter_index);
void __llvm_profile_instrument_memop(u64 target_value, void *data,
- u32 counter_index)
+ u32 counter_index)
{
u64 rep_value;
diff --git a/kernel/pgo/pgo.h b/kernel/pgo/pgo.h
index a9ff51abbfd5..2840da63c7cd 100644
--- a/kernel/pgo/pgo.h
+++ b/kernel/pgo/pgo.h
@@ -212,17 +212,13 @@ struct prf_mod_private_data {
struct list_head link;
struct rcu_head rcu;
- void *buffer;
- unsigned long size;
-
- char mod_name[MODULE_NAME_LEN];
struct module *mod;
struct dentry *file;
int current_node;
};
-/* Mutex protecting the prf_mod_list and entries */
+/* Mutex protecting the prf_mod_list */
extern struct mutex prf_mod_lock;
/* List of modules profiled */
@@ -231,10 +227,7 @@ extern struct list_head prf_mod_list;
extern void prf_modules_init(void);
extern void prf_modules_exit(void);
-/* Update each modules snapshot of the profiling data. */
-extern int prf_modules_snapshot(void);
-
-/* below funcs are required by prf_modules_snapshot() */
+/* below funcs are required by prf_modules */
extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds);
extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer);
--
2.31.1
On Fri, May 28, 2021 at 11:08:21PM +0300, Jarmo Tiitto wrote:
> PGO profile data is exported from the kernel by
> creating debugfs files: pgo/<module>.profraw for each module.
Again, I do not really have many comments on the actual code as I am not
super familiar with it.
However, fs_mod.c duplicates a lot of the functions in fs.c, which makes
maintaining this code even more difficult, especially against LLVM PGO
profile data format changes. I just want to make sure I understand this:
does PGO currently not work with modules? Or does this patch series just
make it so that each module has its own .profraw file so individual
modules can be optimized?
If it is the latter, what is the point? Why would you want to optimize
just a module and not the entire kernel, if you already have to go
through the profiling steps?
If it is the former, there has to be a better way to share more of the
machinery between fs.c and fs_mod.c than basically duplicating
everything because there are some parameters and logic that have to
change. I do not have a ton of time to outline exactly what that might
look like but for example, prf_fill_header and prf_module_fill_header
are basically the same thing aside from the mod parameter and the
prf_..._count() calls. It seems like if mod was NULL, you would call the
vmlinux versions of the functions.
> Modules are register into the profiler via module notifier callback
> similiar to gcov/base.c. Note that if module does not have __llvm_prf_xxx
> sections required the module ignored.
>
> Also there is no "reset" support for yet for these files.
>
> Signed-off-by: Jarmo Tiitto <[email protected]>
> ---
> kernel/pgo/Makefile | 2 +-
> kernel/pgo/fs.c | 54 ++++--
> kernel/pgo/fs_mod.c | 415 ++++++++++++++++++++++++++++++++++++++++
> kernel/pgo/instrument.c | 12 +-
> kernel/pgo/pgo.h | 11 +-
> 5 files changed, 466 insertions(+), 28 deletions(-)
> create mode 100644 kernel/pgo/fs_mod.c
>
> diff --git a/kernel/pgo/Makefile b/kernel/pgo/Makefile
> index 41e27cefd9a4..662b7dfdfbe9 100644
> --- a/kernel/pgo/Makefile
> +++ b/kernel/pgo/Makefile
> @@ -2,4 +2,4 @@
> GCOV_PROFILE := n
> PGO_PROFILE := n
>
> -obj-y += fs.o instrument.o
> +obj-y += fs.o fs_mod.o instrument.o
> diff --git a/kernel/pgo/fs.c b/kernel/pgo/fs.c
> index 575142735273..5471d270a5bb 100644
> --- a/kernel/pgo/fs.c
> +++ b/kernel/pgo/fs.c
> @@ -227,15 +227,15 @@ static unsigned long prf_buffer_size(void)
> * Serialize the profiling data into a format LLVM's tools can understand.
> * Note: caller *must* hold pgo_lock.
> */
> -static int prf_serialize(struct prf_private_data *p)
> +static int prf_serialize(struct prf_private_data *p, unsigned long *buf_size)
> {
> int err = 0;
> void *buffer;
>
> - p->size = prf_buffer_size();
> - p->buffer = vzalloc(p->size);
> + /* re-check buffer size */
> + *buf_size = prf_buffer_size();
>
> - if (!p->buffer) {
> + if (p->size < *buf_size || !p->buffer) {
> err = -ENOMEM;
> goto out;
> }
> @@ -262,7 +262,8 @@ static int prf_open(struct inode *inode, struct file *file)
> {
> struct prf_private_data *data;
> unsigned long flags;
> - int err;
> + unsigned long buf_size;
> + int err = 0;
>
> data = kzalloc(sizeof(*data), GFP_KERNEL);
> if (!data) {
> @@ -270,18 +271,41 @@ static int prf_open(struct inode *inode, struct file *file)
> goto out;
> }
>
> + /* estimate amount of memory needed:
> + * can't vzalloc() while prf_lock() is held:
> + * CONFIG_DEBUG_ATOMIC_SLEEP complains.
> + * So first get buffer size, release the lock,
> + * vzalloc(), re-lock and try serialize.
> + */
> flags = prf_lock();
> + buf_size = prf_buffer_size();
>
> - err = prf_serialize(data);
> - if (unlikely(err)) {
> - kfree(data);
> - goto out_unlock;
> - }
> + do {
> + prf_unlock(flags);
>
> - file->private_data = data;
> + /* resize buffer */
> + if (data->size < buf_size && data->buffer) {
> + vfree(data->buffer);
> + data->buffer = NULL;
> + }
> +
> + if (!data->buffer) {
> + data->size = buf_size;
> + data->buffer = vzalloc(data->size);
>
> -out_unlock:
> + if (!data->buffer) {
> + err = -ENOMEM;
> + goto out;
> + }
> + }
> + /* try serialize */
> + flags = prf_lock();
> + } while (prf_serialize(data, &buf_size));
> prf_unlock(flags);
> +
> + data->size = buf_size;
> + file->private_data = data;
> +
> out:
> return err;
> }
> @@ -363,6 +387,8 @@ static const struct file_operations prf_reset_fops = {
> /* Create debugfs entries. */
> static int __init pgo_init(void)
> {
> + pr_notice("Clang PGO profile data available.");
> +
> directory = debugfs_create_dir("pgo", NULL);
> if (!directory)
> goto err_remove;
> @@ -375,6 +401,8 @@ static int __init pgo_init(void)
> &prf_reset_fops))
> goto err_remove;
>
> + prf_modules_init();
> +
> return 0;
>
> err_remove:
> @@ -385,6 +413,8 @@ static int __init pgo_init(void)
> /* Remove debugfs entries. */
> static void __exit pgo_exit(void)
> {
> + prf_modules_exit();
> +
> debugfs_remove_recursive(directory);
> }
>
> diff --git a/kernel/pgo/fs_mod.c b/kernel/pgo/fs_mod.c
> new file mode 100644
> index 000000000000..0808d44227f1
> --- /dev/null
> +++ b/kernel/pgo/fs_mod.c
> @@ -0,0 +1,415 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Jarmo Tiitto
> + *
> + * Author:
> + * Jarmo Tiitto <[email protected]>
> + *
> + * Based on the clang PGO kernel patch by:
> + * Sami Tolvanen <[email protected]>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#define pr_fmt(fmt) "pgo: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/debugfs.h>
> +#include <linux/fs.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +#include "pgo.h"
> +
> +/*
> + * PGO profile data reporting for modules:
> + * We maintain one debugfs pgo/<module>.profraw file per module.
> + */
> +
> +
> +DEFINE_MUTEX(prf_mod_lock);
> +LIST_HEAD(prf_mod_list);
> +
> +struct prf_mod_data {
> + void *buffer;
> + unsigned long size;
> +};
> +
> +/* these are trivial, but each one differs a bit */
> +static inline unsigned long prf_mod_data_size(struct module *mod)
> +{
> + return mod->prf_data_size;
> +}
> +
> +static inline unsigned long prf_mod_data_count(struct module *mod)
> +{
> + return mod->prf_data_size / sizeof(struct llvm_prf_data);
> +}
> +
> +static inline unsigned long prf_mod_cnts_size(struct module *mod)
> +{
> + return mod->prf_cnts_num * sizeof(mod->prf_cnts[0]);
> +}
> +
> +static inline unsigned long prf_mod_cnts_count(struct module *mod)
> +{
> + return mod->prf_cnts_num;
> +}
> +
> +static inline unsigned long prf_mod_names_size(struct module *mod)
> +{
> + return mod->prf_names_num * sizeof(mod->prf_names[0]);
> +}
> +
> +static inline unsigned long prf_mod_names_count(struct module *mod)
> +{
> + return mod->prf_names_num;
> +}
> +
> +static inline unsigned long prf_mod_vnds_size(struct module *mod)
> +{
> + return mod->prf_vnds_size;
> +}
> +
> +static inline unsigned long prf_mod_vnds_count(struct module *mod)
> +{
> + return mod->prf_vnds_size / sizeof(struct llvm_prf_value_node);
> +}
> +
> +/*
> + * Raw profile data format:
> + *
> + * - llvm_prf_header
> + * - __llvm_prf_data
> + * - __llvm_prf_cnts
> + * - __llvm_prf_names
> + * - zero padding to 8 bytes
> + * - for each llvm_prf_data in __llvm_prf_data:
> + * - llvm_prf_value_data
> + * - llvm_prf_value_record + site count array
> + * - llvm_prf_value_node_data
> + * ...
> + * ...
> + * ...
> + */
> +
> +static void prf_module_fill_header(struct module *mod, void **buffer)
> +{
> + struct llvm_prf_header *header = *(struct llvm_prf_header **)buffer;
> +
> +#ifdef CONFIG_64BIT
> + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_64;
> +#else
> + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_32;
> +#endif
> + header->version = LLVM_VARIANT_MASK_IR_PROF | LLVM_INSTR_PROF_RAW_VERSION;
> + header->data_size = prf_mod_data_count(mod);
> + header->padding_bytes_before_counters = 0;
> + header->counters_size = prf_mod_cnts_count(mod);
> + header->padding_bytes_after_counters = 0;
> + header->names_size = prf_mod_names_count(mod);
> + header->counters_delta = (u64)mod->prf_cnts;
> + header->names_delta = (u64)mod->prf_names;
> + header->value_kind_last = LLVM_INSTR_PROF_IPVK_LAST;
> +
> + *buffer += sizeof(*header);
> +}
> +
> +/*
> + * Copy the source into the buffer, incrementing the pointer into buffer in the
> + * process.
> + */
> +static void prf_copy_to_buffer(void **buffer, const void *src, unsigned long size)
> +{
> + memcpy(*buffer, src, size);
> + *buffer += size;
> +}
> +
> +/* extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds) */
> +
> +static u32 prf_module_get_value_size(struct module *mod)
> +{
> + u32 size = 0;
> + struct llvm_prf_data *p;
> + struct llvm_prf_data *start = mod->prf_data;
> + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> +
> + for (p = start; p < end; p++)
> + size += __prf_get_value_size(p, NULL);
> +
> + return size;
> +}
> +
> +/* Serialize the profiling's value.
> + * extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer)
> + */
> +
> +static void prf_module_serialize_values(struct module *mod, void **buffer)
> +{
> + struct llvm_prf_data *p;
> + struct llvm_prf_data *start = mod->prf_data;
> + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> +
> + for (p = start; p < end; p++)
> + prf_serialize_value(p, buffer);
> +}
> +
> +static inline unsigned long prf_get_padding(unsigned long size)
> +{
> + return 7 & (sizeof(u64) - size % sizeof(u64));
> +}
> +
> +static unsigned long prf_module_buffer_size(struct module *mod)
> +{
> + return sizeof(struct llvm_prf_header) +
> + prf_mod_data_size(mod) +
> + prf_mod_cnts_size(mod) +
> + prf_mod_names_size(mod) +
> + prf_get_padding(prf_mod_names_size(mod)) +
> + prf_module_get_value_size(mod);
> +}
> +
> +/*
> + * Serialize the profiling data into a format LLVM's tools can understand.
> + * Note: caller *must* hold pgo_lock and hold reference to the module.
> + */
> +static int prf_module_serialize(struct module *mod, struct prf_mod_data *p, unsigned long *buf_size)
> +{
> + int err = 0;
> + void *buffer;
> +
> + /* re-check buffer size */
> + *buf_size = prf_module_buffer_size(mod);
> +
> + if (p->size < *buf_size || !p->buffer) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + buffer = p->buffer;
> +
> + prf_module_fill_header(mod, &buffer);
> + prf_copy_to_buffer(&buffer, mod->prf_data, prf_mod_data_size(mod));
> + prf_copy_to_buffer(&buffer, mod->prf_cnts, prf_mod_cnts_size(mod));
> + prf_copy_to_buffer(&buffer, mod->prf_names, prf_mod_names_size(mod));
> + buffer += prf_get_padding(prf_mod_names_size(mod));
> +
> + prf_module_serialize_values(mod, &buffer);
> +
> +out:
> + return err;
> +}
> +
> +/* open() implementation for module PGO. */
> +static int prf_module_open(struct inode *inode, struct file *file)
> +{
> + struct prf_mod_private_data *data;
> + struct prf_mod_data *pinfo;
> + struct module *mod;
> + unsigned long flags;
> + unsigned long buf_size = 0;
> + int err = 0;
> +
> + mutex_lock(&prf_mod_lock);
> + data = inode->i_private; /* see: pgo_module_notifier() */
> +
> + BUG_ON(!data);
> +
> + /* grab the module */
> + mod = READ_ONCE(data->mod);
> + if (mod && try_module_get(mod)) {
> + // Is it live?
> + if (mod->state != MODULE_STATE_LIVE) {
> + err = -EAGAIN;
> + goto put_unlock;
> + }
> +
> + pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
> + if (!pinfo) {
> + err = -ENOMEM;
> + goto put_unlock;
> + }
> +
> + mutex_unlock(&prf_mod_lock);
> +
> + /* estimate amount of memory needed:
> + * can't vzalloc() while prf_lock() is held
> + * and prf_module_buffer_size() only works while it is held..
> + */
> + flags = prf_lock();
> + buf_size = prf_module_buffer_size(mod);
> + do {
> + prf_unlock(flags);
> +
> + /* resize buffer */
> + if (pinfo->size < buf_size && pinfo->buffer) {
> + vfree(pinfo->buffer);
> + pinfo->buffer = NULL;
> + }
> +
> + if (!pinfo->buffer) {
> + pinfo->size = buf_size;
> + pinfo->buffer = vzalloc(pinfo->size);
> +
> + if (!pinfo->buffer) {
> + err = -ENOMEM;
> + goto out;
> + }
> + }
> +
> + /* try serialize */
> + flags = prf_lock();
> +
> + } while (prf_module_serialize(mod, pinfo, &buf_size));
> +
> + prf_unlock(flags);
> +
> + /* success! */
> + pinfo->size = buf_size;
> + file->private_data = pinfo;
> +
> + module_put(mod);
> + return err;
> + }
> +
> +put_unlock:
> + module_put(mod);
> + mutex_unlock(&prf_mod_lock);
> +out:
> + return err;
> +}
> +
> +/* read() implementation for PGO. */
> +static ssize_t prf_module_read(struct file *file, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + struct prf_mod_data *pinfo = file->private_data;
> +
> + BUG_ON(!pinfo);
> +
> + return simple_read_from_buffer(buf, count, ppos, pinfo->buffer,
> + pinfo->size);
> +}
> +
> +/* release() implementation for PGO. Release resources allocated by open(). */
> +static int prf_module_release(struct inode *inode, struct file *file)
> +{
> + struct prf_mod_data *pinfo = file->private_data;
> +
> + if (pinfo) {
> + vfree(pinfo->buffer);
> + kfree(pinfo);
> + file->private_data = 0;
> + }
> + return 0;
> +}
> +
> +static const struct file_operations prf_mod_fops = {
> + .owner = THIS_MODULE,
> + .open = prf_module_open,
> + .read = prf_module_read,
> + .llseek = default_llseek,
> + .release = prf_module_release
> +};
> +
> +static void prf_module_free(struct rcu_head *rcu)
> +{
> + struct prf_mod_private_data *data;
> +
> + data = container_of(rcu, struct prf_mod_private_data, rcu);
> +
> + debugfs_remove(data->file);
> +
> + kfree(data);
> +}
> +
> +static int pgo_module_notifier(struct notifier_block *nb, unsigned long event,
> + void *pdata)
> +{
> + struct module *mod = pdata;
> + struct prf_mod_private_data *data;
> + char fsname[MODULE_NAME_LEN + 9]; // +strlen(".profraw")
> +
> + if (event == MODULE_STATE_LIVE) {
> + /* does the module have profiling info? */
> + if (mod->prf_data
> + && mod->prf_cnts
> + && mod->prf_names
> + && mod->prf_vnds) {
> + /* add module prf_mod_private_data entry */
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> +
> + fsname[0] = 0;
> + snprintf(fsname, sizeof(fsname), "%s.profraw", mod->name);
> +
> + mutex_lock(&prf_mod_lock);
> +
> + data->file = debugfs_create_file(fsname, 0600, directory, data, &prf_mod_fops);
> + if (!data->file) {
> + pr_err("Failed setup module pgo: %s", fsname);
> + kfree(data);
> + mutex_unlock(&prf_mod_lock);
> + return NOTIFY_OK;
> + }
> +
> + WRITE_ONCE(data->mod, mod);
> +
> + list_add_tail_rcu(&data->link, &prf_mod_list);
> + mutex_unlock(&prf_mod_lock);
> + }
> + }
> + if (event == MODULE_STATE_GOING) {
> + /* remove module from the list */
> + rcu_read_lock();
> + list_for_each_entry_rcu(data, &prf_mod_list, link) {
> + if (strcmp(data->mod->name, mod->name) == 0) {
> +
> + mutex_lock(&prf_mod_lock);
> + /* remofe from profiled modules */
> + list_del_rcu(&data->link);
> + /* mark it stale */
> + WRITE_ONCE(data->mod, NULL);
> + mutex_unlock(&prf_mod_lock);
> + call_rcu(&data->rcu, prf_module_free);
> + break;
> + }
> + }
> + rcu_read_unlock();
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block pgo_module_nb = {
> + .notifier_call = pgo_module_notifier
> +};
> +
> +void prf_modules_init(void)
> +{
> + register_module_notifier(&pgo_module_nb);
> +}
> +
> +void prf_modules_exit(void)
> +{
> + struct prf_mod_private_data *p;
> +
> + /* unsubscribe the notifier and do cleanup. */
> + unregister_module_notifier(&pgo_module_nb);
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(p, &prf_mod_list, link) {
> + /* delete nodes */
> + list_del(&p->link);
> + WRITE_ONCE(p->mod, NULL);
> + call_rcu(&p->rcu, prf_module_free);
> + }
> + rcu_read_unlock();
> +}
> \ No newline at end of file
> diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
> index 464b3bc77431..98cfa11a7b76 100644
> --- a/kernel/pgo/instrument.c
> +++ b/kernel/pgo/instrument.c
> @@ -139,11 +139,11 @@ EXPORT_SYMBOL(__llvm_profile_instrument_target);
>
> /* Counts the number of times a range of targets values are seen. */
> void __llvm_profile_instrument_range(u64 target_value, void *data,
> - u32 index, s64 precise_start,
> - s64 precise_last, s64 large_value);
> + u32 index, s64 precise_start,
> + s64 precise_last, s64 large_value);
> void __llvm_profile_instrument_range(u64 target_value, void *data,
> - u32 index, s64 precise_start,
> - s64 precise_last, s64 large_value)
> + u32 index, s64 precise_start,
> + s64 precise_last, s64 large_value)
> {
> if (large_value != S64_MIN && (s64)target_value >= large_value)
> target_value = large_value;
> @@ -176,9 +176,9 @@ static u64 inst_prof_get_range_rep_value(u64 value)
> * defined in compiler-rt/include/profile/InstrProfData.inc.
> */
> void __llvm_profile_instrument_memop(u64 target_value, void *data,
> - u32 counter_index);
> + u32 counter_index);
> void __llvm_profile_instrument_memop(u64 target_value, void *data,
> - u32 counter_index)
> + u32 counter_index)
> {
> u64 rep_value;
>
> diff --git a/kernel/pgo/pgo.h b/kernel/pgo/pgo.h
> index a9ff51abbfd5..2840da63c7cd 100644
> --- a/kernel/pgo/pgo.h
> +++ b/kernel/pgo/pgo.h
> @@ -212,17 +212,13 @@ struct prf_mod_private_data {
> struct list_head link;
> struct rcu_head rcu;
>
> - void *buffer;
> - unsigned long size;
> -
> - char mod_name[MODULE_NAME_LEN];
> struct module *mod;
> struct dentry *file;
>
> int current_node;
> };
>
> -/* Mutex protecting the prf_mod_list and entries */
> +/* Mutex protecting the prf_mod_list */
> extern struct mutex prf_mod_lock;
>
> /* List of modules profiled */
> @@ -231,10 +227,7 @@ extern struct list_head prf_mod_list;
> extern void prf_modules_init(void);
> extern void prf_modules_exit(void);
>
> -/* Update each modules snapshot of the profiling data. */
> -extern int prf_modules_snapshot(void);
> -
> -/* below funcs are required by prf_modules_snapshot() */
> +/* below funcs are required by prf_modules */
> extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds);
>
> extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer);
> --
> 2.31.1
On Mon, May 31, 2021 at 12:09 PM Nathan Chancellor <[email protected]> wrote:
>
> On Fri, May 28, 2021 at 11:08:21PM +0300, Jarmo Tiitto wrote:
> > PGO profile data is exported from the kernel by
> > creating debugfs files: pgo/<module>.profraw for each module.
>
> Again, I do not really have many comments on the actual code as I am not
> super familiar with it.
>
> However, fs_mod.c duplicates a lot of the functions in fs.c, which makes
> maintaining this code even more difficult, especially against LLVM PGO
> profile data format changes. I just want to make sure I understand this:
> does PGO currently not work with modules? Or does this patch series just
> make it so that each module has its own .profraw file so individual
> modules can be optimized?
>
> If it is the latter, what is the point? Why would you want to optimize
> just a module and not the entire kernel, if you already have to go
> through the profiling steps?
>
> If it is the former, there has to be a better way to share more of the
> machinery between fs.c and fs_mod.c than basically duplicating
> everything because there are some parameters and logic that have to
> change. I do not have a ton of time to outline exactly what that might
> look like but for example, prf_fill_header and prf_module_fill_header
> are basically the same thing aside from the mod parameter and the
> prf_..._count() calls. It seems like if mod was NULL, you would call the
> vmlinux versions of the functions.
>
Functions definitely shouldn't be duplicated with only minor changes.
We should determine a way to combine them.
As for whether the original PGO patch supports profiling in modules,
the answer is "it depends". :-) I believe that clang inserts profiling
hooks into all code that's compiled with the "-fprofile..." flags.
This would include the modules of course. In GCOV, it's possible to
retrieve profiling information for a single file. Jarmo, is that the
intention of your patches?
-bw
> > Modules are register into the profiler via module notifier callback
> > similiar to gcov/base.c. Note that if module does not have __llvm_prf_xxx
> > sections required the module ignored.
> >
> > Also there is no "reset" support for yet for these files.
> >
> > Signed-off-by: Jarmo Tiitto <[email protected]>
> > ---
> > kernel/pgo/Makefile | 2 +-
> > kernel/pgo/fs.c | 54 ++++--
> > kernel/pgo/fs_mod.c | 415 ++++++++++++++++++++++++++++++++++++++++
> > kernel/pgo/instrument.c | 12 +-
> > kernel/pgo/pgo.h | 11 +-
> > 5 files changed, 466 insertions(+), 28 deletions(-)
> > create mode 100644 kernel/pgo/fs_mod.c
> >
> > diff --git a/kernel/pgo/Makefile b/kernel/pgo/Makefile
> > index 41e27cefd9a4..662b7dfdfbe9 100644
> > --- a/kernel/pgo/Makefile
> > +++ b/kernel/pgo/Makefile
> > @@ -2,4 +2,4 @@
> > GCOV_PROFILE := n
> > PGO_PROFILE := n
> >
> > -obj-y += fs.o instrument.o
> > +obj-y += fs.o fs_mod.o instrument.o
> > diff --git a/kernel/pgo/fs.c b/kernel/pgo/fs.c
> > index 575142735273..5471d270a5bb 100644
> > --- a/kernel/pgo/fs.c
> > +++ b/kernel/pgo/fs.c
> > @@ -227,15 +227,15 @@ static unsigned long prf_buffer_size(void)
> > * Serialize the profiling data into a format LLVM's tools can understand.
> > * Note: caller *must* hold pgo_lock.
> > */
> > -static int prf_serialize(struct prf_private_data *p)
> > +static int prf_serialize(struct prf_private_data *p, unsigned long *buf_size)
> > {
> > int err = 0;
> > void *buffer;
> >
> > - p->size = prf_buffer_size();
> > - p->buffer = vzalloc(p->size);
> > + /* re-check buffer size */
> > + *buf_size = prf_buffer_size();
> >
> > - if (!p->buffer) {
> > + if (p->size < *buf_size || !p->buffer) {
> > err = -ENOMEM;
> > goto out;
> > }
> > @@ -262,7 +262,8 @@ static int prf_open(struct inode *inode, struct file *file)
> > {
> > struct prf_private_data *data;
> > unsigned long flags;
> > - int err;
> > + unsigned long buf_size;
> > + int err = 0;
> >
> > data = kzalloc(sizeof(*data), GFP_KERNEL);
> > if (!data) {
> > @@ -270,18 +271,41 @@ static int prf_open(struct inode *inode, struct file *file)
> > goto out;
> > }
> >
> > + /* estimate amount of memory needed:
> > + * can't vzalloc() while prf_lock() is held:
> > + * CONFIG_DEBUG_ATOMIC_SLEEP complains.
> > + * So first get buffer size, release the lock,
> > + * vzalloc(), re-lock and try serialize.
> > + */
> > flags = prf_lock();
> > + buf_size = prf_buffer_size();
> >
> > - err = prf_serialize(data);
> > - if (unlikely(err)) {
> > - kfree(data);
> > - goto out_unlock;
> > - }
> > + do {
> > + prf_unlock(flags);
> >
> > - file->private_data = data;
> > + /* resize buffer */
> > + if (data->size < buf_size && data->buffer) {
> > + vfree(data->buffer);
> > + data->buffer = NULL;
> > + }
> > +
> > + if (!data->buffer) {
> > + data->size = buf_size;
> > + data->buffer = vzalloc(data->size);
> >
> > -out_unlock:
> > + if (!data->buffer) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > + }
> > + /* try serialize */
> > + flags = prf_lock();
> > + } while (prf_serialize(data, &buf_size));
> > prf_unlock(flags);
> > +
> > + data->size = buf_size;
> > + file->private_data = data;
> > +
> > out:
> > return err;
> > }
> > @@ -363,6 +387,8 @@ static const struct file_operations prf_reset_fops = {
> > /* Create debugfs entries. */
> > static int __init pgo_init(void)
> > {
> > + pr_notice("Clang PGO profile data available.");
> > +
> > directory = debugfs_create_dir("pgo", NULL);
> > if (!directory)
> > goto err_remove;
> > @@ -375,6 +401,8 @@ static int __init pgo_init(void)
> > &prf_reset_fops))
> > goto err_remove;
> >
> > + prf_modules_init();
> > +
> > return 0;
> >
> > err_remove:
> > @@ -385,6 +413,8 @@ static int __init pgo_init(void)
> > /* Remove debugfs entries. */
> > static void __exit pgo_exit(void)
> > {
> > + prf_modules_exit();
> > +
> > debugfs_remove_recursive(directory);
> > }
> >
> > diff --git a/kernel/pgo/fs_mod.c b/kernel/pgo/fs_mod.c
> > new file mode 100644
> > index 000000000000..0808d44227f1
> > --- /dev/null
> > +++ b/kernel/pgo/fs_mod.c
> > @@ -0,0 +1,415 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2021 Jarmo Tiitto
> > + *
> > + * Author:
> > + * Jarmo Tiitto <[email protected]>
> > + *
> > + * Based on the clang PGO kernel patch by:
> > + * Sami Tolvanen <[email protected]>
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#define pr_fmt(fmt) "pgo: " fmt
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/fs.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/vmalloc.h>
> > +#include "pgo.h"
> > +
> > +/*
> > + * PGO profile data reporting for modules:
> > + * We maintain one debugfs pgo/<module>.profraw file per module.
> > + */
> > +
> > +
> > +DEFINE_MUTEX(prf_mod_lock);
> > +LIST_HEAD(prf_mod_list);
> > +
> > +struct prf_mod_data {
> > + void *buffer;
> > + unsigned long size;
> > +};
> > +
> > +/* these are trivial, but each one differs a bit */
> > +static inline unsigned long prf_mod_data_size(struct module *mod)
> > +{
> > + return mod->prf_data_size;
> > +}
> > +
> > +static inline unsigned long prf_mod_data_count(struct module *mod)
> > +{
> > + return mod->prf_data_size / sizeof(struct llvm_prf_data);
> > +}
> > +
> > +static inline unsigned long prf_mod_cnts_size(struct module *mod)
> > +{
> > + return mod->prf_cnts_num * sizeof(mod->prf_cnts[0]);
> > +}
> > +
> > +static inline unsigned long prf_mod_cnts_count(struct module *mod)
> > +{
> > + return mod->prf_cnts_num;
> > +}
> > +
> > +static inline unsigned long prf_mod_names_size(struct module *mod)
> > +{
> > + return mod->prf_names_num * sizeof(mod->prf_names[0]);
> > +}
> > +
> > +static inline unsigned long prf_mod_names_count(struct module *mod)
> > +{
> > + return mod->prf_names_num;
> > +}
> > +
> > +static inline unsigned long prf_mod_vnds_size(struct module *mod)
> > +{
> > + return mod->prf_vnds_size;
> > +}
> > +
> > +static inline unsigned long prf_mod_vnds_count(struct module *mod)
> > +{
> > + return mod->prf_vnds_size / sizeof(struct llvm_prf_value_node);
> > +}
> > +
> > +/*
> > + * Raw profile data format:
> > + *
> > + * - llvm_prf_header
> > + * - __llvm_prf_data
> > + * - __llvm_prf_cnts
> > + * - __llvm_prf_names
> > + * - zero padding to 8 bytes
> > + * - for each llvm_prf_data in __llvm_prf_data:
> > + * - llvm_prf_value_data
> > + * - llvm_prf_value_record + site count array
> > + * - llvm_prf_value_node_data
> > + * ...
> > + * ...
> > + * ...
> > + */
> > +
> > +static void prf_module_fill_header(struct module *mod, void **buffer)
> > +{
> > + struct llvm_prf_header *header = *(struct llvm_prf_header **)buffer;
> > +
> > +#ifdef CONFIG_64BIT
> > + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_64;
> > +#else
> > + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_32;
> > +#endif
> > + header->version = LLVM_VARIANT_MASK_IR_PROF | LLVM_INSTR_PROF_RAW_VERSION;
> > + header->data_size = prf_mod_data_count(mod);
> > + header->padding_bytes_before_counters = 0;
> > + header->counters_size = prf_mod_cnts_count(mod);
> > + header->padding_bytes_after_counters = 0;
> > + header->names_size = prf_mod_names_count(mod);
> > + header->counters_delta = (u64)mod->prf_cnts;
> > + header->names_delta = (u64)mod->prf_names;
> > + header->value_kind_last = LLVM_INSTR_PROF_IPVK_LAST;
> > +
> > + *buffer += sizeof(*header);
> > +}
> > +
> > +/*
> > + * Copy the source into the buffer, incrementing the pointer into buffer in the
> > + * process.
> > + */
> > +static void prf_copy_to_buffer(void **buffer, const void *src, unsigned long size)
> > +{
> > + memcpy(*buffer, src, size);
> > + *buffer += size;
> > +}
> > +
> > +/* extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds) */
> > +
> > +static u32 prf_module_get_value_size(struct module *mod)
> > +{
> > + u32 size = 0;
> > + struct llvm_prf_data *p;
> > + struct llvm_prf_data *start = mod->prf_data;
> > + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> > +
> > + for (p = start; p < end; p++)
> > + size += __prf_get_value_size(p, NULL);
> > +
> > + return size;
> > +}
> > +
> > +/* Serialize the profiling's value.
> > + * extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer)
> > + */
> > +
> > +static void prf_module_serialize_values(struct module *mod, void **buffer)
> > +{
> > + struct llvm_prf_data *p;
> > + struct llvm_prf_data *start = mod->prf_data;
> > + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> > +
> > + for (p = start; p < end; p++)
> > + prf_serialize_value(p, buffer);
> > +}
> > +
> > +static inline unsigned long prf_get_padding(unsigned long size)
> > +{
> > + return 7 & (sizeof(u64) - size % sizeof(u64));
> > +}
> > +
> > +static unsigned long prf_module_buffer_size(struct module *mod)
> > +{
> > + return sizeof(struct llvm_prf_header) +
> > + prf_mod_data_size(mod) +
> > + prf_mod_cnts_size(mod) +
> > + prf_mod_names_size(mod) +
> > + prf_get_padding(prf_mod_names_size(mod)) +
> > + prf_module_get_value_size(mod);
> > +}
> > +
> > +/*
> > + * Serialize the profiling data into a format LLVM's tools can understand.
> > + * Note: caller *must* hold pgo_lock and hold reference to the module.
> > + */
> > +static int prf_module_serialize(struct module *mod, struct prf_mod_data *p, unsigned long *buf_size)
> > +{
> > + int err = 0;
> > + void *buffer;
> > +
> > + /* re-check buffer size */
> > + *buf_size = prf_module_buffer_size(mod);
> > +
> > + if (p->size < *buf_size || !p->buffer) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + buffer = p->buffer;
> > +
> > + prf_module_fill_header(mod, &buffer);
> > + prf_copy_to_buffer(&buffer, mod->prf_data, prf_mod_data_size(mod));
> > + prf_copy_to_buffer(&buffer, mod->prf_cnts, prf_mod_cnts_size(mod));
> > + prf_copy_to_buffer(&buffer, mod->prf_names, prf_mod_names_size(mod));
> > + buffer += prf_get_padding(prf_mod_names_size(mod));
> > +
> > + prf_module_serialize_values(mod, &buffer);
> > +
> > +out:
> > + return err;
> > +}
> > +
> > +/* open() implementation for module PGO. */
> > +static int prf_module_open(struct inode *inode, struct file *file)
> > +{
> > + struct prf_mod_private_data *data;
> > + struct prf_mod_data *pinfo;
> > + struct module *mod;
> > + unsigned long flags;
> > + unsigned long buf_size = 0;
> > + int err = 0;
> > +
> > + mutex_lock(&prf_mod_lock);
> > + data = inode->i_private; /* see: pgo_module_notifier() */
> > +
> > + BUG_ON(!data);
> > +
> > + /* grab the module */
> > + mod = READ_ONCE(data->mod);
> > + if (mod && try_module_get(mod)) {
> > + // Is it live?
> > + if (mod->state != MODULE_STATE_LIVE) {
> > + err = -EAGAIN;
> > + goto put_unlock;
> > + }
> > +
> > + pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
> > + if (!pinfo) {
> > + err = -ENOMEM;
> > + goto put_unlock;
> > + }
> > +
> > + mutex_unlock(&prf_mod_lock);
> > +
> > + /* estimate amount of memory needed:
> > + * can't vzalloc() while prf_lock() is held
> > + * and prf_module_buffer_size() only works while it is held..
> > + */
> > + flags = prf_lock();
> > + buf_size = prf_module_buffer_size(mod);
> > + do {
> > + prf_unlock(flags);
> > +
> > + /* resize buffer */
> > + if (pinfo->size < buf_size && pinfo->buffer) {
> > + vfree(pinfo->buffer);
> > + pinfo->buffer = NULL;
> > + }
> > +
> > + if (!pinfo->buffer) {
> > + pinfo->size = buf_size;
> > + pinfo->buffer = vzalloc(pinfo->size);
> > +
> > + if (!pinfo->buffer) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > + }
> > +
> > + /* try serialize */
> > + flags = prf_lock();
> > +
> > + } while (prf_module_serialize(mod, pinfo, &buf_size));
> > +
> > + prf_unlock(flags);
> > +
> > + /* success! */
> > + pinfo->size = buf_size;
> > + file->private_data = pinfo;
> > +
> > + module_put(mod);
> > + return err;
> > + }
> > +
> > +put_unlock:
> > + module_put(mod);
> > + mutex_unlock(&prf_mod_lock);
> > +out:
> > + return err;
> > +}
> > +
> > +/* read() implementation for PGO. */
> > +static ssize_t prf_module_read(struct file *file, char __user *buf, size_t count,
> > + loff_t *ppos)
> > +{
> > + struct prf_mod_data *pinfo = file->private_data;
> > +
> > + BUG_ON(!pinfo);
> > +
> > + return simple_read_from_buffer(buf, count, ppos, pinfo->buffer,
> > + pinfo->size);
> > +}
> > +
> > +/* release() implementation for PGO. Release resources allocated by open(). */
> > +static int prf_module_release(struct inode *inode, struct file *file)
> > +{
> > + struct prf_mod_data *pinfo = file->private_data;
> > +
> > + if (pinfo) {
> > + vfree(pinfo->buffer);
> > + kfree(pinfo);
> > + file->private_data = 0;
> > + }
> > + return 0;
> > +}
> > +
> > +static const struct file_operations prf_mod_fops = {
> > + .owner = THIS_MODULE,
> > + .open = prf_module_open,
> > + .read = prf_module_read,
> > + .llseek = default_llseek,
> > + .release = prf_module_release
> > +};
> > +
> > +static void prf_module_free(struct rcu_head *rcu)
> > +{
> > + struct prf_mod_private_data *data;
> > +
> > + data = container_of(rcu, struct prf_mod_private_data, rcu);
> > +
> > + debugfs_remove(data->file);
> > +
> > + kfree(data);
> > +}
> > +
> > +static int pgo_module_notifier(struct notifier_block *nb, unsigned long event,
> > + void *pdata)
> > +{
> > + struct module *mod = pdata;
> > + struct prf_mod_private_data *data;
> > + char fsname[MODULE_NAME_LEN + 9]; // +strlen(".profraw")
> > +
> > + if (event == MODULE_STATE_LIVE) {
> > + /* does the module have profiling info? */
> > + if (mod->prf_data
> > + && mod->prf_cnts
> > + && mod->prf_names
> > + && mod->prf_vnds) {
> > + /* add module prf_mod_private_data entry */
> > + data = kzalloc(sizeof(*data), GFP_KERNEL);
> > +
> > + fsname[0] = 0;
> > + snprintf(fsname, sizeof(fsname), "%s.profraw", mod->name);
> > +
> > + mutex_lock(&prf_mod_lock);
> > +
> > + data->file = debugfs_create_file(fsname, 0600, directory, data, &prf_mod_fops);
> > + if (!data->file) {
> > + pr_err("Failed setup module pgo: %s", fsname);
> > + kfree(data);
> > + mutex_unlock(&prf_mod_lock);
> > + return NOTIFY_OK;
> > + }
> > +
> > + WRITE_ONCE(data->mod, mod);
> > +
> > + list_add_tail_rcu(&data->link, &prf_mod_list);
> > + mutex_unlock(&prf_mod_lock);
> > + }
> > + }
> > + if (event == MODULE_STATE_GOING) {
> > + /* remove module from the list */
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(data, &prf_mod_list, link) {
> > + if (strcmp(data->mod->name, mod->name) == 0) {
> > +
> > + mutex_lock(&prf_mod_lock);
> > + /* remofe from profiled modules */
> > + list_del_rcu(&data->link);
> > + /* mark it stale */
> > + WRITE_ONCE(data->mod, NULL);
> > + mutex_unlock(&prf_mod_lock);
> > + call_rcu(&data->rcu, prf_module_free);
> > + break;
> > + }
> > + }
> > + rcu_read_unlock();
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block pgo_module_nb = {
> > + .notifier_call = pgo_module_notifier
> > +};
> > +
> > +void prf_modules_init(void)
> > +{
> > + register_module_notifier(&pgo_module_nb);
> > +}
> > +
> > +void prf_modules_exit(void)
> > +{
> > + struct prf_mod_private_data *p;
> > +
> > + /* unsubscribe the notifier and do cleanup. */
> > + unregister_module_notifier(&pgo_module_nb);
> > +
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(p, &prf_mod_list, link) {
> > + /* delete nodes */
> > + list_del(&p->link);
> > + WRITE_ONCE(p->mod, NULL);
> > + call_rcu(&p->rcu, prf_module_free);
> > + }
> > + rcu_read_unlock();
> > +}
> > \ No newline at end of file
> > diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
> > index 464b3bc77431..98cfa11a7b76 100644
> > --- a/kernel/pgo/instrument.c
> > +++ b/kernel/pgo/instrument.c
> > @@ -139,11 +139,11 @@ EXPORT_SYMBOL(__llvm_profile_instrument_target);
> >
> > /* Counts the number of times a range of targets values are seen. */
> > void __llvm_profile_instrument_range(u64 target_value, void *data,
> > - u32 index, s64 precise_start,
> > - s64 precise_last, s64 large_value);
> > + u32 index, s64 precise_start,
> > + s64 precise_last, s64 large_value);
> > void __llvm_profile_instrument_range(u64 target_value, void *data,
> > - u32 index, s64 precise_start,
> > - s64 precise_last, s64 large_value)
> > + u32 index, s64 precise_start,
> > + s64 precise_last, s64 large_value)
> > {
> > if (large_value != S64_MIN && (s64)target_value >= large_value)
> > target_value = large_value;
> > @@ -176,9 +176,9 @@ static u64 inst_prof_get_range_rep_value(u64 value)
> > * defined in compiler-rt/include/profile/InstrProfData.inc.
> > */
> > void __llvm_profile_instrument_memop(u64 target_value, void *data,
> > - u32 counter_index);
> > + u32 counter_index);
> > void __llvm_profile_instrument_memop(u64 target_value, void *data,
> > - u32 counter_index)
> > + u32 counter_index)
> > {
> > u64 rep_value;
> >
> > diff --git a/kernel/pgo/pgo.h b/kernel/pgo/pgo.h
> > index a9ff51abbfd5..2840da63c7cd 100644
> > --- a/kernel/pgo/pgo.h
> > +++ b/kernel/pgo/pgo.h
> > @@ -212,17 +212,13 @@ struct prf_mod_private_data {
> > struct list_head link;
> > struct rcu_head rcu;
> >
> > - void *buffer;
> > - unsigned long size;
> > -
> > - char mod_name[MODULE_NAME_LEN];
> > struct module *mod;
> > struct dentry *file;
> >
> > int current_node;
> > };
> >
> > -/* Mutex protecting the prf_mod_list and entries */
> > +/* Mutex protecting the prf_mod_list */
> > extern struct mutex prf_mod_lock;
> >
> > /* List of modules profiled */
> > @@ -231,10 +227,7 @@ extern struct list_head prf_mod_list;
> > extern void prf_modules_init(void);
> > extern void prf_modules_exit(void);
> >
> > -/* Update each modules snapshot of the profiling data. */
> > -extern int prf_modules_snapshot(void);
> > -
> > -/* below funcs are required by prf_modules_snapshot() */
> > +/* below funcs are required by prf_modules */
> > extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32 *value_kinds);
> >
> > extern void prf_serialize_value(struct llvm_prf_data *p, void **buffer);
> > --
> > 2.31.1
Kirjoitit tiistaina 1. kesäkuuta 2021 11.33.48 EEST:
> On Mon, May 31, 2021 at 12:09 PM Nathan Chancellor <[email protected]>
wrote:
> > On Fri, May 28, 2021 at 11:08:21PM +0300, Jarmo Tiitto wrote:
> > > PGO profile data is exported from the kernel by
> > > creating debugfs files: pgo/<module>.profraw for each module.
> >
> > Again, I do not really have many comments on the actual code as I am not
> > super familiar with it.
> >
> > However, fs_mod.c duplicates a lot of the functions in fs.c, which makes
> > maintaining this code even more difficult, especially against LLVM PGO
> > profile data format changes. I just want to make sure I understand this:
> > does PGO currently not work with modules? Or does this patch series just
> > make it so that each module has its own .profraw file so individual
> > modules can be optimized?
> >
> > If it is the latter, what is the point? Why would you want to optimize
> > just a module and not the entire kernel, if you already have to go
> > through the profiling steps?
> >
> > If it is the former, there has to be a better way to share more of the
> > machinery between fs.c and fs_mod.c than basically duplicating
> > everything because there are some parameters and logic that have to
> > change. I do not have a ton of time to outline exactly what that might
> > look like but for example, prf_fill_header and prf_module_fill_header
> > are basically the same thing aside from the mod parameter and the
> > prf_..._count() calls. It seems like if mod was NULL, you would call the
> > vmlinux versions of the functions.
>
> Functions definitely shouldn't be duplicated with only minor changes.
> We should determine a way to combine them.
>
> As for whether the original PGO patch supports profiling in modules,
> the answer is "it depends". :-) I believe that clang inserts profiling
> hooks into all code that's compiled with the "-fprofile..." flags.
> This would include the modules of course. In GCOV, it's possible to
> retrieve profiling information for a single file. Jarmo, is that the
> intention of your patches?
>
> -bw
>
Thanks for replying Nathan and Bill!
My original motivation for writing this patch was the realization that no
profile data could be obtained from modules using the original patch only.
My insight when testing the original patch was that the compiler indeed does
instrument+profile all code, including any loaded modules. But this is where
the current instrument.c falls short:
The allocate_node() function may consume nodes from __llvm_prf_vnds_start
that are actually in a module and not in vmlinux.
So llvm_prf_data *p argument may point into an module section, and not into
__llvm_prf_data_start range.
This can result in early exhaustion of of nodes for vmlinux and less accurate
profile data. I think this is actually a bug in the original patch.
So no, the PGO does not currently work with modules. And it somewhat works
for vmlinux.
My code confines the llvm_prf_value_node reservation to vmlinux or module in
instrument.c based on where the llvm_prf_data *p argument points into.
My next intention with the patch is that vmlinux and each loadable module
exports their own, separate profile data file in debugfs/pgo/ like the vmlinux
already does.
So no file level information like in gcov. Only per whole "module.ko" and the
vmlinux binary.
This way you can inspect what each module is doing individually using "llvm-
profdata show mod.profraw"
For PGO final build I intended combining the profile data from vmlinux and all
modules with "llvm-profdata merge" into single profile for optimized build.
I agree fully that the current code duplication is bad.
Maybe I should pass in the mod->prf_xxx sections in a struct to the
serializing functions?
In that way, the struct can be initialized from either module or the vmlinux
sections and all serializing code can share the same code.
Either way, thanks to your questions and info I can try an write better
version.????
I have learned a lot, thanks.
-Jarmo Tiitto
> > > Modules are register into the profiler via module notifier callback
> > > similiar to gcov/base.c. Note that if module does not have
> > > __llvm_prf_xxx
> > > sections required the module ignored.
> > >
> > > Also there is no "reset" support for yet for these files.
> > >
> > > Signed-off-by: Jarmo Tiitto <[email protected]>
> > > ---
> > >
> > > kernel/pgo/Makefile | 2 +-
> > > kernel/pgo/fs.c | 54 ++++--
> > > kernel/pgo/fs_mod.c | 415 ++++++++++++++++++++++++++++++++++++++++
> > > kernel/pgo/instrument.c | 12 +-
> > > kernel/pgo/pgo.h | 11 +-
> > > 5 files changed, 466 insertions(+), 28 deletions(-)
> > > create mode 100644 kernel/pgo/fs_mod.c
> > >
> > > diff --git a/kernel/pgo/Makefile b/kernel/pgo/Makefile
> > > index 41e27cefd9a4..662b7dfdfbe9 100644
> > > --- a/kernel/pgo/Makefile
> > > +++ b/kernel/pgo/Makefile
> > > @@ -2,4 +2,4 @@
> > >
> > > GCOV_PROFILE := n
> > > PGO_PROFILE := n
> > >
> > > -obj-y += fs.o instrument.o
> > > +obj-y += fs.o fs_mod.o instrument.o
> > > diff --git a/kernel/pgo/fs.c b/kernel/pgo/fs.c
> > > index 575142735273..5471d270a5bb 100644
> > > --- a/kernel/pgo/fs.c
> > > +++ b/kernel/pgo/fs.c
> > > @@ -227,15 +227,15 @@ static unsigned long prf_buffer_size(void)
> > >
> > > * Serialize the profiling data into a format LLVM's tools can
> > > understand.
> > > * Note: caller *must* hold pgo_lock.
> > > */
> > >
> > > -static int prf_serialize(struct prf_private_data *p)
> > > +static int prf_serialize(struct prf_private_data *p, unsigned long
> > > *buf_size)> >
> > > {
> > >
> > > int err = 0;
> > > void *buffer;
> > >
> > > - p->size = prf_buffer_size();
> > > - p->buffer = vzalloc(p->size);
> > > + /* re-check buffer size */
> > > + *buf_size = prf_buffer_size();
> > >
> > > - if (!p->buffer) {
> > > + if (p->size < *buf_size || !p->buffer) {
> > >
> > > err = -ENOMEM;
> > > goto out;
> > >
> > > }
> > >
> > > @@ -262,7 +262,8 @@ static int prf_open(struct inode *inode, struct file
> > > *file)> >
> > > {
> > >
> > > struct prf_private_data *data;
> > > unsigned long flags;
> > >
> > > - int err;
> > > + unsigned long buf_size;
> > > + int err = 0;
> > >
> > > data = kzalloc(sizeof(*data), GFP_KERNEL);
> > > if (!data) {
> > >
> > > @@ -270,18 +271,41 @@ static int prf_open(struct inode *inode, struct
> > > file *file)> >
> > > goto out;
> > >
> > > }
> > >
> > > + /* estimate amount of memory needed:
> > > + * can't vzalloc() while prf_lock() is held:
> > > + * CONFIG_DEBUG_ATOMIC_SLEEP complains.
> > > + * So first get buffer size, release the lock,
> > > + * vzalloc(), re-lock and try serialize.
> > > + */
> > >
> > > flags = prf_lock();
> > >
> > > + buf_size = prf_buffer_size();
> > >
> > > - err = prf_serialize(data);
> > > - if (unlikely(err)) {
> > > - kfree(data);
> > > - goto out_unlock;
> > > - }
> > > + do {
> > > + prf_unlock(flags);
> > >
> > > - file->private_data = data;
> > > + /* resize buffer */
> > > + if (data->size < buf_size && data->buffer) {
> > > + vfree(data->buffer);
> > > + data->buffer = NULL;
> > > + }
> > > +
> > > + if (!data->buffer) {
> > > + data->size = buf_size;
> > > + data->buffer = vzalloc(data->size);
> > >
> > > -out_unlock:
> > > + if (!data->buffer) {
> > > + err = -ENOMEM;
> > > + goto out;
> > > + }
> > > + }
> > > + /* try serialize */
> > > + flags = prf_lock();
> > > + } while (prf_serialize(data, &buf_size));
> > >
> > > prf_unlock(flags);
> > >
> > > +
> > > + data->size = buf_size;
> > > + file->private_data = data;
> > > +
> > >
> > > out:
> > > return err;
> > >
> > > }
> > >
> > > @@ -363,6 +387,8 @@ static const struct file_operations prf_reset_fops =
> > > {
> > >
> > > /* Create debugfs entries. */
> > > static int __init pgo_init(void)
> > > {
> > >
> > > + pr_notice("Clang PGO profile data available.");
> > > +
> > >
> > > directory = debugfs_create_dir("pgo", NULL);
> > > if (!directory)
> > >
> > > goto err_remove;
> > >
> > > @@ -375,6 +401,8 @@ static int __init pgo_init(void)
> > >
> > > &prf_reset_fops))
> > >
> > > goto err_remove;
> > >
> > > + prf_modules_init();
> > > +
> > >
> > > return 0;
> > >
> > > err_remove:
> > > @@ -385,6 +413,8 @@ static int __init pgo_init(void)
> > >
> > > /* Remove debugfs entries. */
> > > static void __exit pgo_exit(void)
> > > {
> > >
> > > + prf_modules_exit();
> > > +
> > >
> > > debugfs_remove_recursive(directory);
> > >
> > > }
> > >
> > > diff --git a/kernel/pgo/fs_mod.c b/kernel/pgo/fs_mod.c
> > > new file mode 100644
> > > index 000000000000..0808d44227f1
> > > --- /dev/null
> > > +++ b/kernel/pgo/fs_mod.c
> > > @@ -0,0 +1,415 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2021 Jarmo Tiitto
> > > + *
> > > + * Author:
> > > + * Jarmo Tiitto <[email protected]>
> > > + *
> > > + * Based on the clang PGO kernel patch by:
> > > + * Sami Tolvanen <[email protected]>
> > > + *
> > > + * This software is licensed under the terms of the GNU General Public
> > > + * License version 2, as published by the Free Software Foundation, and
> > > + * may be copied, distributed, and modified under those terms.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + */
> > > +
> > > +#define pr_fmt(fmt) "pgo: " fmt
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/debugfs.h>
> > > +#include <linux/fs.h>
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/vmalloc.h>
> > > +#include "pgo.h"
> > > +
> > > +/*
> > > + * PGO profile data reporting for modules:
> > > + * We maintain one debugfs pgo/<module>.profraw file per module.
> > > + */
> > > +
> > > +
> > > +DEFINE_MUTEX(prf_mod_lock);
> > > +LIST_HEAD(prf_mod_list);
> > > +
> > > +struct prf_mod_data {
> > > + void *buffer;
> > > + unsigned long size;
> > > +};
> > > +
> > > +/* these are trivial, but each one differs a bit */
> > > +static inline unsigned long prf_mod_data_size(struct module *mod)
> > > +{
> > > + return mod->prf_data_size;
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_data_count(struct module *mod)
> > > +{
> > > + return mod->prf_data_size / sizeof(struct llvm_prf_data);
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_cnts_size(struct module *mod)
> > > +{
> > > + return mod->prf_cnts_num * sizeof(mod->prf_cnts[0]);
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_cnts_count(struct module *mod)
> > > +{
> > > + return mod->prf_cnts_num;
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_names_size(struct module *mod)
> > > +{
> > > + return mod->prf_names_num * sizeof(mod->prf_names[0]);
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_names_count(struct module *mod)
> > > +{
> > > + return mod->prf_names_num;
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_vnds_size(struct module *mod)
> > > +{
> > > + return mod->prf_vnds_size;
> > > +}
> > > +
> > > +static inline unsigned long prf_mod_vnds_count(struct module *mod)
> > > +{
> > > + return mod->prf_vnds_size / sizeof(struct llvm_prf_value_node);
> > > +}
> > > +
> > > +/*
> > > + * Raw profile data format:
> > > + *
> > > + * - llvm_prf_header
> > > + * - __llvm_prf_data
> > > + * - __llvm_prf_cnts
> > > + * - __llvm_prf_names
> > > + * - zero padding to 8 bytes
> > > + * - for each llvm_prf_data in __llvm_prf_data:
> > > + * - llvm_prf_value_data
> > > + * - llvm_prf_value_record + site count array
> > > + * - llvm_prf_value_node_data
> > > + * ...
> > > + * ...
> > > + * ...
> > > + */
> > > +
> > > +static void prf_module_fill_header(struct module *mod, void **buffer)
> > > +{
> > > + struct llvm_prf_header *header = *(struct llvm_prf_header
> > > **)buffer;
> > > +
> > > +#ifdef CONFIG_64BIT
> > > + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_64;
> > > +#else
> > > + header->magic = LLVM_INSTR_PROF_RAW_MAGIC_32;
> > > +#endif
> > > + header->version = LLVM_VARIANT_MASK_IR_PROF |
> > > LLVM_INSTR_PROF_RAW_VERSION; + header->data_size =
> > > prf_mod_data_count(mod);
> > > + header->padding_bytes_before_counters = 0;
> > > + header->counters_size = prf_mod_cnts_count(mod);
> > > + header->padding_bytes_after_counters = 0;
> > > + header->names_size = prf_mod_names_count(mod);
> > > + header->counters_delta = (u64)mod->prf_cnts;
> > > + header->names_delta = (u64)mod->prf_names;
> > > + header->value_kind_last = LLVM_INSTR_PROF_IPVK_LAST;
> > > +
> > > + *buffer += sizeof(*header);
> > > +}
> > > +
> > > +/*
> > > + * Copy the source into the buffer, incrementing the pointer into
> > > buffer in the + * process.
> > > + */
> > > +static void prf_copy_to_buffer(void **buffer, const void *src, unsigned
> > > long size) +{
> > > + memcpy(*buffer, src, size);
> > > + *buffer += size;
> > > +}
> > > +
> > > +/* extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32
> > > *value_kinds) */ +
> > > +static u32 prf_module_get_value_size(struct module *mod)
> > > +{
> > > + u32 size = 0;
> > > + struct llvm_prf_data *p;
> > > + struct llvm_prf_data *start = mod->prf_data;
> > > + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> > > +
> > > + for (p = start; p < end; p++)
> > > + size += __prf_get_value_size(p, NULL);
> > > +
> > > + return size;
> > > +}
> > > +
> > > +/* Serialize the profiling's value.
> > > + * extern void prf_serialize_value(struct llvm_prf_data *p, void
> > > **buffer)
> > > + */
> > > +
> > > +static void prf_module_serialize_values(struct module *mod, void
> > > **buffer)
> > > +{
> > > + struct llvm_prf_data *p;
> > > + struct llvm_prf_data *start = mod->prf_data;
> > > + struct llvm_prf_data *end = start + prf_mod_data_count(mod);
> > > +
> > > + for (p = start; p < end; p++)
> > > + prf_serialize_value(p, buffer);
> > > +}
> > > +
> > > +static inline unsigned long prf_get_padding(unsigned long size)
> > > +{
> > > + return 7 & (sizeof(u64) - size % sizeof(u64));
> > > +}
> > > +
> > > +static unsigned long prf_module_buffer_size(struct module *mod)
> > > +{
> > > + return sizeof(struct llvm_prf_header) +
> > > + prf_mod_data_size(mod) +
> > > + prf_mod_cnts_size(mod) +
> > > + prf_mod_names_size(mod) +
> > > + prf_get_padding(prf_mod_names_size(mod)) +
> > > + prf_module_get_value_size(mod);
> > > +}
> > > +
> > > +/*
> > > + * Serialize the profiling data into a format LLVM's tools can
> > > understand.
> > > + * Note: caller *must* hold pgo_lock and hold reference to the module.
> > > + */
> > > +static int prf_module_serialize(struct module *mod, struct prf_mod_data
> > > *p, unsigned long *buf_size) +{
> > > + int err = 0;
> > > + void *buffer;
> > > +
> > > + /* re-check buffer size */
> > > + *buf_size = prf_module_buffer_size(mod);
> > > +
> > > + if (p->size < *buf_size || !p->buffer) {
> > > + err = -ENOMEM;
> > > + goto out;
> > > + }
> > > +
> > > + buffer = p->buffer;
> > > +
> > > + prf_module_fill_header(mod, &buffer);
> > > + prf_copy_to_buffer(&buffer, mod->prf_data,
> > > prf_mod_data_size(mod));
> > > + prf_copy_to_buffer(&buffer, mod->prf_cnts,
> > > prf_mod_cnts_size(mod));
> > > + prf_copy_to_buffer(&buffer, mod->prf_names,
> > > prf_mod_names_size(mod));
> > > + buffer += prf_get_padding(prf_mod_names_size(mod));
> > > +
> > > + prf_module_serialize_values(mod, &buffer);
> > > +
> > > +out:
> > > + return err;
> > > +}
> > > +
> > > +/* open() implementation for module PGO. */
> > > +static int prf_module_open(struct inode *inode, struct file *file)
> > > +{
> > > + struct prf_mod_private_data *data;
> > > + struct prf_mod_data *pinfo;
> > > + struct module *mod;
> > > + unsigned long flags;
> > > + unsigned long buf_size = 0;
> > > + int err = 0;
> > > +
> > > + mutex_lock(&prf_mod_lock);
> > > + data = inode->i_private; /* see: pgo_module_notifier() */
> > > +
> > > + BUG_ON(!data);
> > > +
> > > + /* grab the module */
> > > + mod = READ_ONCE(data->mod);
> > > + if (mod && try_module_get(mod)) {
> > > + // Is it live?
> > > + if (mod->state != MODULE_STATE_LIVE) {
> > > + err = -EAGAIN;
> > > + goto put_unlock;
> > > + }
> > > +
> > > + pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
> > > + if (!pinfo) {
> > > + err = -ENOMEM;
> > > + goto put_unlock;
> > > + }
> > > +
> > > + mutex_unlock(&prf_mod_lock);
> > > +
> > > + /* estimate amount of memory needed:
> > > + * can't vzalloc() while prf_lock() is held
> > > + * and prf_module_buffer_size() only works while it is
> > > held..
> > > + */
> > > + flags = prf_lock();
> > > + buf_size = prf_module_buffer_size(mod);
> > > + do {
> > > + prf_unlock(flags);
> > > +
> > > + /* resize buffer */
> > > + if (pinfo->size < buf_size && pinfo->buffer) {
> > > + vfree(pinfo->buffer);
> > > + pinfo->buffer = NULL;
> > > + }
> > > +
> > > + if (!pinfo->buffer) {
> > > + pinfo->size = buf_size;
> > > + pinfo->buffer = vzalloc(pinfo->size);
> > > +
> > > + if (!pinfo->buffer) {
> > > + err = -ENOMEM;
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + /* try serialize */
> > > + flags = prf_lock();
> > > +
> > > + } while (prf_module_serialize(mod, pinfo, &buf_size));
> > > +
> > > + prf_unlock(flags);
> > > +
> > > + /* success! */
> > > + pinfo->size = buf_size;
> > > + file->private_data = pinfo;
> > > +
> > > + module_put(mod);
> > > + return err;
> > > + }
> > > +
> > > +put_unlock:
> > > + module_put(mod);
> > > + mutex_unlock(&prf_mod_lock);
> > > +out:
> > > + return err;
> > > +}
> > > +
> > > +/* read() implementation for PGO. */
> > > +static ssize_t prf_module_read(struct file *file, char __user *buf,
> > > size_t count, + loff_t *ppos)
> > > +{
> > > + struct prf_mod_data *pinfo = file->private_data;
> > > +
> > > + BUG_ON(!pinfo);
> > > +
> > > + return simple_read_from_buffer(buf, count, ppos, pinfo->buffer,
> > > + pinfo->size);
> > > +}
> > > +
> > > +/* release() implementation for PGO. Release resources allocated by
> > > open(). */ +static int prf_module_release(struct inode *inode, struct
> > > file *file) +{
> > > + struct prf_mod_data *pinfo = file->private_data;
> > > +
> > > + if (pinfo) {
> > > + vfree(pinfo->buffer);
> > > + kfree(pinfo);
> > > + file->private_data = 0;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static const struct file_operations prf_mod_fops = {
> > > + .owner = THIS_MODULE,
> > > + .open = prf_module_open,
> > > + .read = prf_module_read,
> > > + .llseek = default_llseek,
> > > + .release = prf_module_release
> > > +};
> > > +
> > > +static void prf_module_free(struct rcu_head *rcu)
> > > +{
> > > + struct prf_mod_private_data *data;
> > > +
> > > + data = container_of(rcu, struct prf_mod_private_data, rcu);
> > > +
> > > + debugfs_remove(data->file);
> > > +
> > > + kfree(data);
> > > +}
> > > +
> > > +static int pgo_module_notifier(struct notifier_block *nb, unsigned long
> > > event, + void *pdata)
> > > +{
> > > + struct module *mod = pdata;
> > > + struct prf_mod_private_data *data;
> > > + char fsname[MODULE_NAME_LEN + 9]; // +strlen(".profraw")
> > > +
> > > + if (event == MODULE_STATE_LIVE) {
> > > + /* does the module have profiling info? */
> > > + if (mod->prf_data
> > > + && mod->prf_cnts
> > > + && mod->prf_names
> > > + && mod->prf_vnds) {
> > > + /* add module prf_mod_private_data entry */
> > > + data = kzalloc(sizeof(*data), GFP_KERNEL);
> > > +
> > > + fsname[0] = 0;
> > > + snprintf(fsname, sizeof(fsname), "%s.profraw",
> > > mod->name); +
> > > + mutex_lock(&prf_mod_lock);
> > > +
> > > + data->file = debugfs_create_file(fsname, 0600,
> > > directory, data, &prf_mod_fops); + if (!data->file)
> > > {
> > > + pr_err("Failed setup module pgo: %s",
> > > fsname); + kfree(data);
> > > + mutex_unlock(&prf_mod_lock);
> > > + return NOTIFY_OK;
> > > + }
> > > +
> > > + WRITE_ONCE(data->mod, mod);
> > > +
> > > + list_add_tail_rcu(&data->link, &prf_mod_list);
> > > + mutex_unlock(&prf_mod_lock);
> > > + }
> > > + }
> > > + if (event == MODULE_STATE_GOING) {
> > > + /* remove module from the list */
> > > + rcu_read_lock();
> > > + list_for_each_entry_rcu(data, &prf_mod_list, link) {
> > > + if (strcmp(data->mod->name, mod->name) == 0) {
> > > +
> > > + mutex_lock(&prf_mod_lock);
> > > + /* remofe from profiled modules */
> > > + list_del_rcu(&data->link);
> > > + /* mark it stale */
> > > + WRITE_ONCE(data->mod, NULL);
> > > + mutex_unlock(&prf_mod_lock);
> > > + call_rcu(&data->rcu, prf_module_free);
> > > + break;
> > > + }
> > > + }
> > > + rcu_read_unlock();
> > > + }
> > > +
> > > + return NOTIFY_OK;
> > > +}
> > > +
> > > +static struct notifier_block pgo_module_nb = {
> > > + .notifier_call = pgo_module_notifier
> > > +};
> > > +
> > > +void prf_modules_init(void)
> > > +{
> > > + register_module_notifier(&pgo_module_nb);
> > > +}
> > > +
> > > +void prf_modules_exit(void)
> > > +{
> > > + struct prf_mod_private_data *p;
> > > +
> > > + /* unsubscribe the notifier and do cleanup. */
> > > + unregister_module_notifier(&pgo_module_nb);
> > > +
> > > + rcu_read_lock();
> > > + list_for_each_entry_rcu(p, &prf_mod_list, link) {
> > > + /* delete nodes */
> > > + list_del(&p->link);
> > > + WRITE_ONCE(p->mod, NULL);
> > > + call_rcu(&p->rcu, prf_module_free);
> > > + }
> > > + rcu_read_unlock();
> > > +}
> > > \ No newline at end of file
> > > diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
> > > index 464b3bc77431..98cfa11a7b76 100644
> > > --- a/kernel/pgo/instrument.c
> > > +++ b/kernel/pgo/instrument.c
> > > @@ -139,11 +139,11 @@ EXPORT_SYMBOL(__llvm_profile_instrument_target);
> > >
> > > /* Counts the number of times a range of targets values are seen. */
> > > void __llvm_profile_instrument_range(u64 target_value, void *data,
> > >
> > > - u32 index, s64 precise_start,
> > > - s64 precise_last, s64 large_value);
> > > + u32 index, s64 precise_start,
> > > + s64 precise_last, s64
> > > large_value);
> > >
> > > void __llvm_profile_instrument_range(u64 target_value, void *data,
> > >
> > > - u32 index, s64 precise_start,
> > > - s64 precise_last, s64 large_value)
> > > + u32 index, s64 precise_start,
> > > + s64 precise_last, s64
> > > large_value)
> > >
> > > {
> > >
> > > if (large_value != S64_MIN && (s64)target_value >= large_value)
> > >
> > > target_value = large_value;
> > >
> > > @@ -176,9 +176,9 @@ static u64 inst_prof_get_range_rep_value(u64 value)
> > >
> > > * defined in compiler-rt/include/profile/InstrProfData.inc.
> > > */
> > >
> > > void __llvm_profile_instrument_memop(u64 target_value, void *data,
> > >
> > > - u32 counter_index);
> > > + u32 counter_index);
> > >
> > > void __llvm_profile_instrument_memop(u64 target_value, void *data,
> > >
> > > - u32 counter_index)
> > > + u32 counter_index)
> > >
> > > {
> > >
> > > u64 rep_value;
> > >
> > > diff --git a/kernel/pgo/pgo.h b/kernel/pgo/pgo.h
> > > index a9ff51abbfd5..2840da63c7cd 100644
> > > --- a/kernel/pgo/pgo.h
> > > +++ b/kernel/pgo/pgo.h
> > > @@ -212,17 +212,13 @@ struct prf_mod_private_data {
> > >
> > > struct list_head link;
> > > struct rcu_head rcu;
> > >
> > > - void *buffer;
> > > - unsigned long size;
> > > -
> > > - char mod_name[MODULE_NAME_LEN];
> > >
> > > struct module *mod;
> > > struct dentry *file;
> > >
> > > int current_node;
> > >
> > > };
> > >
> > > -/* Mutex protecting the prf_mod_list and entries */
> > > +/* Mutex protecting the prf_mod_list */
> > >
> > > extern struct mutex prf_mod_lock;
> > >
> > > /* List of modules profiled */
> > >
> > > @@ -231,10 +227,7 @@ extern struct list_head prf_mod_list;
> > >
> > > extern void prf_modules_init(void);
> > > extern void prf_modules_exit(void);
> > >
> > > -/* Update each modules snapshot of the profiling data. */
> > > -extern int prf_modules_snapshot(void);
> > > -
> > > -/* below funcs are required by prf_modules_snapshot() */
> > > +/* below funcs are required by prf_modules */
> > >
> > > extern u32 __prf_get_value_size(struct llvm_prf_data *p, u32
> > > *value_kinds);
> > >
> > > extern void prf_serialize_value(struct llvm_prf_data *p, void
> > > **buffer);
> > >
> > > --
> > > 2.31.1
Kirjoitit tiistaina 1. kesäkuuta 2021 20.27.01 EEST:
> On Tue, Jun 1, 2021 at 6:26 AM <[email protected]> wrote:
> > Kirjoitit tiistaina 1. kesäkuuta 2021 11.33.48 EEST:
> > > On Mon, May 31, 2021 at 12:09 PM Nathan Chancellor <[email protected]>
> >
> > wrote:
> > > > On Fri, May 28, 2021 at 11:08:21PM +0300, Jarmo Tiitto wrote:
> > > > > PGO profile data is exported from the kernel by
> > > > > creating debugfs files: pgo/<module>.profraw for each module.
> > > >
> > > > Again, I do not really have many comments on the actual code as I am
> >
> > not
> >
> > > > super familiar with it.
> > > >
> > > > However, fs_mod.c duplicates a lot of the functions in fs.c, which
> >
> > makes
> >
> > > > maintaining this code even more difficult, especially against LLVM PGO
> > > > profile data format changes. I just want to make sure I understand
> >
> > this:
> > > > does PGO currently not work with modules? Or does this patch series
> >
> > just
> >
> > > > make it so that each module has its own .profraw file so individual
> > > > modules can be optimized?
> > > >
> > > > If it is the latter, what is the point? Why would you want to optimize
> > > > just a module and not the entire kernel, if you already have to go
> > > > through the profiling steps?
> > > >
> > > > If it is the former, there has to be a better way to share more of the
> > > > machinery between fs.c and fs_mod.c than basically duplicating
> > > > everything because there are some parameters and logic that have to
> > > > change. I do not have a ton of time to outline exactly what that might
> > > > look like but for example, prf_fill_header and prf_module_fill_header
> > > > are basically the same thing aside from the mod parameter and the
> > > > prf_..._count() calls. It seems like if mod was NULL, you would call
> >
> > the
> >
> > > > vmlinux versions of the functions.
> > >
> > > Functions definitely shouldn't be duplicated with only minor changes.
> > > We should determine a way to combine them.
> > >
> > > As for whether the original PGO patch supports profiling in modules,
> > > the answer is "it depends". :-) I believe that clang inserts profiling
> > > hooks into all code that's compiled with the "-fprofile..." flags.
> > > This would include the modules of course. In GCOV, it's possible to
> > > retrieve profiling information for a single file. Jarmo, is that the
> > > intention of your patches?
> > >
> > > -bw
> >
> > Thanks for replying Nathan and Bill!
> >
> > My original motivation for writing this patch was the realization that no
> > profile data could be obtained from modules using the original patch only.
> >
> > My insight when testing the original patch was that the compiler indeed
> > does
> > instrument+profile all code, including any loaded modules. But this is
> > where
> > the current instrument.c falls short:
> > The allocate_node() function may consume nodes from __llvm_prf_vnds_start
> > that are actually in a module and not in vmlinux.
> > So llvm_prf_data *p argument may point into an module section, and not
> > into
> > __llvm_prf_data_start range.
> >
> > This can result in early exhaustion of of nodes for vmlinux and less
> > accurate
> > profile data. I think this is actually a bug in the original patch.
> >
> > So no, the PGO does not currently work with modules. And it somewhat works
> > for vmlinux.
>
> Hi Jarmo,
> Thanks for the series! Would you mind including the above in a cover letter
> in a v2? (You can use --cover-letter command line arg to `git format-patch`
> to generate a stub). The please explicitly cc
> Sami Tolvanen <[email protected]>
> Bill Wendling <[email protected]>
> on the series? Finally, please specify the cover letter and all patch files
> to git send-email in one command, so that the individual patch files are
> linked on lore.kernel.org. This makes it significantly easier to review and
> test.
>
Hello,
Yeah, I realized afterwards that I screwed up at the git send-mail/message
threading task. Sorry about that. I will correct all of it in my next v2
patch. Make mistakes, and learn new things. ????
I will post new v2 patch once I'm done writing and testing it. Based on the
feed back here I will try keep it simple and unify the vmlinux + modules code
such that there is no fs_mod.c source any more nor necessary code duplication.
Basically it will be an rewrite on my part but I'm just excited to do it.
I feel this first attempt was more like of RFC/prototype such that I could get
in contact with you guys.
Just one question about copyrights: do I need to add my statement to the
sources, if yes, then how should I proceed ?
Regards,
Jarmo Tiitto.
> > My code confines the llvm_prf_value_node reservation to vmlinux or module
> > in
> > instrument.c based on where the llvm_prf_data *p argument points into.
> >
> > My next intention with the patch is that vmlinux and each loadable module
> > exports their own, separate profile data file in debugfs/pgo/ like the
> > vmlinux
> > already does.
> > So no file level information like in gcov. Only per whole "module.ko" and
> > the
> > vmlinux binary.
> > This way you can inspect what each module is doing individually using
> > "llvm-
> > profdata show mod.profraw"
> >
> > For PGO final build I intended combining the profile data from vmlinux and
> > all
> > modules with "llvm-profdata merge" into single profile for optimized
> > build.
> >
> > I agree fully that the current code duplication is bad.
> > Maybe I should pass in the mod->prf_xxx sections in a struct to the
> > serializing functions?
> > In that way, the struct can be initialized from either module or the
> > vmlinux
> > sections and all serializing code can share the same code.
> >
> > Either way, thanks to your questions and info I can try an write better
> > version.????
> >
> > I have learned a lot, thanks.
> > -Jarmo Tiitto
> >
On Tue, Jun 1, 2021 at 1:46 PM <[email protected]> wrote:
>
> Kirjoitit tiistaina 1. kesäkuuta 2021 20.27.01 EEST:
>
> > Hi Jarmo,
> > Thanks for the series! Would you mind including the above in a cover letter
> > in a v2? (You can use --cover-letter command line arg to `git format-patch`
> > to generate a stub). The please explicitly cc
> > Sami Tolvanen <[email protected]>
> > Bill Wendling <[email protected]>
> > on the series? Finally, please specify the cover letter and all patch files
> > to git send-email in one command, so that the individual patch files are
> > linked on lore.kernel.org. This makes it significantly easier to review and
> > test.
> >
>
> Hello,
>
> Yeah, I realized afterwards that I screwed up at the git send-mail/message
> threading task. Sorry about that. I will correct all of it in my next v2
> patch. Make mistakes, and learn new things.
No worries; best way to learn to swim is to jump in the pool!
(Well...I might not actually recommend that to kids, but you catch the
drift; maybe "sink or swim" is the better expression?). Also, you
should use text/plain for your email; you're probably getting
automated responses from LKML about that. In gmail, you can click the
vertical ellipses in the bottom right of a reply; make sure to check
"Plain Text."
> I will post new v2 patch once I'm done writing and testing it. Based on the
> feed back here I will try keep it simple and unify the vmlinux + modules code
> such that there is no fs_mod.c source any more nor necessary code duplication.
>
> Basically it will be an rewrite on my part but I'm just excited to do it.
Better to rewrite it now rather than later, I suppose.
> I feel this first attempt was more like of RFC/prototype such that I could get
> in contact with you guys.
Yep, that's common for v1 of patches. If you're interested in Clang
Built Linux generally, send me your github account name and I'll add
you to our org on github.
https://github.com/ClangBuiltLinux
> Just one question about copyrights: do I need to add my statement to the
> sources, if yes, then how should I proceed ?
Sure, you can add them to the top level of each source file you touch
(we don't do this for Makefiles I think). I think the signed-off-by
tag is enough though, which implies agreement with the Developer's
Certificate of Origin:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#developer-s-certificate-of-origin-1-1.
For instance, I don't think I ever have added my name/copyright to the
top of a file, but that also has to do with my employment agreement I
have with my employer. IANAL
--
Thanks,
~Nick Desaulniers