[PATCH v3 0/5] Improve stack unwind on ppc64
by Aditya Gupta
The Problem:
============
Currently crash is unable to show function arguments and local variables, as
gdb can do. And functionality for moving between frames ('up'/'down') is not
working in crash.
Crash has 'gdb passthroughs' for things gdb can do, but the gdb passthroughs
'bt', 'frame', 'info locals', 'up', 'down' are not working either, due to
gdb not getting the register values from `crash_target::fetch_registers`,
which then uses `machdep->get_cpu_reg`, which is not implemented for PPC64
Proposed Solution:
==================
Fix the gdb passthroughs by implementing "machdep->get_cpu_reg" for PPC64.
This way, "gdb mode in crash" will support this feature for both ELF and
kdump-compressed vmcore formats, while "gdb" would only have supported ELF
format
This way other features of 'gdb', such as seeing
backtraces/registers/variables/arguments/local variables, moving up and
down stack frames, can be used with any ppc64 vmcore, irrespective of
being ELF format or kdump-compressed format.
Implications on Architectures:
====================================
No architecture other than PPC64 has been affected, other than in case of
'frame' command
As mentioned in patch #2, since frame will not be prohibited, so it will print:
crash> frame
#0 <unavailable> in ?? ()
Instead of before prohibited message:
crash> frame
crash: prohibited gdb command: frame
Major change will be in 'gdb mode' on PPC64, that it will print the frames, and
local variables, instead of failing with errors showing no frame, or showing
that couldn't get PC, it will be able to give all this information.
Testing:
========
Git tree with this patch series applied:
https://github.com/adi-g15-ibm/crash/tree/stack-unwind-3
To test various gdb passthroughs:
gdb> set
gdb> set gdb on
gdb> thread
gdb> bt
gdb> info threads
gdb> info threads
gdb> info locals
gdb> info variables irq_rover_lock
gdb> info args
gdb> thread 2
gdb> set gdb off
gdb> set
gdb> set -c 6
gdb> gdb thread
gdb> bt
gdb> gdb bt
gdb> frame
gdb> up
gdb> down
gdb> info locals
Known Issues:
=============
1. In gdb mode, 'bt' might fail to show backtrace in few vmcores collected
from older kernels. This is a known issue due to register mismatch, and
its fix has been merged upstream:
Commit: https://github.com/torvalds/linux/commit/b684c09f09e7a6af3794d4233ef78581...
Fixing GDB passthroughs on other architectures
==============================================
Much of the work for making gdb passthroughs like 'gdb bt', 'gdb
thread', 'gdb info locals' etc. has been done by the patches introducing
'machdep->get_cpu_reg' and this series fixing some issues in that.
Other architectures should be able to fix these gdb functionalities by
simply implementing 'machdep->get_cpu_reg (cpu, regno, ...)'.
The reasoning behind that has been explained with a diagram in commit
description of patch #1
I will assist with my findings/observations fixing it on ppc64 whenever needed.
Additional Notes:
=================
Sorry, it took a long time to send this version. Tried fixing 'info
threads' but wasn't able to. Gave it time again, and was able to fix it
this time after multiple days of debugging.
Some other things from last version review:
* 'info rv' not working:
It's not supported in gdb, instead we need to use 'info locals rv' or
'info variables rv'
* 'info variables' command hangs... and prints nothing after hanging for long
It likely hangs due to a lot of symbols being there, and it's trying to
get all gdb's output and page it, so Control+C messes it up, but if we pass
a regex filter to limit the output, eg. info variables rq, then it doesn't
hang, and prints the variables/symbols.
Even with gdb, ie. simply running 'gdb vmlinux vmcore' also hangs due
to the lot of symbols
* making crashing thread as default in gdb:
This is implemented now, along with synchronising crash & gdb contexts, in
patch #3
* 'info threads' not working:
This turned to be due to a bug in gdb_interface. I fixed 'info
threads' in 2 patches, to simplify it, first for the gdb_interface,
and another patch for setting the context correctly in crash
* other info commands:
I tested all the info commands, in crash along with this patch.
Most of those that fail in crash are due to gdb itself not supporting
them with vmcores, and other than that is the 'info pretty' command,
which might not be needed in crash anyways
* live debugging showing only one thread:
I tried it with crash, crash shows only the current thread, ie.
itself, so it does not have information of registers for the other
CPUs. Similarly gdb does not support live kernel debugging (without
connecting to a gdbstub/QEMU etc.).
If you need I can make it show the current thread id correctly for
the one thread, but I don't think it might help much with live
debugging
Hope, I set the context, thanks for the reviews, I replied and worked
on your suggestions, but got stuck there due to 'info threads'
Changelog:
==========
V3:
+ default gdb thread will be the crashing thread, instead of being
thread '0'
+ synchronise crash cpu and gdb thread context
+ fix bug in gdb_interface, that replaced gdb's output stream, losing
output in some cases, such as info threads and extra output in info
variables
+ fix 'info threads'
RFC V2:
- removed patch implementing 'frame', 'up', 'down' in crash
- updated the cover letter by removing the mention of those commands other
than the respective gdb passthrough
Aditya Gupta (5):
ppc64: correct gdb passthroughs by implementing machdep->get_cpu_reg
remove 'frame' from prohibited commands list
synchronise cpu context changes between crash/gdb
fix gdb_interface: restore gdb's output streams at end of
gdb_interface
fix 'info threads' command
crash_target.c | 44 ++++++++++++++++
defs.h | 130 +++++++++++++++++++++++++++++++++++++++++++++++-
gdb-10.2.patch | 110 +++++++++++++++++++++++++++++++++++++++-
gdb_interface.c | 2 +-
kernel.c | 47 +++++++++++++++--
ppc64.c | 95 +++++++++++++++++++++++++++++++++--
task.c | 14 ++++++
tools.c | 2 +-
8 files changed, 434 insertions(+), 10 deletions(-)
--
2.41.0
6 months, 2 weeks
Re: [PATCH v3] add "files -n" command for an inode
by lijiang
On Thu, Nov 2, 2023 at 9:35 AM Huang Shijie <shijie(a)os.amperecomputing.com>
wrote:
> In the NUMA machine, it is useful to know the memory distribution of
> an inode page cache:
> How many pages in the node 0?
> How many pages in the node 1?
>
> Add "files -n" command to get the memory distribution information:
> 1.) Add new argument for dump_inode_page_cache_info()
> 2.) make page_to_nid() a global function.
> 3.) Add summary_inode_page() to check each page's node
> information.
> 4.) Use print_inode_summary_info() to print the
> memory distribution information of an inode.
>
> Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
> ---
> v2 --> v3:
> 1.) Always return 1 for summary_inode_page().
> 2.) Add more comment for help_files.
>
>
Thank you for the update, Shijie.
> ---
> defs.h | 1 +
> filesys.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> help.c | 14 +++++++++++++-
> memory.c | 3 +--
> 4 files changed, 63 insertions(+), 8 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 788f63a..1fe2d0b 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -5750,6 +5750,7 @@ int dump_inode_page(ulong);
> ulong valid_section_nr(ulong);
> void display_memory_from_file_offset(ulonglong, long, void *);
> void swap_info_init(void);
> +int page_to_nid(ulong);
>
> /*
> * filesys.c
> diff --git a/filesys.c b/filesys.c
> index 1d0ee7f..2c7cc74 100644
> --- a/filesys.c
> +++ b/filesys.c
> @@ -49,7 +49,7 @@ static int match_file_string(char *, char *, char *);
> static ulong get_root_vfsmount(char *);
> static void check_live_arch_mismatch(void);
> static long get_inode_nrpages(ulong);
> -static void dump_inode_page_cache_info(ulong);
> +static void dump_inode_page_cache_info(ulong, void *callback);
>
> #define DENTRY_CACHE (20)
> #define INODE_CACHE (20)
> @@ -2192,8 +2192,31 @@ get_inode_nrpages(ulong i_mapping)
> return nrpages;
> }
>
> +/* Used to collect the numa information for an inode */
> +static ulong *numa_node;
> +
> +static void
> +print_inode_summary_info(void)
> +{
> + int i;
> +
> + fprintf(fp, " NODE PAGES\n");
> + for (i = 0; i < vt->numnodes; i++)
> + fprintf(fp, " %2d %8ld\n", i, numa_node[i]);
> +}
> +
> +static int
> +summary_inode_page(ulong page)
> +{
> + int node = page_to_nid(page);
> +
> + if (0 <= node && node < vt->numnodes)
> + numa_node[node]++;
> + return 1;
> +}
>
A clear error message would be nice when the "files -n" command fails.
What's your opinion on the following changes?
+static int
+summary_inode_page(ulong page)
+{
+ int node;
+
+ if (!is_page_ptr(page, NULL))
+ error(FATAL, "Invalid inode page(0x%lx)\n", page);
+
+ node = page_to_nid(page);
+ if (node < 0 || node >= vt->numnodes)
+ error(FATAL, "Invalid node(%d) for page(0x%lx)\n", node,
page);
+
+ numa_node[node]++;
+
+ return 1;
+}
Without the above changes, it will print a lot of failures of "invalid
page" once the corresponding inode page is invalid, unless that is expected
behavior.
crash> files -n ffff8ea84c130938
INODE NRPAGES
ffff8ea84c130938 62527
files: page_to_nid: invalid page: 0
files: page_to_nid: invalid page: 0
files: page_to_nid: invalid page: 0
files: page_to_nid: invalid page: 10
files: page_to_nid: invalid page: 10
files: page_to_nid: invalid page: 10
files: page_to_nid: invalid page: 20
files: page_to_nid: invalid page: 20
files: page_to_nid: invalid page: 20
files: page_to_nid: invalid page: 30
files: page_to_nid: invalid page: 30
files: page_to_nid: invalid page: 30
files: page_to_nid: invalid page: 40
files: page_to_nid: invalid page: 40
files: page_to_nid: invalid page: 40
files: page_to_nid: invalid page: 50
files: page_to_nid: invalid page: 50
files: page_to_nid: invalid page: 50
files: page_to_nid: invalid page: 60
files: page_to_nid: invalid page: 60
...
And also says that in help.c:
+" -n inode given a hexadecimal inode address, check all the pages",
+" in the page cache, and display a NUMA node distribution",
+" if the inode page is valid, otherwise it will fail.",
Just my suggestions, any thoughts?
Thanks.
Lianbo
+
> static void
> -dump_inode_page_cache_info(ulong inode)
> +dump_inode_page_cache_info(ulong inode, void *callback)
> {
> char *inode_buf;
> ulong i_mapping, nrpages, root_rnode, xarray, count;
> @@ -2236,7 +2259,7 @@ dump_inode_page_cache_info(ulong inode)
> root_rnode = i_mapping + OFFSET(address_space_page_tree);
>
> lp.index = 0;
> - lp.value = (void *)&dump_inode_page;
> + lp.value = callback;
>
> if (root_rnode)
> count = do_radix_tree(root_rnode, RADIX_TREE_DUMP_CB, &lp);
> @@ -2276,7 +2299,7 @@ cmd_files(void)
> ref = NULL;
> refarg = NULL;
>
> - while ((c = getopt(argcnt, args, "d:R:p:c")) != EOF) {
> + while ((c = getopt(argcnt, args, "d:n:R:p:c")) != EOF) {
> switch(c)
> {
> case 'R':
> @@ -2295,11 +2318,31 @@ cmd_files(void)
> display_dentry_info(value);
> return;
>
> + case 'n':
> + if (VALID_MEMBER(address_space_page_tree) &&
> + VALID_MEMBER(inode_i_mapping)) {
> + value = htol(optarg, FAULT_ON_ERROR, NULL);
> +
> + /* Allocate the array for this inode */
> + numa_node = malloc(sizeof(ulong) *
> vt->numnodes);
> + BZERO(numa_node, sizeof(ulong) *
> vt->numnodes);
> +
> + dump_inode_page_cache_info(value, (void
> *)&summary_inode_page);
> +
> + /* Print out the NUMA node information for
> this inode */
> + print_inode_summary_info();
> +
> + free(numa_node);
> + numa_node = NULL;
> + } else
> + option_not_supported('n');
> + return;
> +
> case 'p':
> if (VALID_MEMBER(address_space_page_tree) &&
> VALID_MEMBER(inode_i_mapping)) {
> value = htol(optarg, FAULT_ON_ERROR, NULL);
> - dump_inode_page_cache_info(value);
> + dump_inode_page_cache_info(value, (void
> *)&dump_inode_page);
> } else
> option_not_supported('p');
> return;
> diff --git a/help.c b/help.c
> index cc7ab20..e9e28b7 100644
> --- a/help.c
> +++ b/help.c
> @@ -7850,7 +7850,7 @@ NULL
> char *help_files[] = {
> "files",
> "open files",
> -"[-d dentry] | [-p inode] | [-c] [-R reference] [pid | taskp] ... ",
> +"[-d dentry] | [-p inode] | [-n inode] | [-c] [-R reference] [pid |
> taskp] ... ",
> " This command displays information about open files of a context.",
> " It prints the context's current root directory and current working",
> " directory, and then for each open file descriptor it prints a pointer",
> @@ -7863,6 +7863,8 @@ char *help_files[] = {
> " specific, and only shows the data requested.\n",
> " -d dentry given a hexadecimal dentry address, display its inode,",
> " super block, file type, and full pathname.",
> +" -n inode given a hexadecimal inode address, check all the pages",
> +" in the page cache, and display a NUMA node
> distribution.",
> " -p inode given a hexadecimal inode address, dump all of its
> pages",
> " that are in the page cache.",
> " -c for each open file descriptor, prints a pointer to its",
> @@ -7974,6 +7976,16 @@ char *help_files[] = {
> " ca1ddde0 2eeef000 f59b91ac 3 2 82c
> referenced,uptodate,lru,private",
> " ca36b300 3b598000 f59b91ac 4 2 82c
> referenced,uptodate,lru,private",
> " ca202680 30134000 f59b91ac 5 2 82c
> referenced,uptodate,lru,private",
> +" ",
> +" For the inode at address ffff07ff8c6f97f8, display the NUMA node",
> +" distribution of its pages that are in the page cache:",
> +" %s> files -n ffff07ff8c6f97f8",
> +" INODE NRPAGES",
> +" ffff07ff8c6f97f8 25240",
> +" ",
> +" NODE PAGES",
> +" 0 25240",
> +" 1 0",
> " ",
> NULL
> };
> diff --git a/memory.c b/memory.c
> index 86ccec5..ed1a4fb 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -300,7 +300,6 @@ static int dump_vm_event_state(void);
> static int dump_page_states(void);
> static int generic_read_dumpfile(ulonglong, void *, long, char *, ulong);
> static int generic_write_dumpfile(ulonglong, void *, long, char *, ulong);
> -static int page_to_nid(ulong);
> static int get_kmem_cache_list(ulong **);
> static int get_kmem_cache_root_list(ulong **);
> static int get_kmem_cache_child_list(ulong **, ulong);
> @@ -19846,7 +19845,7 @@ is_kmem_cache_addr_common(ulong vaddr, char *kbuf)
> /*
> * Kernel-config-neutral page-to-node evaluator.
> */
> -static int
> +int
> page_to_nid(ulong page)
> {
> int i;
> --
> 2.40.1
>
>
6 months, 2 weeks
[PATCH] help.c: Document kmem -l a|i|ic|id
by Li Zhijian
Per the soucde code, '-l' requires extra arguments but it's missing in
documentation.
Signed-off-by: Li Zhijian <lizhijian(a)fujitsu.com>
---
help.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/help.c b/help.c
index cc7ab20e343e..568b8f3b151a 100644
--- a/help.c
+++ b/help.c
@@ -6888,8 +6888,9 @@ char *help_kmem[] = {
" members of the associated page struct are displayed.",
" address when used with -c, the address must be a page pointer address;",
" the page_hash_table entry containing the page is displayed.",
-" address when used with -l, the address must be a page pointer address;",
+" address when used with [-l a|i|ic|id], the address must be a page pointer address;",
" the page address is displayed if it is contained with the list.",
+" where a:active_list, i:inactive_list, ic:inactive_clean_list, id:inactive_dirty_list",
" address when used with -v, the address can be a mapped kernel virtual",
" address or physical address; the mapped region containing the",
" address is displayed.\n",
--
2.29.2
6 months, 2 weeks
[PATCH V2 1/2] Add a new helper function get_value_vmcoreinfo
by Huang Shijie
Add get_value_vmcoreinfo() to get the value for @name in vmcoreinfo.
1.) add macro GET_SYMBOL/GET_OFFSET/GET_SIZE to simplify the code.
2.) use get_value_vmcoreinfo to simplify the arm64 code.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
v1 -->v2:
1.) more powerful get_value_vmcoreinfo().
2.) I only replace the code in arm64, since I do not have
boards for other ARCHs, such as x86/ppc.
---
arm64.c | 87 ++++++------------------
defs.h | 10 +++
kernel.c | 198 ++++++++++++++++++++++--------------------------------
symbols.c | 7 +-
4 files changed, 113 insertions(+), 189 deletions(-)
diff --git a/arm64.c b/arm64.c
index 2b6b0e5..e131102 100644
--- a/arm64.c
+++ b/arm64.c
@@ -160,11 +160,8 @@ arm64_init(int when)
if (!ms->kimage_voffset && STREQ(pc->live_memsrc, "/dev/crash"))
ioctl(pc->mfd, DEV_CRASH_ARCH_DATA, &ms->kimage_voffset);
- if (!ms->kimage_voffset &&
- (string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) {
- ms->kimage_voffset = htol(string, QUIET, NULL);
- free(string);
- }
+ if (!ms->kimage_voffset)
+ get_value_vmcoreinfo("kimage_voffset", &ms->kimage_voffset, NUMBER_H_TYPE);
if (ms->kimage_voffset ||
(ACTIVE() && (symbol_value_from_proc_kallsyms("kimage_voffset") != BADVAL))) {
@@ -443,9 +440,8 @@ arm64_init(int when)
arm64_get_section_size_bits();
if (!machdep->max_physmem_bits) {
- if ((string = pc->read_vmcoreinfo("NUMBER(MAX_PHYSMEM_BITS)"))) {
- machdep->max_physmem_bits = atol(string);
- free(string);
+ if (get_value_vmcoreinfo("MAX_PHYSMEM_BITS", &machdep->max_physmem_bits, NUMBER_TYPE)) {
+ /* nothing */;
} else if (machdep->machspec->VA_BITS == 52) /* guess */
machdep->max_physmem_bits = _MAX_PHYSMEM_BITS_52;
else if (THIS_KERNEL_VERSION >= LINUX(3,17,0))
@@ -572,19 +568,6 @@ static int arm64_get_struct_page_max_shift(struct machine_specific *ms)
return (int)ceil(log2(ms->struct_page_size));
}
-/* Return TRUE if we succeed, return FALSE on failure. */
-static int arm64_get_vmcoreinfo_ul(unsigned long *vaddr, const char* label)
-{
- char *string = pc->read_vmcoreinfo(label);
-
- if (!string)
- return FALSE;
-
- *vaddr = strtoul(string, NULL, 0);
- free(string);
- return TRUE;
-}
-
/*
* The change is caused by the kernel patch since v5.18-rc1:
* "arm64: crash_core: Export MODULES, VMALLOC, and VMEMMAP ranges"
@@ -594,21 +577,21 @@ static struct kernel_range *arm64_get_range_v5_18(struct machine_specific *ms)
struct kernel_range *r = &tmp_range;
/* Get the MODULES_VADDR ~ MODULES_END */
- if (!arm64_get_vmcoreinfo_ul(&r->modules_vaddr, "NUMBER(MODULES_VADDR)"))
+ if (!get_value_vmcoreinfo("MODULES_VADDR", &r->modules_vaddr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->modules_end, "NUMBER(MODULES_END)"))
+ if (!get_value_vmcoreinfo("MODULES_END", &r->modules_end, NUMBER_H_TYPE))
return NULL;
/* Get the VMEMMAP_START ~ VMEMMAP_END */
- if (!arm64_get_vmcoreinfo_ul(&r->vmemmap_vaddr, "NUMBER(VMEMMAP_START)"))
+ if (!get_value_vmcoreinfo("VMEMMAP_START", &r->vmemmap_vaddr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->vmemmap_end, "NUMBER(VMEMMAP_END)"))
+ if (!get_value_vmcoreinfo("VMEMMAP_END", &r->vmemmap_end, NUMBER_H_TYPE))
return NULL;
/* Get the VMALLOC_START ~ VMALLOC_END */
- if (!arm64_get_vmcoreinfo_ul(&r->vmalloc_start_addr, "NUMBER(VMALLOC_START)"))
+ if (!get_value_vmcoreinfo("VMALLOC_START", &r->vmalloc_start_addr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->vmalloc_end, "NUMBER(VMALLOC_END)"))
+ if (!get_value_vmcoreinfo("VMALLOC_END", &r->vmalloc_end, NUMBER_H_TYPE))
return NULL;
return r;
@@ -888,12 +871,7 @@ range_failed:
/* Get the size of struct page {} */
static void arm64_get_struct_page_size(struct machine_specific *ms)
{
- char *string;
-
- string = pc->read_vmcoreinfo("SIZE(page)");
- if (string)
- ms->struct_page_size = atol(string);
- free(string);
+ get_value_vmcoreinfo("page", &ms->struct_page_size, SIZE_TYPE);
}
/*
@@ -1469,16 +1447,12 @@ arm64_calc_phys_offset(void)
physaddr_t paddr;
ulong vaddr;
struct syment *sp;
- char *string;
if ((machdep->flags & NEW_VMEMMAP) &&
ms->kimage_voffset && (sp = kernel_symbol_search("memstart_addr"))) {
if (pc->flags & PROC_KCORE) {
- if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
- ms->phys_offset = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("PHYS_OFFSET", &ms->phys_offset, NUMBER_H_TYPE))
return;
- }
vaddr = symbol_value_from_proc_kallsyms("memstart_addr");
if (vaddr == BADVAL)
vaddr = sp->value;
@@ -1560,9 +1534,8 @@ arm64_get_section_size_bits(void)
} else
machdep->section_size_bits = _SECTION_SIZE_BITS;
- if ((string = pc->read_vmcoreinfo("NUMBER(SECTION_SIZE_BITS)"))) {
- machdep->section_size_bits = atol(string);
- free(string);
+ if (get_value_vmcoreinfo("SECTION_SIZE_BITS", &machdep->section_size_bits, NUMBER_TYPE)) {
+ /* nothing */
} else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL)) == IKCONFIG_Y) {
if ((ret = get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS", &string)) == IKCONFIG_STR)
@@ -1581,15 +1554,11 @@ arm64_get_section_size_bits(void)
static int
arm64_kdump_phys_base(ulong *phys_offset)
{
- char *string;
struct syment *sp;
physaddr_t paddr;
- if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
- *phys_offset = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("PHYS_OFFSET", phys_offset, NUMBER_H_TYPE))
return TRUE;
- }
if ((machdep->flags & NEW_VMEMMAP) &&
machdep->machspec->kimage_voffset &&
@@ -4592,10 +4561,9 @@ static int
arm64_set_va_bits_by_tcr(void)
{
ulong value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)")) ||
- (string = pc->read_vmcoreinfo("NUMBER(tcr_el1_t1sz)"))) {
+ if (get_value_vmcoreinfo("TCR_EL1_T1SZ", &value, NUMBER_H_TYPE) ||
+ get_value_vmcoreinfo("tcr_el1_t1sz", &value, NUMBER_H_TYPE)) {
/* See ARMv8 ARM for the description of
* TCR_EL1.T1SZ and how it can be used
* to calculate the vabits_actual
@@ -4604,10 +4572,9 @@ arm64_set_va_bits_by_tcr(void)
* Basically:
* vabits_actual = 64 - T1SZ;
*/
- value = 64 - strtoll(string, NULL, 0);
+ value = 64 - value;
if (CRASHDEBUG(1))
fprintf(fp, "vmcoreinfo : vabits_actual: %ld\n", value);
- free(string);
machdep->machspec->VA_BITS_ACTUAL = value;
machdep->machspec->VA_BITS = value;
machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
@@ -4623,13 +4590,8 @@ arm64_calc_VA_BITS(void)
int bitval;
struct syment *sp;
ulong vabits_actual, value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(VA_BITS)"))) {
- value = atol(string);
- free(string);
- machdep->machspec->CONFIG_ARM64_VA_BITS = value;
- }
+ get_value_vmcoreinfo("VA_BITS", &machdep->machspec->CONFIG_ARM64_VA_BITS, NUMBER_TYPE);
if (kernel_symbol_exists("vabits_actual")) {
if (pc->flags & PROC_KCORE) {
@@ -4754,10 +4716,8 @@ arm64_calc_virtual_memory_ranges(void)
ulong PUD_SIZE = UNINITIALIZED;
if (!machdep->machspec->CONFIG_ARM64_VA_BITS) {
- if ((string = pc->read_vmcoreinfo("NUMBER(VA_BITS)"))) {
- value = atol(string);
- free(string);
- machdep->machspec->CONFIG_ARM64_VA_BITS = value;
+ if (get_value_vmcoreinfo("VA_BITS", &ms->CONFIG_ARM64_VA_BITS, NUMBER_TYPE)) {
+ /* nothing */
} else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
if ((ret = get_kernel_config("CONFIG_ARM64_VA_BITS",
&string)) == IKCONFIG_STR)
@@ -4852,11 +4812,8 @@ arm64_swp_offset(ulong pte)
static void arm64_calc_KERNELPACMASK(void)
{
ulong value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
- value = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("KERNELPACMASK", &value, NUMBER_H_TYPE)) {
machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
if (CRASHDEBUG(1))
fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", value);
diff --git a/defs.h b/defs.h
index 1fe2d0b..282fcc1 100644
--- a/defs.h
+++ b/defs.h
@@ -6055,6 +6055,16 @@ int hide_offline_cpu(int);
int get_highest_cpu_online(void);
int get_highest_cpu_present(void);
int get_cpus_to_display(void);
+enum vmcore_type {
+ SYMBOL_TYPE,
+ OFFSET_TYPE,
+ NUMBER_TYPE,
+ SIZE_TYPE,
+ LENGTH_TYPE,
+ NUMBER_H_TYPE,
+ MAX_TYPE,
+};
+bool get_value_vmcoreinfo(const char *name, ulong *v, enum vmcore_type type);
void get_log_from_vmcoreinfo(char *file);
int in_cpu_map(int, int);
void paravirt_init(void);
diff --git a/kernel.c b/kernel.c
index 6dcf414..3f3d908 100644
--- a/kernel.c
+++ b/kernel.c
@@ -104,6 +104,46 @@ static void check_vmcoreinfo(void);
static int is_pvops_xen(void);
static int get_linux_banner_from_vmlinux(char *, size_t);
+static char *type_names[MAX_TYPE] = {
+ "SYMBOL",
+ "OFFSET",
+ "NUMBER", /* for NUMBER_TYPE */
+ "SIZE",
+ "LENGTH",
+ "NUMBER", /* for NUMBER_H_TYPE */
+};
+
+/* Return TRUE if we succeed, return FALSE on failure. */
+bool
+get_value_vmcoreinfo(const char *name, ulong *v, enum vmcore_type type)
+{
+ char buf[64] = {};
+ char *string;
+
+ if (type >= MAX_TYPE)
+ return FALSE;
+
+ sprintf(buf, "%s(%s)", type_names[type], name);
+ string = pc->read_vmcoreinfo(buf);
+ if (!string)
+ return FALSE;
+
+ switch (type) {
+ case SYMBOL_TYPE:
+ case NUMBER_H_TYPE:
+ *v = htol(string, RETURN_ON_ERROR, NULL);
+ break;
+ case OFFSET_TYPE:
+ case NUMBER_TYPE:
+ case SIZE_TYPE:
+ *v = dtol(string, RETURN_ON_ERROR, NULL);
+ break;
+ }
+
+ free(string);
+ return TRUE;
+}
+
/*
* popuplate the global kernel table (kt) with kernel version
* information parsed from UTSNAME/OSRELEASE string
@@ -10984,6 +11024,22 @@ hypervisor_init(void)
fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
}
+#define GET_SYMBOL(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), SYMBOL_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "SYMBOL(" s "): %lx\n", v); \
+ }
+#define GET_OFFSET(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), OFFSET_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "OFFSET(" s "): %lx\n", v); \
+ }
+#define GET_SIZE(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), SIZE_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "SIZE(" s "): %lx\n", v); \
+ }
+
/*
* Get and display the kernel log buffer using the vmcoreinfo
* data alone without the vmlinux file.
@@ -11024,125 +11080,29 @@ get_log_from_vmcoreinfo(char *file)
} else
error(FATAL, "VMCOREINFO: cannot determine page size\n");
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
- vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf): %lx\n",
- vmc->log_buf_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
- vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_end): %lx\n",
- vmc->log_end_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
- vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
- vmc->log_buf_len_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
- vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(logged_chars): %lx\n",
- vmc->logged_chars_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
- vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
- vmc->log_first_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
- vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
- vmc->log_next_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
- vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(phys_base): %lx\n",
- vmc->phys_base_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(_stext): %lx\n",
- vmc->_stext_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.ts_nsec)"))) {
- vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.ts_nsec): %ld\n",
- vmc->log_ts_nsec_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.ts_nsec)"))) {
- vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.ts_nsec): %ld\n",
- vmc->log_ts_nsec_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.len)"))) {
- vmc->log_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.len): %ld\n",
- vmc->log_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.len)"))) {
- vmc->log_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.len): %ld\n",
- vmc->log_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.text_len)"))) {
- vmc->log_text_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.text_len): %ld\n",
- vmc->log_text_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.text_len)"))) {
- vmc->log_text_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.text_len): %ld\n",
- vmc->log_text_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.dict_len)"))) {
- vmc->log_dict_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.dict_len): %ld\n",
- vmc->log_dict_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.dict_len)"))) {
- vmc->log_dict_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.dict_len): %ld\n",
- vmc->log_dict_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SIZE(log)"))) {
- vmc->log_SIZE = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SIZE(log): %ld\n", vmc->log_SIZE);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("SIZE(printk_log)"))) {
- vmc->log_SIZE = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SIZE(printk_log): %ld\n", vmc->log_SIZE);
- free(string);
- }
+ GET_SYMBOL("log_buf", vmc->log_buf_SYMBOL);
+ GET_SYMBOL("log_end", vmc->log_end_SYMBOL);
+ GET_SYMBOL("log_buf_len", vmc->log_buf_len_SYMBOL);
+ GET_SYMBOL("logged_chars", vmc->logged_chars_SYMBOL);
+ GET_SYMBOL("log_first_idx", vmc->log_first_idx_SYMBOL);
+ GET_SYMBOL("log_next_idx", vmc->log_next_idx_SYMBOL);
+ GET_SYMBOL("phys_base", vmc->phys_base_SYMBOL);
+ GET_SYMBOL("_stext", vmc->_stext_SYMBOL);
+
+ GET_OFFSET("log.ts_nsec", vmc->log_ts_nsec_OFFSET)
+ else GET_OFFSET("printk_log.ts_nsec", vmc->log_ts_nsec_OFFSET);
+
+ GET_OFFSET("log.len", vmc->log_len_OFFSET)
+ else GET_OFFSET("printk_log.len", vmc->log_len_OFFSET);
+
+ GET_OFFSET("log.text_len", vmc->log_text_len_OFFSET)
+ else GET_OFFSET("printk_log.text_len", vmc->log_text_len_OFFSET);
+
+ GET_OFFSET("log.dict_len", vmc->log_dict_len_OFFSET)
+ else GET_OFFSET("printk_log.dict_len", vmc->log_dict_len_OFFSET);
+
+ GET_SIZE("log", vmc->log_SIZE)
+ else GET_SIZE("printk_log", vmc->log_SIZE);
/*
* The per-arch VTOP() macro must be functional.
diff --git a/symbols.c b/symbols.c
index 8e8b4c3..20e598d 100644
--- a/symbols.c
+++ b/symbols.c
@@ -632,11 +632,8 @@ kaslr_init(void)
!machine_type("S390X") && !machine_type("RISCV64")) || (kt->flags & RELOC_SET))
return;
- if (!kt->vmcoreinfo._stext_SYMBOL &&
- (string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- kt->vmcoreinfo._stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- free(string);
- }
+ if (!kt->vmcoreinfo._stext_SYMBOL)
+ get_value_vmcoreinfo("_stext", &kt->vmcoreinfo._stext_SYMBOL, SYMBOL_TYPE);
/*
* --kaslr=auto
--
2.40.1
6 months, 2 weeks
[PATCH V2 1/2] RISCV64: Dump NT_PRSTATUS in 'help -n'
by Song Shuai
With the patch we can get full dump of "struct elf_prstatus" in 'help -n':
```
crash> help -n
<snip>
Elf64_Nhdr:
n_namesz: 5 ("CORE")
n_descsz: 376
n_type: 1 (NT_PRSTATUS)
si.signo: 0 si.code: 0 si.errno: 0
cursig: 0 sigpend: 0 sighold: 0
pid: 1 ppid: 0 pgrp: 0 sid:0
utime: 0.000000 stime: 0.000000
cutime: 0.000000 cstime: 0.000000
epc: ffffffff8000a1dc ra: ffffffff800af958 sp: ff6000001fc501c0
gp: ffffffff81515d38 tp: ff600000000d8000 t0: 6666666666663c5b
t1: ff600000000d88c8 t2: 666666666666663c s0: ff6000001fc50320
s1: ffffffff815170d8 a0: ff6000001fc501c8 a1: c0000000ffffefff
a2: 0000000000000000 a3: 0000000000000001 a4: 0000000000000000
a5: ff60000001782c00 a6: 000000000130e0f0 a7: 0000000000000000
s2: ffffffff81517820 s3: ff6000001fc501c8 s4: 000000000000000f
s5: 0000000000000000 s6: ff20000000013e60 s7: 0000000000000000
s8: ff60000000861000 s9: 00007fffc3641694 s10: 00007fffc3641690
s11: 00005555796ed240 t3: 0000000000010297 t4: ffffffff80c17810
t5: ffffffff8195e7b8 t6: ff6000001fc50048
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000001 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
0000000000000000 0000000000000000
ffffffff8000a1dc ffffffff800af958
ff6000001fc501c0 ffffffff81515d38
ff600000000d8000 6666666666663c5b
<snip>
```
Signed-off-by: Song Shuai <songshuaishuai(a)tinylab.org>
---
Changes since V1:
https://lore.kernel.org/kexec/20231204105015.2341055-1-songshuaishuai@tin...
- patch1: add the missing "s5-s7" format string back
- patch2: add Acked-by from Kazuhito Hagio
- Resend to the new crash list address
---
netdump.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/netdump.c b/netdump.c
index 3907863..32586b6 100644
--- a/netdump.c
+++ b/netdump.c
@@ -2578,6 +2578,8 @@ dump_Elf64_Nhdr(Elf64_Off offset, int store)
display_ELF_note(EM_PPC64, PRSTATUS_NOTE, note, nd->ofp);
if (machine_type("ARM64") && (note->n_type == NT_PRSTATUS))
display_ELF_note(EM_AARCH64, PRSTATUS_NOTE, note, nd->ofp);
+ if (machine_type("RISCV64") && (note->n_type == NT_PRSTATUS))
+ display_ELF_note(EM_RISCV, PRSTATUS_NOTE, note, nd->ofp);
}
for (i = lf = 0; i < note->n_descsz/sizeof(ulonglong); i++) {
if (((i%2)==0)) {
@@ -3399,6 +3401,80 @@ display_prstatus_arm64(void *note_ptr, FILE *ofp)
space(sp), pr->pr_reg[33], pr->pr_fpvalid);
}
+struct riscv64_elf_siginfo {
+ int si_signo;
+ int si_code;
+ int si_errno;
+};
+
+struct riscv64_elf_prstatus {
+ struct riscv64_elf_siginfo pr_info;
+ short pr_cursig;
+ unsigned long pr_sigpend;
+ unsigned long pr_sighold;
+ pid_t pr_pid;
+ pid_t pr_ppid;
+ pid_t pr_pgrp;
+ pid_t pr_sid;
+ struct timeval pr_utime;
+ struct timeval pr_stime;
+ struct timeval pr_cutime;
+ struct timeval pr_cstime;
+/* elf_gregset_t pr_reg; => typedef struct user_regs_struct elf_gregset_t; */
+ unsigned long pr_reg[32];
+ int pr_fpvalid;
+};
+
+static void
+display_prstatus_riscv64(void *note_ptr, FILE *ofp)
+{
+ struct riscv64_elf_prstatus *pr;
+ Elf64_Nhdr *note;
+ int sp;
+
+ note = (Elf64_Nhdr *)note_ptr;
+ pr = (struct riscv64_elf_prstatus *)(
+ (char *)note + sizeof(Elf64_Nhdr) + note->n_namesz);
+ pr = (struct riscv64_elf_prstatus *)roundup((ulong)pr, 4);
+ sp = nd->num_prstatus_notes ? 25 : 22;
+
+ fprintf(ofp,
+ "%ssi.signo: %d si.code: %d si.errno: %d\n"
+ "%scursig: %d sigpend: %lx sighold: %lx\n"
+ "%spid: %d ppid: %d pgrp: %d sid:%d\n"
+ "%sutime: %01lld.%06d stime: %01lld.%06d\n"
+ "%scutime: %01lld.%06d cstime: %01lld.%06d\n",
+ space(sp), pr->pr_info.si_signo, pr->pr_info.si_code, pr->pr_info.si_errno,
+ space(sp), pr->pr_cursig, pr->pr_sigpend, pr->pr_sighold,
+ space(sp), pr->pr_pid, pr->pr_ppid, pr->pr_pgrp, pr->pr_sid,
+ space(sp), (long long)pr->pr_utime.tv_sec, (int)pr->pr_utime.tv_usec,
+ (long long)pr->pr_stime.tv_sec, (int)pr->pr_stime.tv_usec,
+ space(sp), (long long)pr->pr_cutime.tv_sec, (int)pr->pr_cutime.tv_usec,
+ (long long)pr->pr_cstime.tv_sec, (int)pr->pr_cstime.tv_usec);
+ fprintf(ofp,
+ "%sepc: %016lx ra: %016lx sp: %016lx\n"
+ "%s gp: %016lx tp: %016lx t0: %016lx\n"
+ "%s t1: %016lx t2: %016lx s0: %016lx\n"
+ "%s s1: %016lx a0: %016lx a1: %016lx\n"
+ "%s a2: %016lx a3: %016lx a4: %016lx\n"
+ "%s a5: %016lx a6: %016lx a7: %016lx\n"
+ "%s s2: %016lx s3: %016lx s4: %016lx\n"
+ "%s s5: %016lx s6: %016lx s7: %016lx\n"
+ "%s s8: %016lx s9: %016lx s10: %016lx\n"
+ "%ss11: %016lx t3: %016lx t4: %016lx\n"
+ "%s t5: %016lx t6: %016lx\n",
+ space(sp), pr->pr_reg[0], pr->pr_reg[1], pr->pr_reg[2],
+ space(sp), pr->pr_reg[3], pr->pr_reg[4], pr->pr_reg[5],
+ space(sp), pr->pr_reg[6], pr->pr_reg[7], pr->pr_reg[8],
+ space(sp), pr->pr_reg[9], pr->pr_reg[10], pr->pr_reg[11],
+ space(sp), pr->pr_reg[12], pr->pr_reg[13], pr->pr_reg[14],
+ space(sp), pr->pr_reg[15], pr->pr_reg[16], pr->pr_reg[17],
+ space(sp), pr->pr_reg[18], pr->pr_reg[19], pr->pr_reg[20],
+ space(sp), pr->pr_reg[21], pr->pr_reg[22], pr->pr_reg[23],
+ space(sp), pr->pr_reg[24], pr->pr_reg[25], pr->pr_reg[26],
+ space(sp), pr->pr_reg[27], pr->pr_reg[28], pr->pr_reg[29],
+ space(sp), pr->pr_reg[30], pr->pr_reg[31]);
+}
void
display_ELF_note(int machine, int type, void *note, FILE *ofp)
@@ -3449,6 +3525,14 @@ display_ELF_note(int machine, int type, void *note, FILE *ofp)
break;
}
break;
+ case EM_RISCV:
+ switch (type)
+ {
+ case PRSTATUS_NOTE:
+ display_prstatus_riscv64(note, ofp);
+ break;
+ }
+ break;
default:
return;
--
2.20.1
6 months, 2 weeks
[PATCH 0/3] s390x: uncouple physical and virtual memory spaces
by Alexander Gordeev
Hi all,
Currently physical and virtual addresses are the same on S390X,
but in the future it is going to be uncoupled just like on any
other architecture. This series supports that feature, but it
does not impact the current and older kernel versions.
Patch 2 is basically NOP and only fix semantics.
Patch 3 uses the feature if it is available in kernel.
Thanks!
Alexander Gordeev (3):
Fix identity_map_base value dump on S390
s390x: fix virtual vs physical address confusion
s390x: uncouple physical and virtual memory spaces
defs.h | 20 ++++-
s390.c | 2 +-
s390x.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 255 insertions(+), 12 deletions(-)
--
2.40.1
6 months, 2 weeks
Re: [External Mail]Re: [BUG FIXED]fix bug of CACHED in kmem -i show memory
by 薛国伦
Hi Erlandsson:
your patch work well for this problem， thanks a lot.
I am think about that if kernel version without change [2] but google use clang to built, it may unusable for this problem.
- [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit...
Thanks!
Best regards
________________________________
From: Johan.Erlandsson(a)sony.com <Johan.Erlandsson(a)sony.com>
Sent: Friday, December 1, 2023 4:55:08 PM
To: 薛国伦; HAGIO KAZUHITO(萩尾 一仁); devel(a)lists.crash-utility.osci.io
Cc: Lianbo Jiang
Subject: Re: [Crash-utility] Re: [External Mail]Re: [BUG FIXED]fix bug of CACHED in kmem -i show memory
[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec(a)xiaomi.xn--com-iw3ew31vyqjqpq
Hi
I have also seen this problem on GKI kernel [1]. They are for instance
built with clang, might have an impact. So the 'nr_swapper_spaces'
appear to be optimized out. Probably because of change [2].
I made a change to try handle the case, patch attached.
Regards
Johan
- [1] https://source.android.com/docs/core/architecture/kernel/gki-android13-5_...
- [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit...
________________________________________
From: 薛国伦 <xueguolun(a)xiaomi.com>
Sent: Friday, 1 December 2023 09:07
To: HAGIO KAZUHITO(萩尾 一仁); devel(a)lists.crash-utility.osci.io
Cc: Lianbo Jiang
Subject: [Crash-utility] Re: [External Mail]Re: [BUG FIXED]fix bug of CACHED in kmem -i show memory
Hi kazu: 1. I know that kernel have nr_swapper_spaces since kernel 4. 11 which brought into kernel with struct address_space *swapper_spaces[] my team think GKI vmlinux did not have nr_swapper_spaces this symbol, may due to compile rule changed
ZjQcmQRYFpfptBannerStart
Caution : This email originated from outside of Sony.
Do not click links or open any attachments unless you recognize the sender and know the content is safe. Please report phishing if unsure.
ZjQcmQRYFpfptBannerEnd
Hi kazu:
1. I know that kernel have nr_swapper_spaces since kernel 4.11 which brought into kernel with struct address_space *swapper_spaces[]
my team think GKI vmlinux did not have nr_swapper_spaces this symbol, may due to compile rule changed in AOSP code.
we will try to find the root cause of vmlinux without symbol name nr_swappera_space.
>> RELEASE: 6.1.25-android14-11-maybe-dirty-qki-consolidate
>> crash> nr_swapper_spaces
>> crash: command not found: nr_swapper_spaces
2. I printf log of crash when execute kmem -i, it run into branch of "symbol_exists("swapper_spaces")", also can not find nr_swapper_spaces
if (symbol_exists("nr_swapper_spaces") {
......
} else if (symbol_exists("swapper_spaces") {
......
} else if (symbol_exists("swapper_space") {
......
}
3. using my patch, it can fix output log of kmem -i which CACHED show not right.
Fix before:
crash> kmem -i
PAGES TOTAL PERCENTAGE
TOTAL MEM 2854115 10.9 GB ----
FREE 169699 662.9 MB 5% of TOTAL MEM
USED 2684416 10.2 GB 94% of TOTAL MEM
SHARED 891094 3.4 GB 31% of TOTAL MEM
BUFFERS 329 1.3 MB 0% of TOTAL MEM
CACHED 873327085626 3331478.4 GB 30598875% of TOTAL MEM
SLAB 230128 898.9 MB 8% of TOTAL MEM
Fix after:
crash> kmem -i
PAGES TOTAL PERCENTAGE
TOTAL MEM 2854115 10.9 GB ----
FREE 169699 662.9 MB 5% of TOTAL MEM
USED 2684416 10.2 GB 94% of TOTAL MEM
SHARED 891094 3.4 GB 31% of TOTAL MEM
BUFFERS 329 1.3 MB 0% of TOTAL MEM
CACHED 1729018 6.6 GB 60% of TOTAL MEM
SLAB 230128 898.9 MB 8% of TOTAL MEM
I think crash tools without my patch, maybe miss some situation like this which symbol miss in kenrel vmlinux.
I think that using both branch of symbol_exist("nr_swapper_spaces") and symbol_exists("swapper_spaces") means to double check
the situation of all kinds of kernel and vmlinux.
It seen that bring my patch can deal with more unusual situation and make crash tool more compatible.
Thanks！
Best Regards
________________________________
From: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
Sent: Friday, December 1, 2023 3:08:03 PM
To: 薛国伦; devel(a)lists.crash-utility.osci.io
Cc: Lianbo Jiang
Subject: Re: [External Mail]Re: [Crash-utility] [BUG FIXED]fix bug of CACHED in kmem -i show memory
[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec(a)xiaomi.xn--com-iw3ew31vyqjqpq
On 2023/11/30 22:05, 薛国伦 wrote:
> Hi kazu:
>
>
> 1. I found that vmlinux of kernel-6.1 did not have symbol nr_swapper_spaces but only have swapper_spaces
>
> Also check in GKI vmlinux, can not find nr_swapper_spaces
Hmm, upstream kernel 6.1 has it.
$ git show v6.1:mm/swap_state.c
struct address_space *swapper_spaces[MAX_SWAPFILES] __read_mostly;
static unsigned int nr_swapper_spaces[MAX_SWAPFILES] __read_mostly;
>
> It may have some problem in symbol of nr_swapper_spaces
>
>
> 2. I think that crash tools will check nr_swapper_spaces first and then check *swapper_spaces, swaper_space[] last.
>
> so first check nr_swapper_spaces not found, crash tool can use *swapper_spaces to enhance compatibility.
>
> The patch i send can resolve this situation which nr_swapper_spaces symbols can not found.
But I cannot determine whether your patch is correct and should be
applied, without kernel patches or the cause of no nr_swapper_spaces.
Could you please find the information (e.g. links) of the related kernel
patches?
Thanks,
Kazu
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
6 months, 3 weeks
Re: [PATCH 2/2] RISCV64: Fix 'bt' output when no ra on the stack top
by HAGIO KAZUHITO(萩尾 一仁)
Resending to the new crash list address (without kexec list)
On 2023/12/04 19:50, Song Shuai wrote:
> Same as the Linux commit f766f77a74f5 ("riscv/stacktrace: Fix
> stack output without ra on the stack top").
>
> When a function doesn't have a callee, then it will not
> push ra into the stack, such as lkdtm functions, so
> correct the FP of the second frame and use pt_regs to get
> the right PC of the second frame.
>
> TEST:
>
> Before this patch, the `bt -f` outputs only the first frame with
> the wrong PC and FP of next frame:
>
> ```
> crash> bt -f
> PID: 1 TASK: ff600000000e0000 CPU: 1 COMMAND: "sh"
> #0 [ff20000000013cf0] lkdtm_EXCEPTION at ffffffff805303c0
> [PC: ffffffff805303c0 RA: ff20000000013d10 SP: ff20000000013cf0 SIZE: 16] <- wrong next PC
> ff20000000013cf0: 0000000000000001 ff20000000013d10 <- next FP
> ff20000000013d00: ff20000000013d40
> crash>
> ```
> After this patch, the `bt` outputs the full frames:
>
> ```
> crash> bt
> PID: 1 TASK: ff600000000e0000 CPU: 1 COMMAND: "sh"
> #0 [ff20000000013cf0] lkdtm_EXCEPTION at ffffffff805303c0
> #1 [ff20000000013d00] lkdtm_do_action at ffffffff8052fe36
> #2 [ff20000000013d10] direct_entry at ffffffff80530018
> #3 [ff20000000013d40] full_proxy_write at ffffffff80305044
> #4 [ff20000000013d80] vfs_write at ffffffff801b68b4
> #5 [ff20000000013e30] ksys_write at ffffffff801b6c4a
> #6 [ff20000000013e80] __riscv_sys_write at ffffffff801b6cc4
> #7 [ff20000000013e90] do_trap_ecall_u at ffffffff80836798
> crash>
> ```
> Signed-off-by: Song Shuai <songshuaishuai(a)tinylab.org>
Looks good.
Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
Thanks,
Kazu
> ---
> riscv64.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/riscv64.c b/riscv64.c
> index 0aaa14b..872be59 100644
> --- a/riscv64.c
> +++ b/riscv64.c
> @@ -747,11 +747,14 @@ riscv64_back_trace_cmd(struct bt_info *bt)
> {
> struct riscv64_unwind_frame current, previous;
> struct stackframe curr_frame;
> + struct riscv64_register * regs;
> int level = 0;
>
> if (bt->flags & BT_REGS_NOT_FOUND)
> return;
>
> + regs = (struct riscv64_register *) bt->machdep;
> +
> current.pc = bt->instptr;
> current.sp = bt->stkptr;
> current.fp = bt->frameptr;
> @@ -788,8 +791,16 @@ riscv64_back_trace_cmd(struct bt_info *bt)
> sizeof(curr_frame), "get stack frame", RETURN_ON_ERROR))
> return;
>
> - previous.pc = curr_frame.ra;
> - previous.fp = curr_frame.fp;
> + /* correct PC and FP of the second frame when the first frame has no callee */
> +
> + if (regs && (regs->regs[RISCV64_REGS_EPC] == current.pc) && curr_frame.fp & 0x7){
> + previous.pc = regs->regs[RISCV64_REGS_RA];
> + previous.fp = curr_frame.ra;
> + } else {
> + previous.pc = curr_frame.ra;
> + previous.fp = curr_frame.fp;
> + }
> +
> previous.sp = current.fp;
>
> riscv64_dump_backtrace_entry(bt, symbol, ¤t, &previous, level++);
6 months, 3 weeks
Re: [PATCH 1/2] RISCV64: Dump NT_PRSTATUS in 'help -n'
by HAGIO KAZUHITO(萩尾 一仁)
Resending to the new crash list address (without kexec list)
On 2023/12/04 19:50, Song Shuai wrote:
> With the patch we can get full dump of "struct elf_prstatus" in 'help -n':
>
> ```
> crash> help -n
> <snip>
> Elf64_Nhdr:
> n_namesz: 5 ("CORE")
> n_descsz: 376
> n_type: 1 (NT_PRSTATUS)
> si.signo: 0 si.code: 0 si.errno: 0
> cursig: 0 sigpend: 0 sighold: 0
> pid: 0 ppid: 0 pgrp: 0 sid:0
> utime: 0.000000 stime: 0.000000
> cutime: 0.000000 cstime: 0.000000
> epc: ffffffff80822426 ra: ffffffff80822422 sp: ff20000000093f20
> gp: ffffffff814ee2a8 tp: ff60000000104800 t0: ff2000000027bb18
> t1: 0000000000000088 t2: 0000000000000000 s0: ff20000000093f30
> s1: 0000000000000001 a0: 0000000000000004 a1: 0000000000000008
> a2: ff6000009e9f0000 a3: 0000000000005fe4 a4: ffffffff80c1c290
> a5: 0000000000000000 a6: ff6000001f60a6b0 a7: 00000088e4450548
> s2: ffffffff814ef220 s3: 0000000000000002 s4: 000000000000003f
> s8: ffffffff814ef3d8 s9: 0000000000000000 s10: ffffffff814ef0d0
> s11: ffffffff81525d10 t3: ffffffff80c1d840 t4: 0000000000000000
> t5: 0000000000000001 t6: 00000000000000aa
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> 0000000000000000 0000000000000000
> ffffffff80822426 ffffffff80822422
> ff20000000093f20 ffffffff814ee2a8
> ff60000000104800 ff2000000027bb18
> <snip>
> ```
>
> Signed-off-by: Song Shuai <songshuaishuai(a)tinylab.org>
> ---
> netdump.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/netdump.c b/netdump.c
> index 3907863..b266b27 100644
> --- a/netdump.c
> +++ b/netdump.c
> @@ -2578,6 +2578,8 @@ dump_Elf64_Nhdr(Elf64_Off offset, int store)
> display_ELF_note(EM_PPC64, PRSTATUS_NOTE, note, nd->ofp);
> if (machine_type("ARM64") && (note->n_type == NT_PRSTATUS))
> display_ELF_note(EM_AARCH64, PRSTATUS_NOTE, note, nd->ofp);
> + if (machine_type("RISCV64") && (note->n_type == NT_PRSTATUS))
> + display_ELF_note(EM_RISCV, PRSTATUS_NOTE, note, nd->ofp);
> }
> for (i = lf = 0; i < note->n_descsz/sizeof(ulonglong); i++) {
> if (((i%2)==0)) {
> @@ -3399,6 +3401,79 @@ display_prstatus_arm64(void *note_ptr, FILE *ofp)
> space(sp), pr->pr_reg[33], pr->pr_fpvalid);
> }
>
> +struct riscv64_elf_siginfo {
> + int si_signo;
> + int si_code;
> + int si_errno;
> +};
> +
> +struct riscv64_elf_prstatus {
> + struct riscv64_elf_siginfo pr_info;
> + short pr_cursig;
> + unsigned long pr_sigpend;
> + unsigned long pr_sighold;
> + pid_t pr_pid;
> + pid_t pr_ppid;
> + pid_t pr_pgrp;
> + pid_t pr_sid;
> + struct timeval pr_utime;
> + struct timeval pr_stime;
> + struct timeval pr_cutime;
> + struct timeval pr_cstime;
> +/* elf_gregset_t pr_reg; => typedef struct user_regs_struct elf_gregset_t; */
> + unsigned long pr_reg[32];
> + int pr_fpvalid;
> +};
> +
> +static void
> +display_prstatus_riscv64(void *note_ptr, FILE *ofp)
> +{
> + struct riscv64_elf_prstatus *pr;
> + Elf64_Nhdr *note;
> + int sp;
> +
> + note = (Elf64_Nhdr *)note_ptr;
> + pr = (struct riscv64_elf_prstatus *)(
> + (char *)note + sizeof(Elf64_Nhdr) + note->n_namesz);
> + pr = (struct riscv64_elf_prstatus *)roundup((ulong)pr, 4);
> + sp = nd->num_prstatus_notes ? 25 : 22;
> +
> + fprintf(ofp,
> + "%ssi.signo: %d si.code: %d si.errno: %d\n"
> + "%scursig: %d sigpend: %lx sighold: %lx\n"
> + "%spid: %d ppid: %d pgrp: %d sid:%d\n"
> + "%sutime: %01lld.%06d stime: %01lld.%06d\n"
> + "%scutime: %01lld.%06d cstime: %01lld.%06d\n",
> + space(sp), pr->pr_info.si_signo, pr->pr_info.si_code, pr->pr_info.si_errno,
> + space(sp), pr->pr_cursig, pr->pr_sigpend, pr->pr_sighold,
> + space(sp), pr->pr_pid, pr->pr_ppid, pr->pr_pgrp, pr->pr_sid,
> + space(sp), (long long)pr->pr_utime.tv_sec, (int)pr->pr_utime.tv_usec,
> + (long long)pr->pr_stime.tv_sec, (int)pr->pr_stime.tv_usec,
> + space(sp), (long long)pr->pr_cutime.tv_sec, (int)pr->pr_cutime.tv_usec,
> + (long long)pr->pr_cstime.tv_sec, (int)pr->pr_cstime.tv_usec);
> + fprintf(ofp,
> + "%sepc: %016lx ra: %016lx sp: %016lx\n"
> + "%s gp: %016lx tp: %016lx t0: %016lx\n"
> + "%s t1: %016lx t2: %016lx s0: %016lx\n"
> + "%s s1: %016lx a0: %016lx a1: %016lx\n"
> + "%s a2: %016lx a3: %016lx a4: %016lx\n"
> + "%s a5: %016lx a6: %016lx a7: %016lx\n"
> + "%s s2: %016lx s3: %016lx s4: %016lx\n"
> + "%s s8: %016lx s9: %016lx s10: %016lx\n"
> + "%ss11: %016lx t3: %016lx t4: %016lx\n"
> + "%s t5: %016lx t6: %016lx\n",
10 lines.
> + space(sp), pr->pr_reg[0], pr->pr_reg[1], pr->pr_reg[2],
> + space(sp), pr->pr_reg[3], pr->pr_reg[4], pr->pr_reg[5],
> + space(sp), pr->pr_reg[6], pr->pr_reg[7], pr->pr_reg[8],
> + space(sp), pr->pr_reg[9], pr->pr_reg[10], pr->pr_reg[11],
> + space(sp), pr->pr_reg[12], pr->pr_reg[13], pr->pr_reg[14],
> + space(sp), pr->pr_reg[15], pr->pr_reg[16], pr->pr_reg[17],
> + space(sp), pr->pr_reg[18], pr->pr_reg[19], pr->pr_reg[20],
> + space(sp), pr->pr_reg[21], pr->pr_reg[22], pr->pr_reg[23],
> + space(sp), pr->pr_reg[24], pr->pr_reg[25], pr->pr_reg[26],
> + space(sp), pr->pr_reg[27], pr->pr_reg[28], pr->pr_reg[29],
> + space(sp), pr->pr_reg[30], pr->pr_reg[31]);
11 lines. It looks short of the format string. (s5 - s7?)
$ make warn TARGET=RISCV64
...
cc -c -g -DRISCV64 -DGDB_10_2 netdump.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
netdump.c: In function 'display_prstatus_riscv64':
netdump.c:3455:3: warning: too many arguments for format [-Wformat-extra-args]
"%sepc: %016lx ra: %016lx sp: %016lx\n"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Thanks,
Kazu
> +}
>
> void
> display_ELF_note(int machine, int type, void *note, FILE *ofp)
> @@ -3449,6 +3524,14 @@ display_ELF_note(int machine, int type, void *note, FILE *ofp)
> break;
> }
> break;
> + case EM_RISCV:
> + switch (type)
> + {
> + case PRSTATUS_NOTE:
> + display_prstatus_riscv64(note, ofp);
> + break;
> + }
> + break;
>
> default:
> return;
6 months, 3 weeks
Re: [BUG FIXED]fix bug of CACHED in kmem -i show memory
by lijiang
On Thu, Dec 7, 2023 at 6:25 PM Johan.Erlandsson(a)sony.com <
Johan.Erlandsson(a)sony.com> wrote:
> >>>>> (gdb) p nr_swapper_spaces
> >>>>> $4 = <optimized out>
> >>>> Thank you folks, for the various information.
> >>>>
> >>>> Apparently the data of nr_swapper_spaces exists in the kernel, but its
> >>>> symbol does not exist. I think this means that we cannot calculate
> the
> >>>> number of pages from swapper_spaces[] with such a debuginfo, because
> we
> >>>> cannot get the array size of a swapper_spaces entry.
> >>> That's correct, Kazu.
> >>>
> >>>
> >>>> So I'd like to go with Johan's patch.
> >>>>
> >>>> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_10_2 memory.c -Wall -O2
> >>>> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> >>>> -Wformat-security
> >>>> memory.c: In function dump_kmeminfo:
> >>>> memory.c:8612:50: warning: pointer targets in passing argument 2 of
> >>>> dump_vm_stat differ in signedness [-Wpointer-sign]
> >>>> 8612 | if (dump_vm_stat("NR_SWAPCACHE",
> >>>> &swapper_space_nrpages, 0)) {
> >>>> |
> >>>> ^~~~~~~~~~~~~~~~~~~~~~
> >>>> | |
> >>>> | ulong *
> {aka
> >>>> long unsigned int *}
> >>>> memory.c:298:33: note: expected long int * but argument is of type
> ulong
> >>>> * {aka long unsigned int *}
> >>>> 298 | static int dump_vm_stat(char *, long *, ulong);
> >>>> | ^~~~~~
> >>>>
> >>>> So with the following,
> >>>>
> >>>> - ulong swapper_space_nrpages;
> >>>> + long swapper_space_nrpages;
> >>> Looks good. Maybe the patch log also needs to be improved a little bit.
> >>>
> >>> Although the checking of kernel version looks not very good, seems no
> >>> better way.
> >> hmm, this sounds strange. My ack is for Johan's patch (attached), which
> >
> >Ah, I did not see the attached patch. Thank you for pointing out this
> >issue, Kazu.
> >
> >> does not have kernel version check, is this ok? sorry for confusing.
> >
> >Sounds good idea if there is no kernel version check.
> >
> >I still have one question: the following two commits were introduced
> >separately in kernel 5.12 and 4.11.
> >
> >b6038942480e ("mm: memcg: add swapcache stat for memcg v2")
> >
> >4b3ef9daa4fc ("mm/swap: split swap cache into 64MB trunks")
> >
> >It indicates that the attached patch may not work well between kernel
> >4.11 and 5.12, is it possible to encounter the similar issue(the
> >nr_swapper_spaces may be optimized away)?
>
> I have not noticed any problems with the attached patch on
> earlier kernel versions. For instance on GKI kernels based on 5.10
> [1] the 'nr_swapper_spaces' variable is not optimized out.
>
>
Thank you for the feedback, Johan. It makes sense, let's leave it there if
the above issue does not occur in the actual production environment.
I have no other issues, for Johan's patch: Ack.
Thanks.
Lianbo
> - [1]
> https://source.android.com/docs/core/architecture/kernel/gki-android13-5_...
>
> Regards
> Johan
6 months, 3 weeks
