Re: [PATCH v2 00/12] Add build ID to stacktraces
Quoting peter enderborg (2021-03-25 04:06:17) > On 3/24/21 9:55 AM, Christoph Hellwig wrote: > > On Tue, Mar 23, 2021 at 07:04:31PM -0700, Stephen Boyd wrote: > >> x5 : x4 : 0001 > >> x3 : 0008 x2 : ff93fef25a70 > >> x1 : ff93fef15788 x0 : ffe3622352e0 > >> Call trace: > >> lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] > >> direct_entry+0x16c/0x1b4 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] > > Yikes. No, please do not make the backtraces a complete mess for > > something that serves absolutely no need. It serves a need. Please look at the patches to understand that I'm adding the buildid to automatically find the associated debug information on distros. > > Would a "verbose" flag be acceptable solution? Something like write 1 to > /sys/kernel/debug/verbose_stack to get the extra info. > > I think I see a need for it. > Or a kernel config option and a commandline parameter? That would be OK for me as I said on v1 of this series. I'll add that in for the next patch series given all the distaste for some more hex characters next to the module name. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 00/12] Add build ID to stacktraces
Quoting peter enderborg (2021-03-25 04:14:31) > > el0_sync_compat_handler+0xa8/0xcc > > el0_sync_compat+0x178/0x180 > > ---[ end trace 3d95032303e59e68 ]--- > > How will this work with the ftrace? > It won't affect ftrace, if that's the question you're asking. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 2/3] kernel/resource: make walk_mem_res() find all busy IORESOURCE_MEM resources
It used to be true that we can have system RAM (IORESOURCE_SYSTEM_RAM |
IORESOURCE_BUSY) only on the first level in the resource tree. However,
this is no longer holds for driver-managed system RAM (i.e., added via
dax/kmem and virtio-mem), which gets added on lower levels, for example,
inside device containers.
IORESOURCE_SYSTEM_RAM is defined as IORESOURCE_MEM | IORESOURCE_SYSRAM
and just a special type of IORESOURCE_MEM.
The function walk_mem_res() only consideres the first level and is
used in arch/x86/mm/ioremap.c:__ioremap_check_mem() only. We currently
fail to identify System RAM added by dax/kmem and virtio-mem as
"IORES_MAP_SYSTEM_RAM", for example, allowing for remapping of such
"normal RAM" in __ioremap_caller().
Let's find all IORESOURCE_MEM | IORESOURCE_BUSY resources, making the
function behave similar to walk_system_ram_res().
Fixes: ebf71552bb0e ("virtio-mem: Add parent resource for all added "System
RAM"")
Fixes: c221c0b0308f ("device-dax: "Hotplug" persistent memory for use like
normal RAM")
Reviewed-by: Dan Williams
Cc: Andrew Morton
Cc: Greg Kroah-Hartman
Cc: Dan Williams
Cc: Daniel Vetter
Cc: Andy Shevchenko
Cc: Mauro Carvalho Chehab
Cc: Dave Young
Cc: Baoquan He
Cc: Vivek Goyal
Cc: Dave Hansen
Cc: Keith Busch
Cc: Michal Hocko
Cc: Qian Cai
Cc: Oscar Salvador
Cc: Eric Biederman
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: Tom Lendacky
Cc: Brijesh Singh
Cc: x...@kernel.org
Cc: kexec@lists.infradead.org
Signed-off-by: David Hildenbrand
---
kernel/resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 4efd6e912279..16e0c7e8ed24 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -470,7 +470,7 @@ int walk_mem_res(u64 start, u64 end, void *arg,
{
unsigned long flags = IORESOURCE_MEM | IORESOURCE_BUSY;
- return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, true,
+ return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, false,
arg, func);
}
--
2.29.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 3/3] kernel/resource: remove first_lvl / siblings_only logic
All functions that search for IORESOURCE_SYSTEM_RAM or IORESOURCE_MEM
resources now properly consider the whole resource tree, not just the first
level. Let's drop the unused first_lvl / siblings_only logic.
Remove documentation that indicates that some functions behave differently,
all consider the full resource tree now.
Reviewed-by: Dan Williams
Reviewed-by: Andy Shevchenko
Cc: Andrew Morton
Cc: Greg Kroah-Hartman
Cc: Dan Williams
Cc: Daniel Vetter
Cc: Andy Shevchenko
Cc: Mauro Carvalho Chehab
Cc: Dave Young
Cc: Baoquan He
Cc: Vivek Goyal
Cc: Dave Hansen
Cc: Keith Busch
Cc: Michal Hocko
Cc: Qian Cai
Cc: Oscar Salvador
Cc: Eric Biederman
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: Tom Lendacky
Cc: Brijesh Singh
Cc: x...@kernel.org
Cc: kexec@lists.infradead.org
Signed-off-by: David Hildenbrand
---
kernel/resource.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 16e0c7e8ed24..7e00239a023a 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -64,12 +64,8 @@ static DEFINE_RWLOCK(resource_lock);
static struct resource *bootmem_resource_free;
static DEFINE_SPINLOCK(bootmem_resource_lock);
-static struct resource *next_resource(struct resource *p, bool sibling_only)
+static struct resource *next_resource(struct resource *p)
{
- /* Caller wants to traverse through siblings only */
- if (sibling_only)
- return p->sibling;
-
if (p->child)
return p->child;
while (!p->sibling && p->parent)
@@ -81,7 +77,7 @@ static void *r_next(struct seq_file *m, void *v, loff_t *pos)
{
struct resource *p = v;
(*pos)++;
- return (void *)next_resource(p, false);
+ return (void *)next_resource(p);
}
#ifdef CONFIG_PROC_FS
@@ -330,14 +326,10 @@ EXPORT_SYMBOL(release_resource);
* of the resource that's within [@start..@end]; if none is found, returns
* -ENODEV. Returns -EINVAL for invalid parameters.
*
- * This function walks the whole tree and not just first level children
- * unless @first_lvl is true.
- *
* @start: start address of the resource searched for
* @end: end address of same resource
* @flags: flags which the resource must have
* @desc: descriptor the resource must have
- * @first_lvl: walk only the first level children, if set
* @res: return ptr, if resource found
*
* The caller must specify @start, @end, @flags, and @desc
@@ -345,9 +337,8 @@ EXPORT_SYMBOL(release_resource);
*/
static int find_next_iomem_res(resource_size_t start, resource_size_t end,
unsigned long flags, unsigned long desc,
- bool first_lvl, struct resource *res)
+ struct resource *res)
{
- bool siblings_only = true;
struct resource *p;
if (!res)
@@ -358,7 +349,7 @@ static int find_next_iomem_res(resource_size_t start,
resource_size_t end,
read_lock(_lock);
- for (p = iomem_resource.child; p; p = next_resource(p, siblings_only)) {
+ for (p = iomem_resource.child; p; p = next_resource(p)) {
/* If we passed the resource we are looking for, stop */
if (p->start > end) {
p = NULL;
@@ -369,13 +360,6 @@ static int find_next_iomem_res(resource_size_t start,
resource_size_t end,
if (p->end < start)
continue;
- /*
-* Now that we found a range that matches what we look for,
-* check the flags and the descriptor. If we were not asked to
-* use only the first level, start looking at children as well.
-*/
- siblings_only = first_lvl;
-
if ((p->flags & flags) != flags)
continue;
if ((desc != IORES_DESC_NONE) && (desc != p->desc))
@@ -402,14 +386,14 @@ static int find_next_iomem_res(resource_size_t start,
resource_size_t end,
static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end,
unsigned long flags, unsigned long desc,
-bool first_lvl, void *arg,
+void *arg,
int (*func)(struct resource *, void *))
{
struct resource res;
int ret = -EINVAL;
while (start < end &&
- !find_next_iomem_res(start, end, flags, desc, first_lvl, )) {
+ !find_next_iomem_res(start, end, flags, desc, )) {
ret = (*func)(, arg);
if (ret)
break;
@@ -431,7 +415,6 @@ static int __walk_iomem_res_desc(resource_size_t start,
resource_size_t end,
* @arg: function argument for the callback @func
* @func: callback function that is called for each
[PATCH v2 1/3] kernel/resource: make walk_system_ram_res() find all busy IORESOURCE_SYSTEM_RAM resources
It used to be true that we can have system RAM (IORESOURCE_SYSTEM_RAM |
IORESOURCE_BUSY) only on the first level in the resource tree. However,
this is no longer holds for driver-managed system RAM (i.e., added via
dax/kmem and virtio-mem), which gets added on lower levels, for example,
inside device containers.
We have two users of walk_system_ram_res(), which currently only
consideres the first level:
a) kernel/kexec_file.c:kexec_walk_resources() -- We properly skip
IORESOURCE_SYSRAM_DRIVER_MANAGED resources via
locate_mem_hole_callback(), so even after this change, we won't be
placing kexec images onto dax/kmem and virtio-mem added memory. No
change.
b) arch/x86/kernel/crash.c:fill_up_crash_elf_data() -- we're currently
not adding relevant ranges to the crash elf header, resulting in them
not getting dumped via kdump.
This change fixes loading a crashkernel via kexec_file_load() and including
dax/kmem and virtio-mem added System RAM in the crashdump on x86-64. Note
that e.g,, arm64 relies on memblock data and, therefore, always considers
all added System RAM already.
Let's find all IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY resources, making
the function behave like walk_system_ram_range().
Fixes: ebf71552bb0e ("virtio-mem: Add parent resource for all added "System
RAM"")
Fixes: c221c0b0308f ("device-dax: "Hotplug" persistent memory for use like
normal RAM")
Reviewed-by: Dan Williams
Acked-by: Baoquan He
Cc: Andrew Morton
Cc: Greg Kroah-Hartman
Cc: Dan Williams
Cc: Daniel Vetter
Cc: Andy Shevchenko
Cc: Mauro Carvalho Chehab
Cc: Dave Young
Cc: Baoquan He
Cc: Vivek Goyal
Cc: Dave Hansen
Cc: Keith Busch
Cc: Michal Hocko
Cc: Qian Cai
Cc: Oscar Salvador
Cc: Eric Biederman
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: Tom Lendacky
Cc: Brijesh Singh
Cc: x...@kernel.org
Cc: kexec@lists.infradead.org
Signed-off-by: David Hildenbrand
---
kernel/resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 627e61b0c124..4efd6e912279 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -457,7 +457,7 @@ int walk_system_ram_res(u64 start, u64 end, void *arg,
{
unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
- return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, true,
+ return __walk_iomem_res_desc(start, end, flags, IORES_DESC_NONE, false,
arg, func);
}
--
2.29.2
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 00/12] Add build ID to stacktraces
On 3/24/21 3:04 AM, Stephen Boyd wrote: > This series adds the kernel's build ID[1] to the stacktrace header printed > in oops messages, warnings, etc. and the build ID for any module that > appears in the stacktrace after the module name. The goal is to make the > stacktrace more self-contained and descriptive by including the relevant > build IDs in the kernel logs when something goes wrong. This can be used > by post processing tools like script/decode_stacktrace.sh and kernel > developers to easily locate the debug info associated with a kernel > crash and line up what line and file things started falling apart at. > > To show how this can be used I've included a patch to > decode_stacktrace.sh that downloads the debuginfo from a debuginfod > server. > > This also includes some patches to make the buildid.c file use more > const arguments and consolidate logic into buildid.c from kdump. These > are left to the end as they were mostly cleanup patches. I don't know > who exactly maintains this so I guess Andrew is the best option to merge > all this code. > > Here's an example lkdtm stacktrace on arm64. > > WARNING: CPU: 4 PID: 3255 at drivers/misc/lkdtm/bugs.c:83 > lkdtm_WARNING+0x28/0x30 [lkdtm] > Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup > uinput xt_MASQUERADE > CPU: 4 PID: 3255 Comm: bash Not tainted 5.11 #3 > aa23f7a1231c229de205662d5a9e0d4c580f19a1 > Hardware name: Google Lazor (rev3+) with KB Backlight (DT) > pstate: 0049 (nzcv daif +PAN -UAO -TCO BTYPE=--) > pc : lkdtm_WARNING+0x28/0x30 [lkdtm] > lr : lkdtm_do_action+0x24/0x40 [lkdtm] > sp : ffc0134fbca0 > x29: ffc0134fbca0 x28: ff92d53ba240 > x27: x26: > x25: x24: ffe3622352c0 > x23: 0020 x22: ffe362233366 > x21: ffe3622352e0 x20: ffc0134fbde0 > x19: 0008 x18: > x17: ff929b6536fc x16: > x15: x14: 0012 > x13: ffe380ed892c x12: ffe381d05068 > x11: x10: > x9 : 0001 x8 : ffe362237000 > x7 : x6 : > x5 : x4 : 0001 > x3 : 0008 x2 : ff93fef25a70 > x1 : ff93fef15788 x0 : ffe3622352e0 > Call trace: > lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] > direct_entry+0x16c/0x1b4 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] > full_proxy_write+0x74/0xa4 > vfs_write+0xec/0x2e8 > ksys_write+0x84/0xf0 > __arm64_sys_write+0x24/0x30 > el0_svc_common+0xf4/0x1c0 > do_el0_svc_compat+0x28/0x3c > el0_svc_compat+0x10/0x1c > el0_sync_compat_handler+0xa8/0xcc > el0_sync_compat+0x178/0x180 > ---[ end trace 3d95032303e59e68 ]--- How will this work with the ftrace? ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 00/12] Add build ID to stacktraces
On 3/24/21 9:55 AM, Christoph Hellwig wrote: > On Tue, Mar 23, 2021 at 07:04:31PM -0700, Stephen Boyd wrote: >> x5 : x4 : 0001 >> x3 : 0008 x2 : ff93fef25a70 >> x1 : ff93fef15788 x0 : ffe3622352e0 >> Call trace: >> lkdtm_WARNING+0x28/0x30 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] >> direct_entry+0x16c/0x1b4 [lkdtm ed5019fdf5e53be37cb1ba7899292d7e143b259e] > Yikes. No, please do not make the backtraces a complete mess for > something that serves absolutely no need. Would a "verbose" flag be acceptable solution? Something like write 1 to /sys/kernel/debug/verbose_stack to get the extra info. I think I see a need for it. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
