Re: [PATCH] printk: Userspace format enumeration support

[Joe Perches <joe@perches.com>]

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 -------------------------------------------------- */