* [PATCH] Quieten freezer if !CONFIG_PM_DEBUG.
@ 2006-10-21 12:22 Nigel Cunningham
2006-10-21 12:44 ` [linux-pm] " Pavel Machek
2006-10-21 14:01 ` Rafael J. Wysocki
0 siblings, 2 replies; 6+ messages in thread
From: Nigel Cunningham @ 2006-10-21 12:22 UTC (permalink / raw)
To: Linux PM, LKML, suspend2-devel
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 <nigel@suspend2.net>
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [linux-pm] [PATCH] Quieten freezer if !CONFIG_PM_DEBUG.
2006-10-21 12:22 [PATCH] Quieten freezer if !CONFIG_PM_DEBUG Nigel Cunningham
@ 2006-10-21 12:44 ` Pavel Machek
2006-10-21 14:01 ` Rafael J. Wysocki
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2006-10-21 12:44 UTC (permalink / raw)
To: Nigel Cunningham; +Cc: Linux PM, LKML, suspend2-devel
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 <nigel@suspend2.net>
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Quieten freezer if !CONFIG_PM_DEBUG.
2006-10-21 12:22 [PATCH] Quieten freezer if !CONFIG_PM_DEBUG Nigel Cunningham
2006-10-21 12:44 ` [linux-pm] " Pavel Machek
@ 2006-10-21 14:01 ` Rafael J. Wysocki
2006-10-21 22:32 ` Nigel Cunningham
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2006-10-21 14:01 UTC (permalink / raw)
To: Nigel Cunningham; +Cc: Linux PM, LKML, suspend2-devel
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 <nigel@suspend2.net>
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Quieten freezer if !CONFIG_PM_DEBUG.
2006-10-21 14:01 ` Rafael J. Wysocki
@ 2006-10-21 22:32 ` Nigel Cunningham
0 siblings, 0 replies; 6+ messages in thread
From: Nigel Cunningham @ 2006-10-21 22:32 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM, LKML, suspend2-devel
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 <nigel@suspend2.net>
> >
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Quieten Freezer if !CONFIG_PM_DEBUG
@ 2006-10-22 23:35 Nigel Cunningham
2006-10-23 15:10 ` Pavel Machek
0 siblings, 1 reply; 6+ messages in thread
From: Nigel Cunningham @ 2006-10-22 23:35 UTC (permalink / raw)
To: Andrew Morton, Rafael J. Wysocki, LKML
The freezer currently prints an '=' for every process that is frozen.
This is pretty pointless, as the equals sign says nothing about which
process is frozen, and makes logs look messier (especially if there were
a large number of processes running). All we really need to know is that
we started trying to freeze processes and what processes (if any) failed
to freeze, or that we succeeded.
Signed-off-by: Nigel Cunningham <nigel@suspend2.net>
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 29be608..b0edfc6 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -40,7 +40,6 @@ void refrigerator(void)
long save;
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
- printk("=");
frozen_process(current);
spin_lock_irq(¤t->sighand->siglock);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Quieten Freezer if !CONFIG_PM_DEBUG
2006-10-22 23:35 [PATCH] Quieten Freezer " Nigel Cunningham
@ 2006-10-23 15:10 ` Pavel Machek
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2006-10-23 15:10 UTC (permalink / raw)
To: Nigel Cunningham; +Cc: Andrew Morton, Rafael J. Wysocki, LKML
Hi!
> The freezer currently prints an '=' for every process that is frozen.
> This is pretty pointless, as the equals sign says nothing about which
> process is frozen, and makes logs look messier (especially if there were
> a large number of processes running). All we really need to know is that
> we started trying to freeze processes and what processes (if any) failed
> to freeze, or that we succeeded.
> Signed-off-by: Nigel Cunningham <nigel@suspend2.net>
ACK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-10-23 15:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-21 12:22 [PATCH] Quieten freezer if !CONFIG_PM_DEBUG Nigel Cunningham
2006-10-21 12:44 ` [linux-pm] " Pavel Machek
2006-10-21 14:01 ` Rafael J. Wysocki
2006-10-21 22:32 ` Nigel Cunningham
-- strict thread matches above, loose matches on Subject: below --
2006-10-22 23:35 [PATCH] Quieten Freezer " Nigel Cunningham
2006-10-23 15:10 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox