From: Onkarnath <[email protected]>
As %pe is already introduced, it's better to use it in place of (%ld) for
printing errors in logs. It would enhance readability of logs.
Signed-off-by: Maninder Singh <[email protected]>
Signed-off-by: Onkarnath <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
---
v1 -> v2: Updated subject line as per file history & corrected spellings
in description.
v2 -> v3: Updated Reviewed-by tag.
drivers/acpi/acpi_processor.c | 2 +-
drivers/acpi/acpi_watchdog.c | 2 +-
drivers/acpi/pci_slot.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 4fe2ef54088c..2ddd36a21850 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
if (IS_ERR(pdev))
- pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
+ pr_info("%s device creation failed: %pe\n", name, pdev);
}
#ifdef CONFIG_X86
diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
index 8e9e001da38f..14b24157799c 100644
--- a/drivers/acpi/acpi_watchdog.c
+++ b/drivers/acpi/acpi_watchdog.c
@@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
resources, nresources);
if (IS_ERR(pdev))
- pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
+ pr_err("Device creation failed: %pe\n", pdev);
kfree(resources);
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index d6cb2c27a23b..741bcc9d6d6a 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
snprintf(name, sizeof(name), "%llu", sun);
pci_slot = pci_create_slot(pci_bus, device, name, NULL);
if (IS_ERR(pci_slot)) {
- pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+ pr_err("pci_create_slot returned %pe\n", pci_slot);
kfree(slot);
return AE_OK;
}
--
2.25.1
From: Onkarnath <[email protected]>
Instead of printing errros as a number(%ld), it's better to print in string
format for better readability of logs.
Signed-off-by: Onkarnath <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
---
v1 -> v2: Updated subject as per file history.
v2 -> v3: No change in this patch, change done in PATCH v3 1/2.
kernel/sched/cpufreq_schedutil.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index eece6244f9d2..2c42eaa56fa3 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -671,7 +671,7 @@ static int sugov_kthread_create(struct sugov_policy *sg_policy)
"sugov:%d",
cpumask_first(policy->related_cpus));
if (IS_ERR(thread)) {
- pr_err("failed to create sugov thread: %ld\n", PTR_ERR(thread));
+ pr_err("failed to create sugov thread: %pe\n", thread);
return PTR_ERR(thread);
}
--
2.25.1
On Tue, Feb 13, 2024 at 01:14:15PM +0530, Onkarnarth wrote:
> From: Onkarnath <[email protected]>
>
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.
Here are some more candidates that I see regularly:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/dd.c?id=v6.7#n590
Something like:
git grep "\(_info(\|_warn(\).*%d"
finds a ton of them (plus a lot of unrelated hits, of course). If you
were to do this for drivers/pci/, I would want them all for the whole
directory in a single patch, and I would take the opportunity to make
minor changes so the style is more consistent, e.g.,
"... failed (%pe)" or something.
Bjorn
On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <[email protected]> wrote:
>
> From: Onkarnath <[email protected]>
>
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.
>
> Signed-off-by: Maninder Singh <[email protected]>
What exactly is the role of this S-o-b? Has the person helped you to
develop the patch or something else?
> Signed-off-by: Onkarnath <[email protected]>
> Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> v1 -> v2: Updated subject line as per file history & corrected spellings
> in description.
> v2 -> v3: Updated Reviewed-by tag.
>
> drivers/acpi/acpi_processor.c | 2 +-
> drivers/acpi/acpi_watchdog.c | 2 +-
> drivers/acpi/pci_slot.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 4fe2ef54088c..2ddd36a21850 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
>
> pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
> if (IS_ERR(pdev))
> - pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
> + pr_info("%s device creation failed: %pe\n", name, pdev);
> }
>
> #ifdef CONFIG_X86
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> index 8e9e001da38f..14b24157799c 100644
> --- a/drivers/acpi/acpi_watchdog.c
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
> pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
> resources, nresources);
> if (IS_ERR(pdev))
> - pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
> + pr_err("Device creation failed: %pe\n", pdev);
>
> kfree(resources);
>
> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
> index d6cb2c27a23b..741bcc9d6d6a 100644
> --- a/drivers/acpi/pci_slot.c
> +++ b/drivers/acpi/pci_slot.c
> @@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
> snprintf(name, sizeof(name), "%llu", sun);
> pci_slot = pci_create_slot(pci_bus, device, name, NULL);
> if (IS_ERR(pci_slot)) {
> - pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
> + pr_err("pci_create_slot returned %pe\n", pci_slot);
> kfree(slot);
> return AE_OK;
> }
> --
>On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <[email protected]> wrote:
>>
>> From: Onkarnath <[email protected]>
>>
>> As %pe is already introduced, it's better to use it in place of (%ld) for
>> printing errors in logs. It would enhance readability of logs.
>>
>> Signed-off-by: Maninder Singh <[email protected]>
>
>What exactly is the role of this S-o-b? Has the person helped you to
>develop the patch or something else?
>
Yes It was meant for Co-developed tag, Because We are working collectively for making errors more readable for our product kernel.(5.4)
And some part of this patch was made by him.
Then we checked that it is also suggested by open source to have %pe for printing errors:
https://lore.kernel.org/all/[email protected]/
So I prepared same changes for open source kernel, and because of smaller patch I kept it as normal signed-off tag only.
If it is needed I can resend with Co-developed tag.
Thanks,
Onkarnath
On Fri, Feb 16, 2024 at 6:54 AM Onkarnath <[email protected]> wrote:
>
> >On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <[email protected]> wrote:
> >>
> >> From: Onkarnath <[email protected]>
> >>
> >> As %pe is already introduced, it's better to use it in place of (%ld) for
> >> printing errors in logs. It would enhance readability of logs.
> >>
> >> Signed-off-by: Maninder Singh <[email protected]>
> >
> >What exactly is the role of this S-o-b? Has the person helped you to
> >develop the patch or something else?
> >
>
> Yes It was meant for Co-developed tag, Because We are working collectively for making errors more readable for our product kernel.(5.4)
> And some part of this patch was made by him.
>
> Then we checked that it is also suggested by open source to have %pe for printing errors:
> https://lore.kernel.org/all/[email protected]/
>
> So I prepared same changes for open source kernel, and because of smaller patch I kept it as normal signed-off tag only.
> If it is needed I can resend with Co-developed tag.
No need, I can add it for you. Thanks for the explanation!
On Tue, Feb 13, 2024 at 9:20 AM Onkarnarth <[email protected]> wrote:
>
> From: Onkarnath <[email protected]>
>
> As %pe is already introduced, it's better to use it in place of (%ld) for
> printing errors in logs. It would enhance readability of logs.
>
> Signed-off-by: Maninder Singh <[email protected]>
> Signed-off-by: Onkarnath <[email protected]>
> Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> v1 -> v2: Updated subject line as per file history & corrected spellings
> in description.
> v2 -> v3: Updated Reviewed-by tag.
>
> drivers/acpi/acpi_processor.c | 2 +-
> drivers/acpi/acpi_watchdog.c | 2 +-
> drivers/acpi/pci_slot.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 4fe2ef54088c..2ddd36a21850 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -161,7 +161,7 @@ static void cpufreq_add_device(const char *name)
>
> pdev = platform_device_register_simple(name, PLATFORM_DEVID_NONE, NULL, 0);
> if (IS_ERR(pdev))
> - pr_info("%s device creation failed: %ld\n", name, PTR_ERR(pdev));
> + pr_info("%s device creation failed: %pe\n", name, pdev);
> }
>
> #ifdef CONFIG_X86
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> index 8e9e001da38f..14b24157799c 100644
> --- a/drivers/acpi/acpi_watchdog.c
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -179,7 +179,7 @@ void __init acpi_watchdog_init(void)
> pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
> resources, nresources);
> if (IS_ERR(pdev))
> - pr_err("Device creation failed: %ld\n", PTR_ERR(pdev));
> + pr_err("Device creation failed: %pe\n", pdev);
>
> kfree(resources);
>
> diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
> index d6cb2c27a23b..741bcc9d6d6a 100644
> --- a/drivers/acpi/pci_slot.c
> +++ b/drivers/acpi/pci_slot.c
> @@ -111,7 +111,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
> snprintf(name, sizeof(name), "%llu", sun);
> pci_slot = pci_create_slot(pci_bus, device, name, NULL);
> if (IS_ERR(pci_slot)) {
> - pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
> + pr_err("pci_create_slot returned %pe\n", pci_slot);
> kfree(slot);
> return AE_OK;
> }
> --
Applied as 6.9 material, thanks!