Re: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility of struct machdep_table for extension modules

(sorry, I mistook the email to reply.. this is the correct one.) -----Original Message----- > 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&gt; Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com&gt; Lianbo, this is also needed for RHEL9 crash before GA.

Thanks, Kazu > --- > 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 > > > > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://listman.redhat.com/mailman/listinfo/crash-utility