The freezing of processes is currently very noisy. This patch makes the
noise dependant upon CONFIG_PM_DEBUG.
Prepared against current git.
Signed-off-by: Nigel Cunningham <[email protected]>
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 29be608..6829612 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -15,6 +15,12 @@ #include <linux/module.h>
#include <linux/syscalls.h>
#include <linux/freezer.h>
+#ifdef CONFIG_PM_DEBUG
+#define freezer_message(msg, a...) do { printk(msg, ##a); } while(0)
+#else
+#define freezer_message(msg, a...) do { } while(0)
+#endif
+
/*
* Timeout for stopping processes
*/
@@ -40,7 +46,7 @@ void refrigerator(void)
long save;
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
- printk("=");
+ freezer_message("=");
frozen_process(current);
spin_lock_irq(¤t->sighand->siglock);
@@ -87,7 +93,7 @@ int freeze_processes(void)
unsigned long start_time;
struct task_struct *g, *p;
- printk( "Stopping tasks: " );
+ freezer_message( "Stopping tasks: " );
start_time = jiffies;
user_frozen = 0;
do {
@@ -135,7 +141,7 @@ int freeze_processes(void)
* but it cleans up leftover PF_FREEZE requests.
*/
if (todo) {
- printk( "\n" );
+ freezer_message( "\n" );
printk(KERN_ERR " stopping tasks timed out "
"after %d seconds (%d tasks remaining):\n",
TIMEOUT / HZ, todo);
@@ -149,7 +155,7 @@ int freeze_processes(void)
return todo;
}
- printk( "|\n" );
+ freezer_message( "|\n" );
BUG_ON(in_atomic());
return 0;
}
@@ -158,18 +164,18 @@ void thaw_processes(void)
{
struct task_struct *g, *p;
- printk( "Restarting tasks..." );
+ freezer_message( "Restarting tasks..." );
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!freezeable(p))
continue;
if (!thaw_process(p))
- printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
+ freezer_message(KERN_INFO " Strange, %s not stopped\n", p->comm );
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
schedule();
- printk( " done\n" );
+ freezer_message( " done\n" );
}
EXPORT_SYMBOL(refrigerator);
On Saturday, 21 October 2006 14:22, Nigel Cunningham wrote:
> The freezing of processes is currently very noisy. This patch makes the
> noise dependant upon CONFIG_PM_DEBUG.
Well, I don't think it's _that_ noisy. It only printks one character per
frozen task.
In fact I think it at least should print "Freezing processes" and "done"
messages.
> Prepared against current git.
>
> Signed-off-by: Nigel Cunningham <[email protected]>
>
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 29be608..6829612 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -15,6 +15,12 @@ #include <linux/module.h>
> #include <linux/syscalls.h>
> #include <linux/freezer.h>
>
> +#ifdef CONFIG_PM_DEBUG
> +#define freezer_message(msg, a...) do { printk(msg, ##a); } while(0)
> +#else
> +#define freezer_message(msg, a...) do { } while(0)
> +#endif
> +
> /*
> * Timeout for stopping processes
> */
> @@ -40,7 +46,7 @@ void refrigerator(void)
> long save;
> save = current->state;
> pr_debug("%s entered refrigerator\n", current->comm);
> - printk("=");
> + freezer_message("=");
I'd prefer to treat the pr_debug thing in a similar way, like
freezer_debug_message("%s entered refrigerator\n" ...).
Or better yet, I'd leave just one message like
freezer_print_task(current->comm);
instead of the two that will be defined to either printk the entire
"%s entered refrigerator\n", ... message or printk the "=" character or do
nothing, depending on a predefined verbosity level.
>
> frozen_process(current);
> spin_lock_irq(¤t->sighand->siglock);
> @@ -87,7 +93,7 @@ int freeze_processes(void)
> unsigned long start_time;
> struct task_struct *g, *p;
>
> - printk( "Stopping tasks: " );
> + freezer_message( "Stopping tasks: " );
I wouldn't change this.
> start_time = jiffies;
> user_frozen = 0;
> do {
> @@ -135,7 +141,7 @@ int freeze_processes(void)
> * but it cleans up leftover PF_FREEZE requests.
> */
> if (todo) {
> - printk( "\n" );
> + freezer_message( "\n" );
Ditto.
> printk(KERN_ERR " stopping tasks timed out "
> "after %d seconds (%d tasks remaining):\n",
> TIMEOUT / HZ, todo);
> @@ -149,7 +155,7 @@ int freeze_processes(void)
> return todo;
> }
>
> - printk( "|\n" );
> + freezer_message( "|\n" );
I'd call it freezer_print_finish(); and define to printk the "|\n" or do
nothing like for freezer_print_task().
> BUG_ON(in_atomic());
> return 0;
> }
> @@ -158,18 +164,18 @@ void thaw_processes(void)
> {
> struct task_struct *g, *p;
>
> - printk( "Restarting tasks..." );
I wouldn't change this.
> + freezer_message( "Restarting tasks..." );
> read_lock(&tasklist_lock);
> do_each_thread(g, p) {
> if (!freezeable(p))
> continue;
> if (!thaw_process(p))
> - printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
> + freezer_message(KERN_INFO " Strange, %s not stopped\n", p->comm );
> } while_each_thread(g, p);
>
> read_unlock(&tasklist_lock);
> schedule();
> - printk( " done\n" );
> + freezer_message( " done\n" );
Ditto.
> }
>
> EXPORT_SYMBOL(refrigerator);
>
>
> -
Greetings,
Rafael
--
You never change things by fighting the existing reality.
R. Buckminster Fuller
Hi.
On Sat, 2006-10-21 at 16:01 +0200, Rafael J. Wysocki wrote:
> On Saturday, 21 October 2006 14:22, Nigel Cunningham wrote:
> > The freezing of processes is currently very noisy. This patch makes the
> > noise dependant upon CONFIG_PM_DEBUG.
>
> Well, I don't think it's _that_ noisy. It only printks one character per
> frozen task.
Yeah, but it should just work. The only reason for it to printk should
be for information or if there's a possibility of deadlocking or failure
that isn't handled. Doing printks here is akin to doing a printk for
every page you write in the image, and I'm reasonably sure you're not
about to start doing that.
> In fact I think it at least should print "Freezing processes" and "done"
> messages.
That sounds better.
> > Prepared against current git.
> >
> > Signed-off-by: Nigel Cunningham <[email protected]>
> >
> > diff --git a/kernel/power/process.c b/kernel/power/process.c
> > index 29be608..6829612 100644
> > --- a/kernel/power/process.c
> > +++ b/kernel/power/process.c
> > @@ -15,6 +15,12 @@ #include <linux/module.h>
> > #include <linux/syscalls.h>
> > #include <linux/freezer.h>
> >
> > +#ifdef CONFIG_PM_DEBUG
> > +#define freezer_message(msg, a...) do { printk(msg, ##a); } while(0)
> > +#else
> > +#define freezer_message(msg, a...) do { } while(0)
> > +#endif
> > +
> > /*
> > * Timeout for stopping processes
> > */
> > @@ -40,7 +46,7 @@ void refrigerator(void)
> > long save;
> > save = current->state;
> > pr_debug("%s entered refrigerator\n", current->comm);
> > - printk("=");
> > + freezer_message("=");
>
> I'd prefer to treat the pr_debug thing in a similar way, like
> freezer_debug_message("%s entered refrigerator\n" ...).
>
> Or better yet, I'd leave just one message like
>
> freezer_print_task(current->comm);
>
> instead of the two that will be defined to either printk the entire
> "%s entered refrigerator\n", ... message or printk the "=" character or do
> nothing, depending on a predefined verbosity level.
Ah, yeah. Didn't notice the pr_debug thing. Will look at it.
> >
> > frozen_process(current);
> > spin_lock_irq(¤t->sighand->siglock);
> > @@ -87,7 +93,7 @@ int freeze_processes(void)
> > unsigned long start_time;
> > struct task_struct *g, *p;
> >
> > - printk( "Stopping tasks: " );
> > + freezer_message( "Stopping tasks: " );
>
> I wouldn't change this.
>
> > start_time = jiffies;
> > user_frozen = 0;
> > do {
> > @@ -135,7 +141,7 @@ int freeze_processes(void)
> > * but it cleans up leftover PF_FREEZE requests.
> > */
> > if (todo) {
> > - printk( "\n" );
> > + freezer_message( "\n" );
>
> Ditto.
>
> > printk(KERN_ERR " stopping tasks timed out "
> > "after %d seconds (%d tasks remaining):\n",
> > TIMEOUT / HZ, todo);
> > @@ -149,7 +155,7 @@ int freeze_processes(void)
> > return todo;
> > }
> >
> > - printk( "|\n" );
> > + freezer_message( "|\n" );
>
> I'd call it freezer_print_finish(); and define to printk the "|\n" or do
> nothing like for freezer_print_task().
>
> > BUG_ON(in_atomic());
> > return 0;
> > }
> > @@ -158,18 +164,18 @@ void thaw_processes(void)
> > {
> > struct task_struct *g, *p;
> >
> > - printk( "Restarting tasks..." );
>
> I wouldn't change this.
>
> > + freezer_message( "Restarting tasks..." );
> > read_lock(&tasklist_lock);
> > do_each_thread(g, p) {
> > if (!freezeable(p))
> > continue;
> > if (!thaw_process(p))
> > - printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
> > + freezer_message(KERN_INFO " Strange, %s not stopped\n", p->comm );
> > } while_each_thread(g, p);
> >
> > read_unlock(&tasklist_lock);
> > schedule();
> > - printk( " done\n" );
> > + freezer_message( " done\n" );
>
> Ditto.
>
> > }
> >
> > EXPORT_SYMBOL(refrigerator);
Will reconsider and resend tomorrow.
Regards,
Nigel
Hi!
> The freezing of processes is currently very noisy. This patch makes the
> noise dependant upon CONFIG_PM_DEBUG.
>
> Prepared against current git.
>
> Signed-off-by: Nigel Cunningham <[email protected]>
>
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 29be608..6829612 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -15,6 +15,12 @@ #include <linux/module.h>
> #include <linux/syscalls.h>
> #include <linux/freezer.h>
>
> +#ifdef CONFIG_PM_DEBUG
> +#define freezer_message(msg, a...) do { printk(msg, ##a); } while(0)
> +#else
> +#define freezer_message(msg, a...) do { } while(0)
> +#endif
> +
No. We already have pr_debug().
And having stopping tasks: ============== line does not seem that bad
to me. Drivers are so noisy that this is very minor. Feel free to
adjust loglevels so that message is hidden.
...all the messages you modified should probably be fixed to be KERN_INFO...
> @@ -158,18 +164,18 @@ void thaw_processes(void)
> {
> struct task_struct *g, *p;
>
> - printk( "Restarting tasks..." );
> + freezer_message( "Restarting tasks..." );
> read_lock(&tasklist_lock);
> do_each_thread(g, p) {
> if (!freezeable(p))
> continue;
> if (!thaw_process(p))
> - printk(KERN_INFO " Strange, %s not stopped\n", p->comm );
> + freezer_message(KERN_INFO " Strange, %s not
stopped\n", p->comm );
This one definitely needs to stay, maybe should be promoted to
KERN_WARNING or something.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html