[PATCH] irqchip/mbigen: stop printing kernel addresses

[PATCH] irqchip/mbigen: stop printing kernel addresses

[Kefeng Wang]

After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
it will print "____ptrval____" instead of actual addresses when mbigen
create domain fails,

  Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
  Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12

Instead of changing the print to "%px", and leaking kernel addresses,
just remove the print completely.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 98b6e1d4b1a6..d0cf596c801b 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
 		err = -EINVAL;
 
 	if (err) {
-		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
-			mgn_chip->base);
+		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain");
 		return err;
 	}
 
-- 
2.20.1

Re: [PATCH] irqchip/mbigen: stop printing kernel addresses

[Marc Zyngier]

Hi Kefeng,

On Tue, 18 Jun 2019 04:22:02 +0100,
Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> it will print "____ptrval____" instead of actual addresses when mbigen
> create domain fails,
> 
>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
> 
> Instead of changing the print to "%px", and leaking kernel addresses,
> just remove the print completely.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/irqchip/irq-mbigen.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 98b6e1d4b1a6..d0cf596c801b 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  		err = -EINVAL;
>  
>  	if (err) {
> -		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
> -			mgn_chip->base);
> +		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain");

The alternative would be to print res as a resource, which would still
help identifying the offending device by printing its physical
layout, and still not reveal much.

Just let me know what you prefer.

Thanks,

	M.

>  		return err;
>  	}
>  
> -- 
> 2.20.1
> 

-- 
Jazz is not dead, it just smells funny.

Re: [PATCH] irqchip/mbigen: stop printing kernel addresses

[Hanjun Guo]

On 2019/6/18 11:22, Kefeng Wang wrote:
> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> it will print "____ptrval____" instead of actual addresses when mbigen
> create domain fails,
> 
>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
> 
> Instead of changing the print to "%px", and leaking kernel addresses,
> just remove the print completely.

This is a little bit misleading, as the "base" was used for
identify which mbigen failed, so saying 'remove completely'
will make people think that we will miss some debug information.

In fact, we have HISI0152:00 stands for mbigen ACPI HID and
its UID, so we can identify the failing probed mbigen even
we remove the printing "mgn_chip->base". It's better to add
this clarify in the commit message as well.

Thanks
Hanjun

Re: [PATCH] irqchip/mbigen: stop printing kernel addresses

[Kefeng Wang]

On 2019/6/18 15:48, Marc Zyngier wrote:
> Hi Kefeng,
> 
> On Tue, 18 Jun 2019 04:22:02 +0100,
> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
>> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
>> it will print "____ptrval____" instead of actual addresses when mbigen
>> create domain fails,
>>
>>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
>>
>> Instead of changing the print to "%px", and leaking kernel addresses,
>> just remove the print completely.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>  drivers/irqchip/irq-mbigen.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>> index 98b6e1d4b1a6..d0cf596c801b 100644
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>  		err = -EINVAL;
>>  
>>  	if (err) {
>> -		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
>> -			mgn_chip->base);
>> +		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain");
> 
> The alternative would be to print res as a resource, which would still
> help identifying the offending device by printing its physical
> layout, and still not reveal much.

It's better to print res to show the physical layout, and add missing "\n",
will resend v2.

Thanks.

> 
> Just let me know what you prefer.
> 
> Thanks,
> 
> 	M.
> 
>>  		return err;
>>  	}
>>  
>> -- 
>> 2.20.1
>>
> 

Re: [PATCH] irqchip/mbigen: stop printing kernel addresses

[Marc Zyngier]

On 18/06/2019 09:35, Kefeng Wang wrote:
> 
> 
> On 2019/6/18 15:48, Marc Zyngier wrote:
>> Hi Kefeng,
>>
>> On Tue, 18 Jun 2019 04:22:02 +0100,
>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>>
>>> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
>>> it will print "____ptrval____" instead of actual addresses when mbigen
>>> create domain fails,
>>>
>>>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>>>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
>>>
>>> Instead of changing the print to "%px", and leaking kernel addresses,
>>> just remove the print completely.
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>  drivers/irqchip/irq-mbigen.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>>> index 98b6e1d4b1a6..d0cf596c801b 100644
>>> --- a/drivers/irqchip/irq-mbigen.c
>>> +++ b/drivers/irqchip/irq-mbigen.c
>>> @@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>>  		err = -EINVAL;
>>>  
>>>  	if (err) {
>>> -		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
>>> -			mgn_chip->base);
>>> +		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain");
>>
>> The alternative would be to print res as a resource, which would still
>> help identifying the offending device by printing its physical
>> layout, and still not reveal much.
> 
> It's better to print res to show the physical layout, and add missing "\n",
> will resend v2.

