Re: [PATCH] printk: Userspace format enumeration support

Joe Perches
2021-02-06T17:58:12Z

On Fri, 2021-02-05 at 22:25 +0000, Chris Down wrote:
> Petr Mladek writes:
> >   + <module> is already optinaly added by pr_fmt() to the printed strings
> >     as: pr_fmt(): ...
>
> pr_fmts are not consistently used across the kernel, and sometimes differ from
> the module itself. Many modules don't use it at all, and we also don't have it
> for pr_cont. Just picking some random examples:
>
>      % grep -av vmlinux /proc/printk_formats | shuf -n 10
>      mac80211,6%s: mesh STA %pM switches to channel requiring DFS (%d MHz, width:%d, CF1/2: %d/%d MHz), aborting
>      thinkpad_acpi,c N/Athinkpad_acpi,c %dthinkpad_acpi,5thinkpad_acpi: temperatures (Celsius):thinkpad_acpi,3thinkpad_acpi: Out of memory for LED data

I don't understand this format.

"Out of memory for LED data" is a single printk ending with a '\n' newline
I expected this to be broken up into multiple lines, one for each printk
that endsd in a newline.

And what would happen if the function was refactored removing the pr_cont
uses like the below: (basically, any output that uses a mechanism that
aggregates a buffer then emits it, and there are a _lot_ of those)

printk("%s\n", buffer);

And there is already a relatively trivial way to do this using a modified
version of strings that looks for KERN_SOH[0-6], and if dynamic_debug is
enabled, look in the dynamic_debug section, either __verbose or __dyndbg
depending on the kernel version.

---
drivers/platform/x86/thinkpad_acpi.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 18b390153e7f..ff1c09c600f8 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6353,21 +6353,26 @@ static void thermal_dump_all_sensors(void)
{
int n, i;
struct ibm_thermal_sensors_struct t;
+ char output[256];
+ int len = 0;

n = thermal_get_sensors(&t);
if (n <= 0)
return;

- pr_notice("temperatures (Celsius):");
+ len += scnprintf(output + len, sizeof(output) - len,
+ "temperatures (Celsius):");

for (i = 0; i < n; i++) {
- if (t.temp[i] != TPACPI_THERMAL_SENSOR_NA)
- pr_cont(" %d", (int)(t.temp[i] / 1000));
+ if (t.temp[i] == TPACPI_THERMAL_SENSOR_NA)
+ len += scnprintf(output + len, sizeof(output) - len,
+ " N/A");
else
- pr_cont(" N/A");
+ len += scnprintf(output + len, sizeof(output) - len,
+ " %d", t.temp[i] / 1000);
}

- pr_cont("\n");
+ pr_notice("%s\n", output);
}

/* sysfs temp##_input -------------------------------------------------- */