Re: [Crash-utility] [PATCH] Fix "kmem -n" option to display memory blocks on Linux 6.3-rc1 and later

On 2023/03/10 10:48, lijiang wrote: >> Thank you for the fix, Kazu. >> On Thu, Mar 9, 2023 at 9:44 AM HAGIO KAZUHITO(萩尾 一仁) < k-hagio-ab(a)nec.com <mailto:k-hagio-ab@nec.com>> wrote: >> >> Kernel commit d2bf38c088e0 ("driver core: remove private pointer from >> struct bus_type") removed the bus_type.p member, and the "kmem -n" >> option fails with the following error before displaying memory block >> information on Linux 6.3-rc1 and later kernels. >> >> kmem: invalid structure member offset: bus_type_p >> FILE: memory.c LINE: 17852 FUNCTION: init_memory_block() >> >> Search bus_kset.list instead for subsys_private of memory subsys. >> >> Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com <mailto: k-hagio-ab(a)nec.com&gt;&gt; >> --- >> defs.h | 2 ++ >> memory.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++------ >> symbols.c | 2 ++ >> 3 files changed, 62 insertions(+), 6 deletions(-) >> >> diff --git a/defs.h b/defs.h >> index 1f2cf6e0ce01..12ad6aaa0998 100644 >> --- a/defs.h >> +++ b/defs.h >> @@ -2214,6 +2214,8 @@ struct offset_table { /* stash of commonly-used offsets */ >> long inet6_ifaddr_if_list; >> long inet6_ifaddr_if_next; >> long in6_addr_in6_u; >> + long kset_kobj; >> + long subsys_private_subsys; >> }; >> >> struct size_table { /* stash of commonly-used sizes */ >> diff --git a/memory.c b/memory.c >> index c4a6ecd18004..292b15a0dbab 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -17822,6 +17822,13 @@ static void >> init_memory_block_offset(void) >> { >> MEMBER_OFFSET_INIT(bus_type_p, "bus_type", "p"); >> + if (INVALID_MEMBER(bus_type_p)) { >> + MEMBER_OFFSET_INIT(kset_list, "kset", "list"); >> + MEMBER_OFFSET_INIT(kset_kobj, "kset", "kobj"); >> + MEMBER_OFFSET_INIT(kobject_name, "kobject", "name"); >> + MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry"); >> + MEMBER_OFFSET_INIT(subsys_private_subsys, "subsys_private", "subsys"); >> + } >> MEMBER_OFFSET_INIT(subsys_private_klist_devices, >> "subsys_private", "klist_devices"); >> MEMBER_OFFSET_INIT(klist_k_list, "klist", "k_list"); >> @@ -17842,15 +17849,61 @@ init_memory_block_offset(void) >> } >> >> static void >> -init_memory_block(struct list_data *ld, int *klistcnt, ulong **klistbuf) >> +init_memory_block(int *klistcnt, ulong **klistbuf) >> { >> - ulong memory_subsys = symbol_value("memory_subsys"); >> ulong private, klist, start; >> + struct list_data list_data, *ld; >> + >> + ld = &list_data; >> + private = 0; >> >> init_memory_block_offset(); >> >> - readmem(memory_subsys + OFFSET(bus_type_p), KVADDR, &private, >> - sizeof(void *), "memory_subsys.private", FAULT_ON_ERROR); >> + /* >> + * v6.3-rc1 >> + * d2bf38c088e0 driver core: remove private pointer from struct bus_type >> + */ >> + if (INVALID_MEMBER(bus_type_p)) { >> + int i, cnt; >> + char buf[8]; /* need 8 at least to exclude "memory_tiering" */ >> + ulong bus_kset, list, name; >> + >> + BZERO(ld, sizeof(struct list_data)); >> + >> + get_symbol_data("bus_kset", sizeof(ulong), &bus_kset); >> + readmem(bus_kset + OFFSET(kset_list), KVADDR, &list, >> + sizeof(ulong), "bus_kset.list", FAULT_ON_ERROR); >> + >> + ld->flags |= LIST_ALLOCATE; >> + ld->start = list; >> + ld->end = bus_kset + OFFSET(kset_list); >> + ld->list_head_offset = OFFSET(kobject_entry); >> + >> + cnt = do_list(ld); >> + for (i = 0; i < cnt; i++) { >> + readmem(ld->list_ptr[i] + OFFSET(kobject_name), KVADDR, &name, >> + sizeof(ulong), "kobject.name < http://kobject.name>";, FAULT_ON_ERROR); >> + read_string(name, buf, sizeof(buf)); >> + buf[7] = '\0'; >> >> >> The full name will be truncated, and the STREQ(buf, "memory") will include two kinds of the "memory" and "memory_tiering", but the above code comment says to exclude "memory_tiering", looks weird to me. Do I misunderstand this? As the buf array has 8 bytes and the last is set to \0, so "memory" -> memory\0\0 "memory_tiering" -> memory_\0 So we can distinguish these with STREQ(buf, "memory").

> >> + if (CRASHDEBUG(1)) >> + fprintf(fp, "kobject: %lx name: %s\n", ld->list_ptr[i], buf); >> >> >> BTW: for the debug information, it might be good to output the full name of the memory subsys. Yes, I was not sure what is the max length of the name, so chose the minimum length in order to avoid read error due to out of bounds. But ok, looking at the usage of read_string() in crash, most of them have BUF_SIZE-1. Maybe we don't need to care about it so much. I will change it to 32 in v2.

kobject: ffff8e1d83d83a18 name: platform kobject: ffff8e1d83d80e18 name: cpu kobject: ffff8e1d83d83c18 name: memory kobject: ffff8e1d83d81c18 name: node kobject: ffff8e1d83d80c18 name: container kobject: ffff8e1d83d80a18 name: workqueue kobject: ffff8e1d83d82e18 name: gpio kobject: ffff8e1d83d83418 name: virtio kobject: ffff8e19cb77da18 name: pci kobject: ffff8e19cb77e018 name: pci_express kobject: ffff8e19cb77ce18 name: spi kobject: ffff8e19cb77f818 name: i2c kobject: ffff8e19cb7c7a18 name: memory_tiering kobject: ffff8e1d8466ee18 name: acpi kobject: ffff8e1605d4e418 name: pnp kobject: ffff8e1605d4d018 name: xen kobject: ffff8e1605d4c418 name: dax kobject: ffff8e1605d4fc18 name: cxl kobject: ffff8e1605d4fa18 name: scsi kobject: ffff8e1605d4e218 name: mdio_bus kobject: ffff8e1605d4fe18 name: usb kobject: ffff8e1605d4c018 name: typec kobject: ffff8e1605d4f618 name: serio kobject: ffff8e1605e43018 name: edac kobject: ffff8e1605e41218 name: nvmem kobject: ffff8e16047d8a18 name: thunderbolt kobject: ffff8e160451e018 name: clocksource kobject: ffff8e160451da18 name: clockevents kobject: ffff8e160451f418 name: event_source kobject: ffff8e1d83e76018 name: usb-serial kobject: ffff8e1d86135418 name: hid kobject: ffff8e1611eea418 name: machinecheck kobject: ffff8e1d8d108618 name: wmi kobject: ffff8e160c187e18 name: mei