Re: [PATCH] irqdomain: provide useful debugging information for irq domain

From: Marc Zyngier
Date: Wed Jan 17 2018 - 03:37:12 EST


Hi Yang,

On 17/01/18 04:37, Yang Shunyong wrote:
> With recent hashed kernel pointers change, output with %p will
> output hashed address. This patch changes %p to %px in irq domain
> debug information. As unprivileged user has no permission to mount
> debugfs or set printk level to KERN_DEBUG to access the information,
> changes in this patch are appropriate.
>
> Signed-off-by: Yang Shunyong <shunyong.yang@xxxxxxxxxxxxxxxx>
> ---
> kernel/irq/irqdomain.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 62068ad..d40fbed 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -75,7 +75,7 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id,
> n = kasprintf(GFP_KERNEL, "%s-%d", name, id);
> break;
> default:
> - n = kasprintf(GFP_KERNEL, "irqchip@%p", data);
> + n = kasprintf(GFP_KERNEL, "irqchip@%px", data);
> break;
> }
>
> @@ -450,7 +450,7 @@ EXPORT_SYMBOL_GPL(irq_domain_check_msi_remap);
> */
> void irq_set_default_host(struct irq_domain *domain)
> {
> - pr_debug("Default domain set to @0x%p\n", domain);
> + pr_debug("Default domain set to @0x%px\n", domain);
>
> irq_default_domain = domain;
> }
> @@ -635,7 +635,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> struct device_node *of_node;
> int virq;
>
> - pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
> + pr_debug("%s(0x%px, 0x%lx)\n", __func__, domain, hwirq);
>
> /* Look for default domain if nececssary */
> if (domain == NULL)
> @@ -644,7 +644,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> WARN(1, "%s(, %lx) called with NULL domain\n", __func__, hwirq);
> return 0;
> }
> - pr_debug("-> using domain @%p\n", domain);
> + pr_debug("-> using domain @%px\n", domain);
>
> of_node = irq_domain_get_of_node(domain);
>
> @@ -921,7 +921,7 @@ static void virq_debug_show_one(struct seq_file *m, struct irq_desc *desc)
> chip = irq_data_get_irq_chip(data);
> seq_printf(m, "%-15s ", (chip && chip->name) ? chip->name : "none");
>
> - seq_printf(m, "0x%p ", irq_data_get_irq_chip_data(data));
> + seq_printf(m, "0x%px ", irq_data_get_irq_chip_data(data));
>
> seq_printf(m, " %c ", (desc->action && desc->action->handler) ? '*' : ' ');
> direct = (irq == hwirq) && (irq < domain->revmap_direct_max_irq);
>

In all honesty, I'd be more inclined to remove this debug feature
altogether, as CONFIG_GENERIC_IRQ_DEBUGFS is more complete and more
useful. Is there any feature missing from that infrastructure that
prevents you from using it instead?

If the answer is "none", then I'll post a patch removing it.

Thanks,

M.
--
Jazz is not dead. It just smells funny...