lkml.org 
[lkml]   [2022]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 0/3] printk: introduce new macros pr_<level>_cont()
On 2022-11-30 18:57+0100, Rafael J. Wysocki wrote:
> On Fri, Nov 25, 2022 at 8:10 PM Thomas Weißschuh <linux@weissschuh.net> wrote:
>>
>> This series adds new printk wrapper macros pr_<level>_cont().
>> These create continuation messages with an explicit level.
>>
>> Explicit levels are useful when a continuation message is split from its main
>> message. Without the explicit level KERN_DEFAULT ("warn" by default) is used
>> which can lead to stray partial log messages when filtering by level.
>>
>> Also checkpatch is modified to recommend the new macros over plain pr_cont().
>>
>> Lastly the new macros are used in kernel/power/process.c as this file uses
>> continuation messages during system suspend-resume which creates a high
>> likelyhood of interspersed messages.
>
> Well, if process.c is the only problematic piece of code in this
> respect, I'm not sure if adding the new infrastructure for its benefit
> alone is worth it, because it can very well do without pr_cont() at
> all.

In general all usages of pr_cont() are problematic.
Any continuation can be split from its main message, leading to misleved
continuations.
process.c is just the one that I noticed reliably hitting this problem on my
machine.

> Please see the patch below (compiled only, sorry for gmail-induced
> white space damage). I'll submit it properly later if it works for
> everyone.

The patch looks fine to me and getting rid of usages of pr_cont() seems to be
the better aproach where it is possible.

Petr: do you still want me to submit the new macros even if it is not used
directly anymore?

> ---
> kernel/power/process.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> Index: linux-pm/kernel/power/process.c
> ===================================================================
> --- linux-pm.orig/kernel/power/process.c
> +++ linux-pm/kernel/power/process.c
> @@ -27,6 +27,8 @@ unsigned int __read_mostly freeze_timeou
>
> static int try_to_freeze_tasks(bool user_only)
> {
> + const char *what = user_only ? "user space processes" :
> + "remaining freezable tasks";
> struct task_struct *g, *p;
> unsigned long end_time;
> unsigned int todo;
> @@ -36,6 +38,8 @@ static int try_to_freeze_tasks(bool user
> bool wakeup = false;
> int sleep_usecs = USEC_PER_MSEC;
>
> + pr_info("Freezing %s\n", what);
> +
> start = ktime_get_boottime();
>
> end_time = jiffies + msecs_to_jiffies(freeze_timeout_msecs);
> @@ -82,9 +86,8 @@ static int try_to_freeze_tasks(bool user
> elapsed_msecs = ktime_to_ms(elapsed);
>
> if (todo) {
> - pr_cont("\n");
> - pr_err("Freezing of tasks %s after %d.%03d seconds "
> - "(%d tasks refusing to freeze, wq_busy=%d):\n",
> + pr_err("Freezing %s %s after %d.%03d seconds "
> + "(%d tasks refusing to freeze, wq_busy=%d):\n", what,
> wakeup ? "aborted" : "failed",
> elapsed_msecs / 1000, elapsed_msecs % 1000,
> todo - wq_busy, wq_busy);
> @@ -101,8 +104,8 @@ static int try_to_freeze_tasks(bool user
> read_unlock(&tasklist_lock);
> }
> } else {
> - pr_cont("(elapsed %d.%03d seconds) ", elapsed_msecs / 1000,
> - elapsed_msecs % 1000);
> + pr_info("Freezing %s completed (elapsed %d.%03d seconds)\n",
> + what, elapsed_msecs / 1000, elapsed_msecs % 1000);
> }
>
> return todo ? -EBUSY : 0;
> @@ -130,14 +133,11 @@ int freeze_processes(void)
> static_branch_inc(&freezer_active);
>
> pm_wakeup_clear(0);
> - pr_info("Freezing user space processes ... ");
> pm_freezing = true;
> error = try_to_freeze_tasks(true);
> - if (!error) {
> + if (!error)
> __usermodehelper_set_disable_depth(UMH_DISABLED);
> - pr_cont("done.");
> - }
> - pr_cont("\n");
> +
> BUG_ON(in_atomic());
>
> /*
> @@ -166,14 +166,9 @@ int freeze_kernel_threads(void)
> {
> int error;
>
> - pr_info("Freezing remaining freezable tasks ... ");
> -
> pm_nosig_freezing = true;
> error = try_to_freeze_tasks(false);
> - if (!error)
> - pr_cont("done.");
>
> - pr_cont("\n");
> BUG_ON(in_atomic());
>
> if (error)

\
 
 \ /
  Last update: 2022-12-01 00:38    [W:0.177 / U:1.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site