linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Kieran Bingham <kieran.bingham@linaro.org>, linux-kernel@vger.kernel.org
Cc: lee.jones@linaro.org, peter.griffin@linaro.org, maxime.coquelin@st.com
Subject: Re: [PATCHv3 00/13] scripts/gdb: Linux awareness debug commands
Date: Mon, 14 Mar 2016 16:09:09 +0100	[thread overview]
Message-ID: <56E6D415.6050408@siemens.com> (raw)
In-Reply-To: <56E6CD43.3010400@linaro.org>

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.

> 
>>
>>> 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.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

  reply	other threads:[~2016-03-14 15:09 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-03 11:40 [PATCHv3 00/13] scripts/gdb: Linux awareness debug commands Kieran Bingham
2016-03-03 11:40 ` [PATCHv3 01/13] scripts/gdb: Provide linux constants Kieran Bingham
2016-03-03 11:40 ` [PATCHv3 02/13] scripts/gdb: Provide kernel list item generators Kieran Bingham
2016-03-08  3:47   ` Jeff Mahoney
2016-03-08  7:55     ` Kieran Bingham
2016-03-03 11:40 ` [PATCHv3 03/13] scripts/gdb: Convert modules usage to lists functions Kieran Bingham
2016-03-03 11:40 ` [PATCHv3 04/13] scripts/gdb: Provide exception catching parser Kieran Bingham
2016-03-03 11:40 ` [PATCHv3 05/13] scripts/gdb: Support !CONFIG_MODULES gracefully Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 06/13] scripts/gdb: Provide a dentry_name VFS path helper Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 07/13] scripts/gdb: Add io resource readers Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 08/13] scripts/gdb: Add mount point list command Kieran Bingham
2016-03-13 16:34   ` Jan Kiszka
2016-03-14 14:39     ` Kieran Bingham
2016-03-14 15:05       ` Jan Kiszka
2016-03-15 10:46         ` Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 09/13] scripts/gdb: Add meminfo command Kieran Bingham
2016-03-13 16:34   ` Jan Kiszka
2016-03-13 18:16     ` Kieran Bingham
2016-03-13 19:08       ` Jan Kiszka
2016-03-14 12:13         ` Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 10/13] scripts/gdb: Add cpu iterators Kieran Bingham
2016-03-13 16:33   ` Jan Kiszka
2016-03-13 18:39     ` Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 11/13] scripts/gdb: Add a Radix Tree Parser Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 12/13] scripts/gdb: Add interrupts command Kieran Bingham
2016-03-03 11:41 ` [PATCHv3 13/13] scripts/gdb: Add lx_thread_info_by_pid helper Kieran Bingham
2016-03-13 16:35 ` [PATCHv3 00/13] scripts/gdb: Linux awareness debug commands Jan Kiszka
2016-03-14 14:40   ` Kieran Bingham
2016-03-14 15:09     ` Jan Kiszka [this message]
2016-03-14 17:18       ` Kieran Bingham
2016-03-14 17:31         ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56E6D415.6050408@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=kieran.bingham@linaro.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.coquelin@st.com \
    --cc=peter.griffin@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).