Re: [PATCHv3 00/13] scripts/gdb: Linux awareness debug commands

[Kieran Bingham <kieran.bingham@linaro.org>]

On 14/03/16 15:09, Jan Kiszka wrote:
> On 2016-03-14 15:40, Kieran Bingham wrote:
>> On 13/03/16 16:35, Jan Kiszka wrote:
>>> On 2016-03-03 12:40, Kieran Bingham wrote:
>>>> Hi Jan,
>>>>
>>>> V3 of the patchset respun. Now finally adding the lx-interrupts command
>>>> after I resolved my issues with the Radix Tree parsing.
>>>>
>>>> This command only provides the interrupts that are available generically,
>>>> and it seems that the /proc/interrupts function calls into arch specific
>>>> layers to add extra information about arch specific interrupts.
>>>>
>>>> I'm not sure what to do about this yet - The values returned appear to be
>>>> accurate - but it's just a subset of the information returned by proc.
>>>>
>>>
>>> I didn't test this (due to the breakage in patch 10): Can you give
>>> examples of what is missing, e.g. on ARM or x86?
>> On ARM this is :
>>
>> (gdb) lx-interrupts
>>             CPU0       CPU1
>>   18:     587828     150187
>>   36:      66574          0
>>   41:          8          0
>>   42:        106          0
>>   43:        100          0
>> (gdb) c
>>
>>
>> vs
>>
>> root@ArmACookieMonster:~# cat /proc/interrupts
>>            CPU0       CPU1
>>  18:     588057     150274     GIC-0  27 Edge      arch_timer
>>  20:          0          0     GIC-0  34 Level     timer
>>  36:      66599          0     GIC-0  47 Level     eth0
>>  39:          0          0     GIC-0  41 Level     mmci-pl18x (cmd)
>>  40:          0          0     GIC-0  42 Level     mmci-pl18x (pio)
>>  41:          8          0     GIC-0  44 Level     kmi-pl050
>>  42:        106          0     GIC-0  45 Level     kmi-pl050
>>  43:        112          0     GIC-0  37 Level     uart-pl011
>>  49:          0          0     GIC-0  36 Level     rtc-pl031
>> IPI0:          0          1  CPU wakeup interrupts
>> IPI1:          0          0  Timer broadcast interrupts
>> IPI2:      15867     281362  Rescheduling interrupts
>> IPI3:          0          6  Function call interrupts
>> IPI4:          0          0  CPU stop interrupts
>> IPI5:          0          0  IRQ work interrupts
>> IPI6:          0          0  completion interrupts
>> Err:          0
>>
>> So quite a substantial subset :(
> 
> Indeed. Given this delta, I'm reluctant to include that command at this
> point.

Ok, understandable...

I think the radix-tree lookup could be useful for people though.
This is used across filesystems, and other places.

Perhaps I should wrap this up into a gdb.Function rather than drop it?


>>>> lx_thread_info_by_pid has been useful to me while looking at thread
>>>> awareness, so I've included it into this patch set now. It makes finding
>>>> internal thread information much more convenient.
>>>>
>>>> dentry_name has been moved to the utils module, as I am already using it
>>>> in another command, so it's just not appropriate to be in proc.py
>>>>
>>>> The cpu_list mask iterators make calling for cpu in each_online_cpu() read
>>>> nicely, and I've left the print_cpus() function in for now as a hidden
>>>> helper. It can be used by calling:
>>>>   python linux.cpus.print_cpus()
>>>> to check these generators, which I thought was quite nice - but I didn't
>>>> know if it warranted a full command class for this.
>>>>
>>>> For convenience, this patch set submission can be found at
>>>>   http://git.linaro.org/people/kieran.bingham/linux.git gdb-scripts-2016-03-03-lkml-submission
>>>>
>>>> Patchset Changelog:
>>>> v3:
>>>>  - Radix Tree parser introduced
>>>>  - cpu_list mask iterators added
>>>>  - lx-interrupts command implemented
>>>>  - dentry_name function moved to utils
>>>>  - lx-meminfo command PEP8 warnings fixed
>>>>  - lx_thread_info_by_pid introduced
>>>>
>>>> v2:
>>>>  - Reworked iterators with improved versions from Jeff Mahoney
>>>>  - Fixed !CONFIG_MODULES and !CONFIG_MMU support
>>>>  - Improvements on lx-meminfo
>>>>  - constants.py generated by Kbuild
>>>>  - IS_BUILTIN facility used to provide LX_CONFIG values
>>>>
>>>> v1:
>>>>  - Introduced lx-iomem, lx-ioports, lx-mounts, lx-meminfo
>>>>
>>>> Kieran Bingham (13):
>>>>   scripts/gdb: Provide linux constants
>>>>   scripts/gdb: Provide kernel list item generators
>>>>   scripts/gdb: Convert modules usage to lists functions
>>>>   scripts/gdb: Provide exception catching parser
>>>>   scripts/gdb: Support !CONFIG_MODULES gracefully
>>>>   scripts/gdb: Provide a dentry_name VFS path helper
>>>>   scripts/gdb: Add io resource readers
>>>>   scripts/gdb: Add mount point list command
>>>>   scripts/gdb: Add meminfo command
>>>>   scripts/gdb: Add cpu iterators
>>>>   scripts/gdb: Add a Radix Tree Parser
>>>>   scripts/gdb: Add interrupts command
>>>>   scripts/gdb: Add lx_thread_info_by_pid helper
>>>>
>>>>  Kbuild                            |  10 +
>>>>  scripts/gdb/linux/Makefile        |  12 +-
>>>>  scripts/gdb/linux/constants.py.in |  93 ++++++++
>>>>  scripts/gdb/linux/cpus.py         |  21 ++
>>>>  scripts/gdb/linux/lists.py        |  20 ++
>>>>  scripts/gdb/linux/modules.py      |  22 +-
>>>>  scripts/gdb/linux/proc.py         | 449 ++++++++++++++++++++++++++++++++++++++
>>>>  scripts/gdb/linux/radixtree.py    |  74 +++++++
>>>>  scripts/gdb/linux/tasks.py        |  19 ++
>>>>  scripts/gdb/linux/utils.py        |  15 ++
>>>>  scripts/gdb/vmlinux-gdb.py        |   2 +
>>>>  11 files changed, 724 insertions(+), 13 deletions(-)
>>>>  create mode 100644 scripts/gdb/linux/constants.py.in
>>>>  create mode 100644 scripts/gdb/linux/radixtree.py
>>>>
>>>
>>> Besides the required rebase and, thus, the missing adjustment of the cpu
>>> masks, I only have minor remarks. Maybe I will have more once I can play
>>> with lx-interrupts ;).
>>
>> Ok - well it's fixed up locally, it's just a matter of prefixing the
>> strings with two underscores. I should be able to get the updated
>> patches out soon I hope.
>>
>>> However, I have a growing concern - I think we already discussed this
>>> offline: Without automated tests, all these helpers may quickly fall
>>> apart as the kernel changes. This should not block these patches, but
>>> maybe you can think about some testing approaches before we have dozens
>>> of helpers which can only be validated manually.
>>
>> Yes, indeed - and the breakage already seen between v4.4 and v4.5 only
>> solidifies the need!
>>
>> I was able to chat to a few of the Linaro LAVA guys on this topic while
>> I was at Connect, and they already have virtual kernels being boot
>> tested. This should provide a good infrastructure to run some automated
>> tests on.
> 
> Sounds good!
> 
>>
>> So we have the infrastructure - we just need to get the tests written,
>> and included.
> 
> Yeah, "minor" detail.

Isn't it ever :D

> 
> Jan
>