As Hanjun mentioned in a separate email, pdev->dev seems to be enough to
identify which MBIGEN has failed to probe. So maybe all you need to do
is to add the missing '\n', and tidy up the commit message to reflect that.

Thanks,

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

Re: [PATCH] irqchip/mbigen: stop printing kernel addresses

[Kefeng Wang]

On 2019/6/18 16:42, Marc Zyngier wrote:
> On 18/06/2019 09:35, Kefeng Wang wrote:
>>
>>
>> On 2019/6/18 15:48, Marc Zyngier wrote:
>>> Hi Kefeng,
>>>
>>> On Tue, 18 Jun 2019 04:22:02 +0100,
>>> Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>>>
>>>> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
>>>> it will print "____ptrval____" instead of actual addresses when mbigen
>>>> create domain fails,
>>>>
>>>>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>>>>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
>>>>
>>>> Instead of changing the print to "%px", and leaking kernel addresses,
>>>> just remove the print completely.
>>>>
>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>> ---
>>>>  drivers/irqchip/irq-mbigen.c | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>>>> index 98b6e1d4b1a6..d0cf596c801b 100644
>>>> --- a/drivers/irqchip/irq-mbigen.c
>>>> +++ b/drivers/irqchip/irq-mbigen.c
>>>> @@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
>>>>  		err = -EINVAL;
>>>>  
>>>>  	if (err) {
>>>> -		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
>>>> -			mgn_chip->base);
>>>> +		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain");
>>>
>>> The alternative would be to print res as a resource, which would still
>>> help identifying the offending device by printing its physical
>>> layout, and still not reveal much.
>>
>> It's better to print res to show the physical layout, and add missing "\n",
>> will resend v2.
> 
> As Hanjun mentioned in a separate email, pdev->dev seems to be enough to
> identify which MBIGEN has failed to probe. So maybe all you need to do
> is to add the missing '\n', and tidy up the commit message to reflect that.

OK, done in v2.

> 
> Thanks,
> 
> 	M.
> 

[PATCH v2] irqchip/mbigen: stop printing kernel addresses

[Kefeng Wang]

After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
it will print "____ptrval____" instead of actual addresses when mbigen
create domain fails,

  Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
  Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12

dev_xxx() helper contains the device info, HISI0152:00, which stands for
mbigen ACPI HID and its UID, we can identify the failing probed mbigen,
so just remove the printing "mgn_chip->base", and also add missing "\n".

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 98b6e1d4b1a6..c0f65ea0ae0f 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
 		err = -EINVAL;
 
 	if (err) {
-		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
-			mgn_chip->base);
+		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain\n");
 		return err;
 	}
 
-- 
2.20.1

Re: [PATCH v2] irqchip/mbigen: stop printing kernel addresses

[Hanjun Guo]

On 2019/6/18 17:15, Kefeng Wang wrote:
> After commit ad67b74d2469d9b8 ("printk: hash addresses printed with %p"),
> it will print "____ptrval____" instead of actual addresses when mbigen
> create domain fails,
> 
>   Hisilicon MBIGEN-V2 HISI0152:00: Failed to create mbi-gen@(____ptrval____) irqdomain
>   Hisilicon MBIGEN-V2: probe of HISI0152:00 failed with error -12
> 
> dev_xxx() helper contains the device info, HISI0152:00, which stands for
> mbigen ACPI HID and its UID, we can identify the failing probed mbigen,
> so just remove the printing "mgn_chip->base", and also add missing "\n".
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/irqchip/irq-mbigen.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 98b6e1d4b1a6..c0f65ea0ae0f 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -355,8 +355,7 @@ static int mbigen_device_probe(struct platform_device *pdev)
>  		err = -EINVAL;
>  
>  	if (err) {
> -		dev_err(&pdev->dev, "Failed to create mbi-gen@%p irqdomain",
> -			mgn_chip->base);
> +		dev_err(&pdev->dev, "Failed to create mbi-gen irqdomain\n");
>  		return err;

Reviewed-by: Hanjun Guo <guohanjun@huawei.com>