Re: [Crash-utility] [PATCH] Fix pvops Xen detection for arm machine
by lijiang
Hi, Qi
Thank you for the fix.
> Date: Thu, 16 Dec 2021 15:09:49 +0800
> From: Qi Zheng <zhengqi.arch(a)bytedance.com>
> To: k-hagio-ab(a)nec.com, ptesarik(a)suse.com, lijiang(a)redhat.com
> Cc: Qi Zheng <zhengqi.arch(a)bytedance.com>, crash-utility(a)redhat.com
> Subject: [Crash-utility] [PATCH] Fix pvops Xen detection for arm
> machine
> Message-ID: <20211216070949.17668-1-zhengqi.arch(a)bytedance.com>
> Content-Type: text/plain; charset="US-ASCII"
>
> Since the xen_start_info on the arm/arm64 platform is static defined:
>
> ./arm/xen/enlighten.c:40:static struct start_info _xen_start_info;
> ./arm/xen/enlighten.c:41:struct start_info *xen_start_info = &_xen_start_info;
> ./arm/xen/enlighten.c:42:EXPORT_SYMBOL(xen_start_info);
>
The above variable is statically defined in the kernel, but the
'xen_vcpu_info' is also a static variable, why does the former not
work on arm machines, but the latter works well? Could you please
explain more details?
[linux]$ grep -nr "xen_vcpu_info" *
arch/arm/xen/enlighten.c:51:static struct vcpu_info __percpu *xen_vcpu_info;
Thanks.
Lianbo
> The is_pvops_xen() in commit 4badc6229c69f5cd9da7eb7bdf400a53ec6db01a
> ("Fix pvops Xen detection for kernels >= v4.20") always return TRUE.
> Then the following error will be reported because p2m_mid_missing
> and xen_p2m_addr are not defined:
>
> crash: cannot resolve "p2m_top"
>
> For the arm/arm64 platform, fix it by using xen_vcpu_info instead of
> xen_start_info to detect Xen dumps.
>
> Signed-off-by: Qi Zheng <zhengqi.arch(a)bytedance.com>
> ---
> kernel.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/kernel.c b/kernel.c
> index f4598ea..4b4d789 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -10757,11 +10757,18 @@ is_pvops_xen(void)
> STREQ(sym, "paravirt_patch_default")))
> return TRUE;
>
> - if (symbol_exists("xen_start_info") &&
> - readmem(symbol_value("xen_start_info"), KVADDR, &addr,
> - sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
> - addr != 0)
> + if (machine_type("ARM") || machine_type("ARM64")) {
> + if (symbol_exists("xen_vcpu_info") &&
> + readmem(symbol_value("xen_vcpu_info"), KVADDR, &addr,
> + sizeof(void *), "xen_vcpu_info", RETURN_ON_ERROR) &&
> + addr != 0)
> + return TRUE;
> + } else if (symbol_exists("xen_start_info") &&
> + readmem(symbol_value("xen_start_info"), KVADDR, &addr,
> + sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
> + addr != 0) {
> return TRUE;
> + }
>
> return FALSE;
> }
> --
> 2.11.0
2 years, 11 months
[PATCH 0/4] A few makefile cleanups
by Sven Schnelle
Hi List,
i was looking into why crash doesn't compile with multiple jobs
and prepared a few patches to make that work. While doing that,
i also changed a few other things along the way.
Sven Schnelle (4):
make: set --no-print-directory once
extensions: fix defs.h dependency
make: use -C instead of (cd x; make)
make: replace make by $(MAKE)
Makefile | 61 +++++++++++++++++++++++----------------------
extensions/Makefile | 6 ++---
2 files changed, 34 insertions(+), 33 deletions(-)
--
2.32.0
2 years, 11 months
[PATCH] kernel: minor clean-up in read_in_kernel_config()
by Austin Kim
The 'found' variable is 0 when declared and 'found' set to 1.
Using 'TRUE' statement rather than '1' makes routine be more readable.
Signed-off-by: Austin Kim <austindh.kim(a)gmail.com>
---
kernel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel.c b/kernel.c
index f4598ea..7935d91 100644
--- a/kernel.c
+++ b/kernel.c
@@ -10423,7 +10423,7 @@ void
read_in_kernel_config(int command)
{
struct syment *sp;
- int ii, jj, ret, end, found=0;
+ int ii, jj, ret, end, found = FALSE;
unsigned long size, bufsz;
uint64_t magic;
char *pos, *ln, *buf, *head, *tail, *val, *uncomp;
@@ -10488,7 +10488,7 @@ again:
while (tail < (buf + (size - 1))) {
if (strncmp(tail, MAGIC_END, end)==0) {
- found = 1;
+ found = TRUE;
break;
}
tail++;
--
2.20.1
2 years, 11 months
[PATCH] fix minor spelling typos in symbols.c
by Austin Kim
fix minor spelling typos in symbols.c as below:
useable => usable
higer => higher
versons => versions
Signed-off-by: Austin Kim <austindh.kim(a)gmail.com>
---
symbols.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/symbols.c b/symbols.c
index 73baa95..9ae0187 100644
--- a/symbols.c
+++ b/symbols.c
@@ -3139,7 +3139,7 @@ kallsyms_module_function_size(struct syment *sp, struct load_module *lm, ulong *
if (!(lm->mod_flags & MOD_KALLSYMS) || !(kt->flags & KALLSYMS_V2))
return FALSE;
- if (THIS_KERNEL_VERSION >= LINUX(5,0,0)) /* st_size not useable */
+ if (THIS_KERNEL_VERSION >= LINUX(5,0,0)) /* st_size not usable */
return FALSE;
module_buf = GETBUF(lm->mod_size);
@@ -7624,7 +7624,7 @@ is_percpu_symbol(struct syment *sp)
* args[0] is checked to see whether it's the name of a variable, structure,
* union, or typedef. If so, args[0] is changed to the appropriate command,
* i.e., "p", "struct", "union", or "whatis", and the original args are all
- * shifted into the next higer args[] location.
+ * shifted into the next higher args[] location.
*/
int
is_datatype_command(void)
@@ -11608,7 +11608,7 @@ calculate_load_order_v2(struct load_module *lm, bfd *bfd, int dynamic,
}
/*
- * Later versons of insmod store basic address information of each
+ * Later versions of insmod store basic address information of each
* module in a format that looks like the following example of the
* nfsd module:
*
--
2.20.1
2 years, 11 months
[PATCH] Fix pvops Xen detection for arm machine
by Qi Zheng
Since the xen_start_info on the arm/arm64 platform is static defined:
./arm/xen/enlighten.c:40:static struct start_info _xen_start_info;
./arm/xen/enlighten.c:41:struct start_info *xen_start_info = &_xen_start_info;
./arm/xen/enlighten.c:42:EXPORT_SYMBOL(xen_start_info);
The is_pvops_xen() in commit 4badc6229c69f5cd9da7eb7bdf400a53ec6db01a
("Fix pvops Xen detection for kernels >= v4.20") always return TRUE.
Then the following error will be reported because p2m_mid_missing
and xen_p2m_addr are not defined:
crash: cannot resolve "p2m_top"
For the arm/arm64 platform, fix it by using xen_vcpu_info instead of
xen_start_info to detect Xen dumps.
Signed-off-by: Qi Zheng <zhengqi.arch(a)bytedance.com>
---
kernel.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/kernel.c b/kernel.c
index f4598ea..4b4d789 100644
--- a/kernel.c
+++ b/kernel.c
@@ -10757,11 +10757,18 @@ is_pvops_xen(void)
STREQ(sym, "paravirt_patch_default")))
return TRUE;
- if (symbol_exists("xen_start_info") &&
- readmem(symbol_value("xen_start_info"), KVADDR, &addr,
- sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
- addr != 0)
+ if (machine_type("ARM") || machine_type("ARM64")) {
+ if (symbol_exists("xen_vcpu_info") &&
+ readmem(symbol_value("xen_vcpu_info"), KVADDR, &addr,
+ sizeof(void *), "xen_vcpu_info", RETURN_ON_ERROR) &&
+ addr != 0)
+ return TRUE;
+ } else if (symbol_exists("xen_start_info") &&
+ readmem(symbol_value("xen_start_info"), KVADDR, &addr,
+ sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
+ addr != 0) {
return TRUE;
+ }
return FALSE;
}
--
2.11.0
2 years, 11 months
[PATCH] Fix pvops Xen detection for arm machine
by Qi Zheng
Since the xen_start_info on the arm/arm64 platform is static defined:
./arm/xen/enlighten.c:40:static struct start_info _xen_start_info;
./arm/xen/enlighten.c:41:struct start_info *xen_start_info = &_xen_start_info;
./arm/xen/enlighten.c:42:EXPORT_SYMBOL(xen_start_info);
The is_pvops_xen() in commit 4badc6229c69f5cd9da7eb7bdf400a53ec6db01a
("Fix pvops Xen detection for kernels >= v4.20") always return TRUE.
Then the following error will be reported because p2m_mid_missing
and xen_p2m_addr are not defined:
crash: cannot resolve "p2m_top"
Fix it by using xen_vcpu_id instead of xen_start_info to detect Xen dumps.
Signed-off-by: Qi Zheng <zhengqi.arch(a)bytedance.com>
---
kernel.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/kernel.c b/kernel.c
index f4598ea..b216fbe 100644
--- a/kernel.c
+++ b/kernel.c
@@ -10745,6 +10745,9 @@ is_pvops_xen(void)
{
ulong addr;
char *sym;
+ struct syment * sp;
+ ulong ptr;
+ int i;
if (!PVOPS())
return FALSE;
@@ -10757,11 +10760,19 @@ is_pvops_xen(void)
STREQ(sym, "paravirt_patch_default")))
return TRUE;
- if (symbol_exists("xen_start_info") &&
- readmem(symbol_value("xen_start_info"), KVADDR, &addr,
- sizeof(void *), "xen_start_info", RETURN_ON_ERROR) &&
- addr != 0)
- return TRUE;
+ sp = per_cpu_symbol_search("xen_vcpu_id");
+ if (sp) {
+ for (i = 0; i < NR_CPUS; i++) {
+ if (!kt->__per_cpu_offset[i])
+ continue;
+
+ ptr = sp->value + kt->__per_cpu_offset[i];
+ if (ptr && readmem(ptr, KVADDR, &ptr, sizeof(uint32_t),
+ "xen_vcpu_id", FAULT_ON_ERROR))
+ return TRUE;
+ return FALSE;
+ }
+ }
return FALSE;
}
--
2.11.0
2 years, 11 months
[PATCH] defs.h: fix breakage of compatibility of struct machdep_table for extension modules
by d.hatayama＠fujitsu.com
Commit 2f967fb5ebd737ce5eadba462df35935122e8865 (crash_taget:
fetch_registers support) added new member get_cpu_reg in the middle of
struct machdep_table as:
@@ -1013,6 +1013,7 @@ struct machdep_table {
ulong (*processor_speed)(void);
int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
+ int (*get_cpu_reg)(int, int, const char *, int, void *);
ulong (*get_task_pgd)(ulong);
void (*dump_irq)(int);
void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
which breaks compatibility of struct machdep_table for extension
modules. In general, new member variables must be added at the end of
structures to maintain compatibility of data structures shared among
extension modules.
For example, as the result, crash gcore command results in unexpected
behavior:
crash> gcore -v 7
GETBUF(168 -> 0)
<readmem: ffff8b267dd28020, KVADDR, "task_struct.stack", 8, (FOE), 7fffaef8afa8>
<read_kdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8>
read_netdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8 offset: 767ec020
<readmem: ffffa3f740abff58, KVADDR, "genregs_get: pt_regs", 168, (FOE), e71f80>
gcore: invalid kernel virtual address: ffffa3f740abff58 type: "genregs_get: pt_regs"
vma hit rate: 0% (0 of 274308)
crash gcore uses machdep->get_stacktop() to retrieve registers saved
at the bottom of a given user-space task's kernel stack:
static int genregs_get(struct task_context *target,
const struct user_regset *regset,
unsigned int size, void *buf)
{
...snip...
pt_regs_buf = GETBUF(SIZE(pt_regs));
readmem(machdep->get_stacktop(target->task) - SIZE(pt_regs), KVADDR,
pt_regs_buf, SIZE(pt_regs), "genregs_get: pt_regs",
gcore_verbose_error_handle());
As seen above, the position of get_stacktop in struct machdep_table is
behind the position where get_cpu_reg was added:
int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong);
void (*dump_irq)(int);
void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
ulong (*get_stackbase)(ulong);
ulong (*get_stacktop)(ulong);
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/defs.h b/defs.h
index e9e8143..b63741c 100644
--- a/defs.h
+++ b/defs.h
@@ -1013,7 +1013,6 @@ struct machdep_table {
ulong (*processor_speed)(void);
int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
- int (*get_cpu_reg)(int, int, const char *, int, void *);
ulong (*get_task_pgd)(ulong);
void (*dump_irq)(int);
void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
@@ -1063,6 +1062,7 @@ struct machdep_table {
void (*get_irq_affinity)(int);
void (*show_interrupts)(int, ulong *);
int (*is_page_ptr)(ulong, physaddr_t *);
+ int (*get_cpu_reg)(int, int, const char *, int, void *);
};
/*
--
2.31.1
2 years, 11 months
Re: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility of struct symbol_table_data for extension modules
by lijiang
Hi, Hatayama
Thank you for the fix.
> Date: Wed, 8 Dec 2021 12:07:34 +0000
> From: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>
> To: "crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
> Subject: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility
> of struct symbol_table_data for extension modules
> Message-ID:
> <TYAPR01MB65079183F79B7F72B9486823956F9(a)TYAPR01MB6507.jpnprd01.prod.outlook.com>
>
> Content-Type: text/plain; charset="iso-2022-jp"
>
> Commit 2fab8fbc0c4f1c4cbe889de4cead5f7457a19f77 (symbols: Implement
> install and remove operations for mod_symname_hash) added new member
> variable mod_symname_hash in the middle of struct symbol_table_data:
>
> diff --git a/defs.h b/defs.h
> index cbd45e5..bbdca79 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2755,6 +2755,7 @@ struct symbol_table_data {
> double val_hash_searches;
> double val_hash_iterations;
> struct syment *symname_hash[SYMNAME_HASH];
> + struct syment *mod_symname_hash[SYMNAME_HASH];
> struct symbol_namespace kernel_namespace;
> struct syment *ext_module_symtable;
> struct syment *ext_module_symend;
>
> which breaks compatibility of struct symbol_table_data for extension
> modules. In general, new member variables must be added at the end of
> structures to maintain compatibility of data structures shared among
> extension modules.
Yes. Crash utility has documented the rules, see wiki:
https://github.com/crash-utility/crash/wiki. But, this seems to be
ignored again.
"If you add offset/size/array_length to each table, they have to be
appended to the end of the tables.This is to avoid breaking previously
compiled extension modules. Also, please add them to
dump_offset_table() (in arbitrarily order)."
>
>
> For example, as the result, crash trace command results in
> segmentation fault:
>
> crash> trace show
>
> Segmentation fault (core dumped)
>
> in context of save_proc_kallsyms():
>
> (gdb) bt
> #0 save_proc_kallsyms (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2234
> #1 __trace_cmd_data_output (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2538
> #2 0x00007f804e068dd5 in trace_cmd_data_output (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2588
> #3 ftrace_show (argc=919286707, argv=<optimized out>, argv=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:1825
> #4 0x00000000005010de in exec_command () at main.c:892
> #5 0x0000000000500ed0 in main_loop () at main.c:839
> #6 0x000000000085590d in captured_main (data=<optimized out>) at main.c:1284
> #7 gdb_main (args=<optimized out>) at main.c:1313
> #8 0x0000000000855965 in gdb_main_entry (argc=<optimized out>, argv=<optimized out>) at main.c:1338
> #9 0x00000000005b3db7 in gdb_main_loop (argc=2, argv=0x7ffc5afb3ed8) at gdb_interface.c:81
> #10 0x0000000000500b8d in main (argc=3, argv=0x7ffc5afb3ed8) at main.c:720
>
> when referring to sp->name:
>
> static int save_proc_kallsyms(int fd)
> {
> int i;
> struct syment *sp;
>
> for (sp = st->symtable; sp < st->symend; sp++)
> tmp_fprintf("%lx %c %s\n", sp->value, sp->type, sp->name);
>
> for (i = 0; i < st->mods_installed; i++) {
> struct load_module *lm = &st->load_modules[i];
>
> for (sp = lm->mod_symtable; sp <= lm->mod_symend; sp++) {
> => if (!strncmp(sp->name, "_MODULE_", strlen("_MODULE_")))
> continue;
>
> where sp->name contains an odd address:
>
> (gdb) p sp->name
> Cannot access memory at address 0x20047
>
> As seen above, save_proc_kallsyms() refers to st->load_modules and its
> position is behind the position where mod_symname_hash was added at:
>
> double val_hash_iterations;
> struct syment *symname_hash[SYMNAME_HASH];
> struct syment *mod_symname_hash[SYMNAME_HASH];
> struct symbol_namespace kernel_namespace;
> struct syment *ext_module_symtable;
> struct syment *ext_module_symend;
> long ext_module_symcnt;
> struct symbol_namespace ext_module_namespace;
> int mods_installed;
> struct load_module *current;
> struct load_module *load_modules;
>
> Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
> ---
> defs.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/defs.h b/defs.h
> index 7e2a16e..e9e8143 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2753,7 +2753,6 @@ struct symbol_table_data {
> double val_hash_searches;
> double val_hash_iterations;
> struct syment *symname_hash[SYMNAME_HASH];
> - struct syment *mod_symname_hash[SYMNAME_HASH];
> struct symbol_namespace kernel_namespace;
> struct syment *ext_module_symtable;
> struct syment *ext_module_symend;
> @@ -2780,6 +2779,7 @@ struct symbol_table_data {
> ulong kaiser_init_vmlinux;
> int kernel_symbol_type;
> ulong linux_banner_vmlinux;
> + struct syment *mod_symname_hash[SYMNAME_HASH];
> };
>
> /* flags for st */
> --
> 2.31.1
Ackec-by: Lianbo Jiang <lijiang(a)redhat.com>
2 years, 11 months
[PATCH] defs.h: fix breakage of compatibility of struct symbol_table_data for extension modules
by d.hatayama＠fujitsu.com
Commit 2fab8fbc0c4f1c4cbe889de4cead5f7457a19f77 (symbols: Implement
install and remove operations for mod_symname_hash) added new member
variable mod_symname_hash in the middle of struct symbol_table_data:
diff --git a/defs.h b/defs.h
index cbd45e5..bbdca79 100644
--- a/defs.h
+++ b/defs.h
@@ -2755,6 +2755,7 @@ struct symbol_table_data {
double val_hash_searches;
double val_hash_iterations;
struct syment *symname_hash[SYMNAME_HASH];
+ struct syment *mod_symname_hash[SYMNAME_HASH];
struct symbol_namespace kernel_namespace;
struct syment *ext_module_symtable;
struct syment *ext_module_symend;
which breaks compatibility of struct symbol_table_data for extension
modules. In general, new member variables must be added at the end of
structures to maintain compatibility of data structures shared among
extension modules.
For example, as the result, crash trace command results in
segmentation fault:
crash> trace show
Segmentation fault (core dumped)
in context of save_proc_kallsyms():
(gdb) bt
#0 save_proc_kallsyms (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2234
#1 __trace_cmd_data_output (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2538
#2 0x00007f804e068dd5 in trace_cmd_data_output (fd=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:2588
#3 ftrace_show (argc=919286707, argv=<optimized out>, argv=<optimized out>) at /usr/src/debug/crash-trace-command-3.0-4.el9.x86_64/trace.c:1825
#4 0x00000000005010de in exec_command () at main.c:892
#5 0x0000000000500ed0 in main_loop () at main.c:839
#6 0x000000000085590d in captured_main (data=<optimized out>) at main.c:1284
#7 gdb_main (args=<optimized out>) at main.c:1313
#8 0x0000000000855965 in gdb_main_entry (argc=<optimized out>, argv=<optimized out>) at main.c:1338
#9 0x00000000005b3db7 in gdb_main_loop (argc=2, argv=0x7ffc5afb3ed8) at gdb_interface.c:81
#10 0x0000000000500b8d in main (argc=3, argv=0x7ffc5afb3ed8) at main.c:720
when referring to sp->name:
static int save_proc_kallsyms(int fd)
{
int i;
struct syment *sp;
for (sp = st->symtable; sp < st->symend; sp++)
tmp_fprintf("%lx %c %s\n", sp->value, sp->type, sp->name);
for (i = 0; i < st->mods_installed; i++) {
struct load_module *lm = &st->load_modules[i];
for (sp = lm->mod_symtable; sp <= lm->mod_symend; sp++) {
=> if (!strncmp(sp->name, "_MODULE_", strlen("_MODULE_")))
continue;
where sp->name contains an odd address:
(gdb) p sp->name
Cannot access memory at address 0x20047
As seen above, save_proc_kallsyms() refers to st->load_modules and its
position is behind the position where mod_symname_hash was added at:
double val_hash_iterations;
struct syment *symname_hash[SYMNAME_HASH];
struct syment *mod_symname_hash[SYMNAME_HASH];
struct symbol_namespace kernel_namespace;
struct syment *ext_module_symtable;
struct syment *ext_module_symend;
long ext_module_symcnt;
struct symbol_namespace ext_module_namespace;
int mods_installed;
struct load_module *current;
struct load_module *load_modules;
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/defs.h b/defs.h
index 7e2a16e..e9e8143 100644
--- a/defs.h
+++ b/defs.h
@@ -2753,7 +2753,6 @@ struct symbol_table_data {
double val_hash_searches;
double val_hash_iterations;
struct syment *symname_hash[SYMNAME_HASH];
- struct syment *mod_symname_hash[SYMNAME_HASH];
struct symbol_namespace kernel_namespace;
struct syment *ext_module_symtable;
struct syment *ext_module_symend;
@@ -2780,6 +2779,7 @@ struct symbol_table_data {
ulong kaiser_init_vmlinux;
int kernel_symbol_type;
ulong linux_banner_vmlinux;
+ struct syment *mod_symname_hash[SYMNAME_HASH];
};
/* flags for st */
--
2.31.1
2 years, 11 months
Re: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility of struct machdep_table for extension modules
by lijiang
> Date: Thu, 9 Dec 2021 01:05:07 +0000
> From: "d.hatayama(a)fujitsu.com" <d.hatayama(a)fujitsu.com>
> To: "crash-utility(a)redhat.com" <crash-utility(a)redhat.com>
> Subject: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility
> of struct machdep_table for extension modules
> Message-ID:
> <TYAPR01MB6507D1D65B85CCE7567511AF95709(a)TYAPR01MB6507.jpnprd01.prod.outlook.com>
>
> Content-Type: text/plain; charset="iso-2022-jp"
>
> Commit 2f967fb5ebd737ce5eadba462df35935122e8865 (crash_taget:
> fetch_registers support) added new member get_cpu_reg in the middle of
> struct machdep_table as:
>
> @@ -1013,6 +1013,7 @@ struct machdep_table {
> ulong (*processor_speed)(void);
> int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> + int (*get_cpu_reg)(int, int, const char *, int, void *);
> ulong (*get_task_pgd)(ulong);
> void (*dump_irq)(int);
> void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
>
> which breaks compatibility of struct machdep_table for extension
> modules. In general, new member variables must be added at the end of
> structures to maintain compatibility of data structures shared among
> extension modules.
>
This is similar with another issue, see its reply:
https://listman.redhat.com/archives/crash-utility/2021-December/msg00008....
> For example, as the result, crash gcore command results in unexpected
> behavior:
>
> crash> gcore -v 7
> GETBUF(168 -> 0)
> <readmem: ffff8b267dd28020, KVADDR, "task_struct.stack", 8, (FOE), 7fffaef8afa8>
> <read_kdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8>
> read_netdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8 offset: 767ec020
> <readmem: ffffa3f740abff58, KVADDR, "genregs_get: pt_regs", 168, (FOE), e71f80>
> gcore: invalid kernel virtual address: ffffa3f740abff58 type: "genregs_get: pt_regs"
> vma hit rate: 0% (0 of 274308)
>
> crash gcore uses machdep->get_stacktop() to retrieve registers saved
> at the bottom of a given user-space task's kernel stack:
>
> static int genregs_get(struct task_context *target,
> const struct user_regset *regset,
> unsigned int size, void *buf)
> {
> ...snip...
> pt_regs_buf = GETBUF(SIZE(pt_regs));
>
> readmem(machdep->get_stacktop(target->task) - SIZE(pt_regs), KVADDR,
> pt_regs_buf, SIZE(pt_regs), "genregs_get: pt_regs",
> gcore_verbose_error_handle());
>
> As seen above, the position of get_stacktop in struct machdep_table is
> behind the position where get_cpu_reg was added:
>
> int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong);
> void (*dump_irq)(int);
> void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> ulong (*get_stackbase)(ulong);
> ulong (*get_stacktop)(ulong);
>
> Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
> ---
> defs.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/defs.h b/defs.h
> index e9e8143..b63741c 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1013,7 +1013,6 @@ struct machdep_table {
> ulong (*processor_speed)(void);
> int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
> int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> - int (*get_cpu_reg)(int, int, const char *, int, void *);
> ulong (*get_task_pgd)(ulong);
> void (*dump_irq)(int);
> void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> @@ -1063,6 +1062,7 @@ struct machdep_table {
> void (*get_irq_affinity)(int);
> void (*show_interrupts)(int, ulong *);
> int (*is_page_ptr)(ulong, physaddr_t *);
> + int (*get_cpu_reg)(int, int, const char *, int, void *);
> };
>
Acked-by: Lianbo Jiang <lijiang(a)redhat.com>
> /*
> --
> 2.31.1
2 years, 11 months
