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

Reply via email to