Re: [PATCH v6 05/13] module: Add printk formats to add module build ID to stacktraces
From: Petr Mladek <pmladek@suse.com>
Date: 2021-05-11 11:35:27
Also in:
lkml
On Mon 2021-05-10 17:38:37, Stephen Boyd wrote:
Let's make kernel stacktraces easier to identify by including the build ID[1] of a module if the stacktrace is printing a symbol from a module.
quoted hunk ↗ jump to hunk
--- a/include/linux/module.h +++ b/include/linux/module.h@@ -11,6 +11,7 @@ #include <linux/list.h> #include <linux/stat.h> +#include <linux/buildid.h> #include <linux/compiler.h> #include <linux/cache.h> #include <linux/kmod.h>@@ -364,6 +365,11 @@ struct module { /* Unique handle for this module */ char name[MODULE_NAME_LEN]; +#ifdef CONFIG_STACKTRACE_BUILD_ID + /* Module build ID */ + unsigned char build_id[BUILD_ID_SIZE_MAX]; +#endif + /* Sysfs stuff. */ struct module_kobject mkobj; struct module_attribute *modinfo_attrs;@@ -627,7 +633,7 @@ void *dereference_module_function_descriptor(struct module *mod, void *ptr); const char *module_address_lookup(unsigned long addr, unsigned long *symbolsize, unsigned long *offset, - char **modname, + char **modname, const unsigned char **modbuildid,
Nit: It would have been better to follow the existing style and put only one
parameter per-line.
Please, fix it in the next version if there is any. But no need
to resend it just because of this unless Jessica asks for it.
char *namebuf); int lookup_module_symbol_name(unsigned long addr, char *symname); int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
quoted hunk ↗ jump to hunk
diff --git a/kernel/module.c b/kernel/module.c index 30479355ab85..a5eaf5188a83 100644 --- a/kernel/module.c +++ b/kernel/module.c@@ -4235,7 +4257,7 @@ void * __weak dereference_module_function_descriptor(struct module *mod, const char *module_address_lookup(unsigned long addr, unsigned long *size, unsigned long *offset, - char **modname, + char **modname, const unsigned char **modbuildid,
Same here.
char *namebuf)
{
const char *ret = NULL;Otherwise, it looks good. Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr