Subject: Re: [PATCH] irqdomain: provide useful debugging information for irq
 domain
To: Yang Shunyong <shunyong.yang@hxt-semitech.com>
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org
References: <1516163877-32396-1-git-send-email-shunyong.yang@hxt-semitech.com>
From: Marc Zyngier <marc.zyngier@arm.com>
Organization: ARM Ltd
Message-ID: <210f90d0-1588-2185-4845-d5d143183621@arm.com>
Date: Wed, 17 Jan 2018 08:37:05 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.2
MIME-Version: 1.0
In-Reply-To: <1516163877-32396-1-git-send-email-shunyong.yang@hxt-semitech.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

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@hxt-semitech.com>
> ---
>  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...