[Crash-utility] [PATCH v3] add option -s and -S for subcommand irq

[Per Fransson]

Hi,

I can confirm that hooking in generic_show_interrupts() and
generic_get_irq_affinity() results in meaningful output for ARM as
well, using PATCH v3.

I've got another suggestion however. How about mimicking the kernel
code a little closer?

    fs/proc/interrupts.c

will keep going for one more iteration, i.e. while <=nr_irqs and

    kernel/irq/proc.c:show_interrupts()

will call arch_show_interrupts() when called with ==nr_irqs.

In Crash we could do this in cmd_irq

    for (i = 0; i <= nr_irqs; i++)
        machdep->show_interrupts(i, cpus);

and this at the start of generic_show_interrupts():

    if (irq >= machdep->nr_irqs) {
        return;
    }

making it possible to have machdep->show_interrupts point to a wrapper
function like:

    static void arm_show_interrupts(int irq, ulong *cpus)
    {
            if (irq == machdep->nr_irqs) {
                    // Show FIQ
                    // Show IPI
            }
            else {
                    generic_show_interrupts(irq, cpus);
            }
    }

Just an idea.

Regards,
Per

On Wed, Jan 18, 2012 at 9:11 PM, Dave Anderson <anderson at redhat.com> wrote:
>
>
> ----- Original Message -----
>> Hello, Dave
>>
>> Re-posting two patches as attachments. Very sorry for incorrectly
>> changing the behaviour of the original "irq" command in the last
>> patches.
>>
>> Thanks,
>> Zhang Yanfei
>
> I should first note that I would have preferred that the patch were
> written to address the other actively-supported architectures, given
> that the irq_data and irq_desc structures are not x86/x86_64-specific.
> However, I understand that you may not have access to the other
> architectures.  That being the case, I invite the s390x, ia64, arm,
> ppc32 and ppc64 maintainers to look at this patch, and if you are
> interested, please apply it for those architectures as well.
>
> Anyway, I made a number of changes to the patch as written.
>
> (1) I changed the "irq" option letters to something more meaningful:
>
>    (a) For IRQ affinity, I changed it from "irq -s" to "irq -a".
>    (b) For IRQ stats, I changed it from "irq -S" to "irq -s".
>
> (2) In generic_show_interrupts(), I changed the output so that it
>    does not append additional unnecessary spaces to the end of
>    the output line, i.e., from this:
>
>        fprintf(fp, " %-20s ", name_buf);
>        fprintf(fp, "\n");
>
>    to simply this:
>
>        fprintf(fp, " %s\n", name_buf);
>
> (3) In cmd_irq(), I changed the two new command_not_supported() calls
>    to be option_not_supported(c).
>
> (4) The "irq -s" option fails on earlier kernels, for example, with
>    the RHEL4 kernel:
>
>      crash> irq -s
>      IRQ NAME                 AFFINITY
>
>      irq: invalid structure member offset: irq_desc_t_affinity
>           FILE: kernel.c  LINE: 5543  FUNCTION: generic_get_irq_affinity()
>
>      [/home/anderson/crash-6.0.2/crash] error trace: 459c05 => 4bf432 => 4c1d31 => 4fb70e
>
>        4fb70e: OFFSET_verify+222
>        4c1d31: generic_get_irq_affinity+1153
>        4bf432: cmd_irq+530
>        459c05: exec_command+821
>
>      irq: invalid structure member offset: irq_desc_t_affinity
>
>    The "irq -s" option would also fail on recent kernels if they are
>    not configured with CONFIG_SMP, because irq_data.affinity
>    member would not exist:
>
>      struct irq_data {
>              unsigned int            irq;
>              unsigned long           hwirq;
>              unsigned int            node;
>              unsigned int            state_use_accessors;
>              struct irq_chip         *chip;
>              struct irq_domain       *domain;
>              void                    *handler_data;
>              void                    *chip_data;
>              struct msi_desc         *msi_desc;
>      #ifdef CONFIG_SMP
>              cpumask_var_t           affinity;
>      #endif
>      };
>
>    To handle those two possibilities, I added this to cmd_irq()
>    for the "-a" option:
>
>              if (VALID_STRUCT(irq_data)) {
>                      if (INVALID_MEMBER(irq_data_affinity))
>                              option_not_supported(c);
>              } else if (INVALID_MEMBER(irq_desc_t_affinity))
>                      option_not_supported(c);
>
>
> (5) In x86.c, it makes no sense to set machdep->get_irq_affinity
>    and machdep->show_interrupts in x86_init_hyper() given that the
>    "irq" command is not supported for the Xen hypervisor analysis.
>    (See the xen_hyper_command_table[] in xen_hyper_global_data.c)
>
> (6) In defs.h, I moved:
>
>    (a) get_irq_affinity and show_interrupts to the end of the
>        machdep_table structure
>    (b) irq_desc_t_irq_data, irq_desc_t_kstat_irqs, irq_desc_t_affinity,
>        irq_data_chip, irq_data_affinity and kernel_stat_irqs to the
>        end of the offset_table structure.
>    (c) kernel_stat to the end of the size_table structure.
>
>    This would allow an extension module built with an earlier version
>    of the crash utility to still work without recompiling due to
>    structure member offset changes.
>
> (7) I updated dump_offset_table() to show the new members of the
>    offset_table and the size_table for the "help -o" command.
>
> (8) And lastly, I reworked the "help irq" page.
>
> I will continue testing it on various kernels and architectures,
> and if all goes well, I will queue it for crash-6.0.3.
>
> Dave
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
-------------- next part --------------
A non-text attachment was scrubbed...
Name: irq_stat_affinity.patch
Type: text/x-patch
Size: 1101 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20120119/7b8ddbdd/attachment.bin>