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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx <mailto:k-hagio-ab@xxxxxxx>> 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@xxxxxxx <mailto:k-hagio-ab@xxxxxxx>>
>>     ---
>>      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.

an example without "break;"

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

Thanks,
Kazu

>>
>> Other changes look good to me.
>>
>> Thanks.
>> Lianbo
>>
>>     +                       if (STREQ(buf, "memory")) {
>>     +                               /* entry is subsys_private.subsys.kobj. See bus_to_subsys(). */
>>     +                               private = ld->list_ptr[i] - OFFSET(kset_kobj)
>>     +                                               - OFFSET(subsys_private_subsys);
>>     +                               break;
>>     +                       }
>>     +               }
>>     +               FREEBUF(ld->list_ptr);
>>     +       } else {
>>     +               ulong memory_subsys = symbol_value("memory_subsys");
>>     +               readmem(memory_subsys + OFFSET(bus_type_p), KVADDR, &private,
>>     +                       sizeof(void *), "memory_subsys.private", FAULT_ON_ERROR);
>>     +       }
>>     +
>>     +       if (!private)
>>     +               error(FATAL, "cannot determine subsys_private for memory.\n");
>>     +
>>             klist = private + OFFSET(subsys_private_klist_devices) +
>>                                             OFFSET(klist_k_list);
>>             BZERO(ld, sizeof(struct list_data));
>>     @@ -17875,7 +17928,6 @@ dump_memory_blocks(int initialize)
>>             ulong memory_block, device;
>>             ulong *klistbuf;
>>             int klistcnt, i;
>>     -       struct list_data list_data;
>>             char mb_hdr[BUFSIZE];
>>             char paddr_hdr[BUFSIZE];
>>             char buf1[BUFSIZE];
>>     @@ -17892,7 +17944,7 @@ dump_memory_blocks(int initialize)
>>             if (initialize)
>>                     return;
>>
>>     -       init_memory_block(&list_data, &klistcnt, &klistbuf);
>>     +       init_memory_block(&klistcnt, &klistbuf);
>>
>>             if ((symbol_exists("memory_block_size_probed")) ||
>>                 (MEMBER_EXISTS("memory_block", "end_section_nr")))
>>     diff --git a/symbols.c b/symbols.c
>>     index 28846d06273c..f0721023816d 100644
>>     --- a/symbols.c
>>     +++ b/symbols.c
>>     @@ -10404,6 +10404,7 @@ dump_offset_table(char *spec, ulong makestruct)
>>                     OFFSET(kobject_entry));
>>             fprintf(fp, "                     kset_list: %ld\n",
>>                     OFFSET(kset_list));
>>     +       fprintf(fp, "                     kset_kobj: %ld\n", OFFSET(kset_kobj));
>>             fprintf(fp, "            request_list_count: %ld\n",
>>                     OFFSET(request_list_count));
>>             fprintf(fp, "             request_cmd_flags: %ld\n",
>>     @@ -10441,6 +10442,7 @@ dump_offset_table(char *spec, ulong makestruct)
>>             fprintf(fp, "               blk_mq_tags_rqs: %ld\n",
>>                     OFFSET(blk_mq_tags_rqs));
>>
>>     +       fprintf(fp, "         subsys_private_subsys: %ld\n", OFFSET(subsys_private_subsys));
>>             fprintf(fp, "  subsys_private_klist_devices: %ld\n",
>>                     OFFSET(subsys_private_klist_devices));
>>             fprintf(fp, "                subsystem_kset: %ld\n",
>>     -- 
>>     2.31.1
>>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux