Hi,
The following series of patches cleans up some suspend-related code.
Greetings,
Rafael
--
You never change things by fighting the existing reality.
R. Buckminster Fuller
Move all labels in the swsusp code to the second column, so that they won't
fool diff -p.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/disk.c | 6 +++---
kernel/power/snapshot.c | 8 ++++----
kernel/power/swap.c | 4 ++--
kernel/power/swsusp.c | 2 +-
kernel/power/user.c | 2 +-
5 files changed, 11 insertions(+), 11 deletions(-)
Index: linux-2.6.19-rc5-mm2/kernel/power/disk.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/disk.c
+++ linux-2.6.19-rc5-mm2/kernel/power/disk.c
@@ -119,9 +119,9 @@ static int prepare_processes(void)
return 0;
platform_finish();
-thaw:
+ thaw:
thaw_processes();
-enable_cpus:
+ enable_cpus:
enable_nonboot_cpus();
pm_restore_console();
return error;
@@ -394,7 +394,7 @@ static ssize_t resume_store(struct subsy
noresume = 0;
software_resume();
ret = n;
-out:
+ out:
return ret;
}
Index: linux-2.6.19-rc5-mm2/kernel/power/snapshot.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/snapshot.c
+++ linux-2.6.19-rc5-mm2/kernel/power/snapshot.c
@@ -411,7 +411,7 @@ memory_bm_create(struct memory_bitmap *b
memory_bm_position_reset(bm);
return 0;
-Free:
+ Free:
bm->p_list = ca.chain;
memory_bm_free(bm, PG_UNSAFE_CLEAR);
return -ENOMEM;
@@ -557,7 +557,7 @@ static unsigned long memory_bm_next_pfn(
memory_bm_position_reset(bm);
return BM_END_OF_MAP;
-Return_pfn:
+ Return_pfn:
bm->cur.chunk = chunk;
bm->cur.bit = bit;
return bb->start_pfn + chunk * BM_BITS_PER_CHUNK + bit;
@@ -964,7 +964,7 @@ swsusp_alloc(struct memory_bitmap *orig_
}
return 0;
-Free:
+ Free:
swsusp_free();
return -ENOMEM;
}
@@ -1540,7 +1540,7 @@ prepare_image(struct memory_bitmap *new_
}
return 0;
-Free:
+ Free:
swsusp_free();
return error;
}
Index: linux-2.6.19-rc5-mm2/kernel/power/swsusp.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/swsusp.c
+++ linux-2.6.19-rc5-mm2/kernel/power/swsusp.c
@@ -288,7 +288,7 @@ int swsusp_suspend(void)
* that suspended with irqs off ... no overall powerup.
*/
device_power_up();
-Enable_irqs:
+ Enable_irqs:
local_irq_enable();
return error;
}
Index: linux-2.6.19-rc5-mm2/kernel/power/user.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/user.c
+++ linux-2.6.19-rc5-mm2/kernel/power/user.c
@@ -313,7 +313,7 @@ static int snapshot_ioctl(struct inode *
if (pm_ops->finish)
pm_ops->finish(PM_SUSPEND_MEM);
-OutS3:
+ OutS3:
up(&pm_sem);
break;
Index: linux-2.6.19-rc5-mm2/kernel/power/swap.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/swap.c
+++ linux-2.6.19-rc5-mm2/kernel/power/swap.c
@@ -301,7 +301,7 @@ static int swap_write_page(struct swap_m
handle->cur_swap = offset;
handle->k = 0;
}
-out:
+ out:
return error;
}
@@ -429,7 +429,7 @@ int swsusp_write(void)
if (error)
free_all_swap_pages(root_swap, handle.bitmap);
release_swap_writer(&handle);
-out:
+ out:
swsusp_close();
return error;
}
Fix coding style in suspend.c.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/snapshot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.19-rc5-mm2/kernel/power/snapshot.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/snapshot.c
+++ linux-2.6.19-rc5-mm2/kernel/power/snapshot.c
@@ -85,7 +85,8 @@ unsigned long get_safe_page(gfp_t gfp_ma
return (unsigned long)get_image_page(gfp_mask, PG_SAFE);
}
-static struct page *alloc_image_page(gfp_t gfp_mask) {
+static struct page *alloc_image_page(gfp_t gfp_mask)
+{
struct page *page;
page = alloc_page(gfp_mask);
Move the loop from thaw_processes() to a separate function and call it
independently for kernel threads and user space processes so that the order
of thawing tasks is clearly visible.
Drop thaw_kernel_threads() which is never used.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/linux/freezer.h | 6 -----
kernel/power/process.c | 55 +++++++++++++++++++++++++++---------------------
2 files changed, 33 insertions(+), 28 deletions(-)
Index: linux-2.6.19-rc5-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.19-rc5-mm2.orig/include/linux/freezer.h
+++ linux-2.6.19-rc5-mm2/include/linux/freezer.h
@@ -1,8 +1,5 @@
/* Freezer declarations */
-#define FREEZER_KERNEL_THREADS 0
-#define FREEZER_ALL_THREADS 1
-
#ifdef CONFIG_PM
/*
* Check if a process has been frozen
@@ -60,8 +57,7 @@ static inline void frozen_process(struct
extern void refrigerator(void);
extern int freeze_processes(void);
-#define thaw_processes() do { thaw_some_processes(FREEZER_ALL_THREADS); } while(0)
-#define thaw_kernel_threads() do { thaw_some_processes(FREEZER_KERNEL_THREADS); } while(0)
+extern void thaw_processes(void);
static inline int try_to_freeze(void)
{
Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
+++ linux-2.6.19-rc5-mm2/kernel/power/process.c
@@ -20,6 +20,8 @@
*/
#define TIMEOUT (20 * HZ)
+#define FREEZER_KERNEL_THREADS 0
+#define FREEZER_USER_SPACE 1
static inline int freezeable(struct task_struct * p)
{
@@ -79,6 +81,11 @@ static void cancel_freezing(struct task_
}
}
+static inline int is_user_space(struct task_struct *p)
+{
+ return p->mm && !(p->flags & PF_BORROWED_MM);
+}
+
/* 0 = success, else # of processes that we failed to stop */
int freeze_processes(void)
{
@@ -103,10 +110,9 @@ int freeze_processes(void)
cancel_freezing(p);
continue;
}
- if (p->mm && !(p->flags & PF_BORROWED_MM)) {
- /* The task is a user-space one.
- * Freeze it unless there's a vfork completion
- * pending
+ if (is_user_space(p)) {
+ /* Freeze the task unless there is a vfork
+ * completion pending
*/
if (!p->vfork_done)
freeze_process(p);
@@ -155,31 +161,34 @@ int freeze_processes(void)
return 0;
}
-void thaw_some_processes(int all)
+static void thaw_tasks(int thaw_user_space)
{
struct task_struct *g, *p;
- int pass = 0; /* Pass 0 = Kernel space, 1 = Userspace */
- printk("Restarting tasks... ");
read_lock(&tasklist_lock);
- do {
- do_each_thread(g, p) {
- /*
- * is_user = 0 if kernel thread or borrowed mm,
- * 1 otherwise.
- */
- int is_user = !!(p->mm && !(p->flags & PF_BORROWED_MM));
- if (!freezeable(p) || (is_user != pass))
- continue;
- if (!thaw_process(p))
- printk(KERN_INFO
- "Strange, %s not stopped\n", p->comm);
- } while_each_thread(g, p);
-
- pass++;
- } while (pass < 2 && all);
+ do_each_thread(g, p) {
+ if (!freezeable(p))
+ continue;
+ if (is_user_space(p)) {
+ if (!thaw_user_space)
+ continue;
+ } else {
+ if (thaw_user_space)
+ continue;
+ }
+ if (!thaw_process(p))
+ printk(KERN_WARNING " Strange, %s not stopped\n",
+ p->comm );
+ } while_each_thread(g, p);
read_unlock(&tasklist_lock);
+}
+
+void thaw_processes(void)
+{
+ printk("Restarting tasks ... ");
+ thaw_tasks(FREEZER_KERNEL_THREADS);
+ thaw_tasks(FREEZER_USER_SPACE);
schedule();
printk("done.\n");
}
Move the loop from freeze_processes() to a separate function and call it
independently for user space processes and kernel threads so that the order of
freezing tasks is clearly visible.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/process.c | 88 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 58 insertions(+), 30 deletions(-)
Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
+++ linux-2.6.19-rc5-mm2/kernel/power/process.c
@@ -86,24 +86,23 @@ static inline int is_user_space(struct t
return p->mm && !(p->flags & PF_BORROWED_MM);
}
-/* 0 = success, else # of processes that we failed to stop */
-int freeze_processes(void)
+static unsigned int try_to_freeze_tasks(int freeze_user_space)
{
- int todo, nr_user, user_frozen;
- unsigned long start_time;
struct task_struct *g, *p;
+ unsigned long end_time;
+ unsigned int todo;
- printk("Stopping tasks... ");
- start_time = jiffies;
- user_frozen = 0;
+ end_time = jiffies + TIMEOUT;
do {
- nr_user = todo = 0;
+ todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!freezeable(p))
continue;
+
if (frozen(p))
continue;
+
if (p->state == TASK_TRACED &&
(frozen(p->parent) ||
p->parent->state == TASK_STOPPED)) {
@@ -111,51 +110,80 @@ int freeze_processes(void)
continue;
}
if (is_user_space(p)) {
+ if (!freeze_user_space)
+ continue;
+
/* Freeze the task unless there is a vfork
* completion pending
*/
if (!p->vfork_done)
freeze_process(p);
- nr_user++;
} else {
- /* Freeze only if the user space is frozen */
- if (user_frozen)
- freeze_process(p);
- todo++;
+ if (freeze_user_space)
+ continue;
+
+ freeze_process(p);
}
+ todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
- todo += nr_user;
- if (!user_frozen && !nr_user) {
- sys_sync();
- start_time = jiffies;
- }
- user_frozen = !nr_user;
yield(); /* Yield is okay here */
- if (todo && time_after(jiffies, start_time + TIMEOUT))
+ if (todo && time_after(jiffies, end_time))
break;
- } while(todo);
+ } while (todo);
- /* This does not unfreeze processes that are already frozen
- * (we have slightly ugly calling convention in that respect,
- * and caller must call thaw_processes() if something fails),
- * but it cleans up leftover PF_FREEZE requests.
- */
if (todo) {
+ /* This does not unfreeze processes that are already frozen
+ * (we have slightly ugly calling convention in that respect,
+ * and caller must call thaw_processes() if something fails),
+ * but it cleans up leftover PF_FREEZE requests.
+ */
printk("\n");
- printk(KERN_ERR "Stopping tasks timed out "
- "after %d seconds (%d tasks remaining):\n",
- TIMEOUT / HZ, todo);
+ printk(KERN_ERR "Stopping %s timed out after %d seconds "
+ "(%d tasks refusing to freeze):\n",
+ freeze_user_space ? "user space processes" :
+ "kernel threads",
+ TIMEOUT / HZ, todo);
read_lock(&tasklist_lock);
do_each_thread(g, p) {
+ if (is_user_space(p)) {
+ if(!freeze_user_space)
+ continue;
+ } else {
+ if(freeze_user_space)
+ continue;
+ }
if (freezeable(p) && !frozen(p))
printk(KERN_ERR " %s\n", p->comm);
+
cancel_freezing(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
- return todo;
}
+ return todo;
+}
+
+/**
+ * freeze_processes - tell processes to enter the refrigerator
+ *
+ * Returns 0 on success, or the number of processes that didn't freeze,
+ * although they were told to.
+ */
+int freeze_processes(void)
+{
+ unsigned int nr_unfrozen;
+
+ printk("Stopping tasks ... ");
+ nr_unfrozen = try_to_freeze_tasks(FREEZER_USER_SPACE);
+ if (nr_unfrozen)
+ return nr_unfrozen;
+
+ sys_sync();
+ nr_unfrozen = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
+ if (nr_unfrozen)
+ return nr_unfrozen;
+
printk("done.\n");
BUG_ON(in_atomic());
return 0;
Hi.
On Sat, 2006-11-18 at 23:47 +0100, Rafael J. Wysocki wrote:
> Move the loop from thaw_processes() to a separate function and call it
> independently for kernel threads and user space processes so that the order
> of thawing tasks is clearly visible.
>
> Drop thaw_kernel_threads() which is never used.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Ack.
Nigel
Hi.
On Sat, 2006-11-18 at 23:51 +0100, Rafael J. Wysocki wrote:
> Move all labels in the swsusp code to the second column, so that they won't
> fool diff -p.
This sounds like working around brokenness in diff -p. Should/could a
patch be submitted to the diff maintainer instead?
Regards,
Nigel
On Sunday, 19 November 2006 00:06, Nigel Cunningham wrote:
> Hi.
>
> On Sat, 2006-11-18 at 23:51 +0100, Rafael J. Wysocki wrote:
> > Move all labels in the swsusp code to the second column, so that they won't
> > fool diff -p.
>
> This sounds like working around brokenness in diff -p. Should/could a
> patch be submitted to the diff maintainer instead?
No. This feature of diff is actually documented.
There was a discussion on LKML about it some time ago and the patch follows
the conclusion.
Greetings,
Rafael
--
You never change things by fighting the existing reality.
R. Buckminster Fuller
On Sat 2006-11-18 23:51:01, Rafael J. Wysocki wrote:
> Move all labels in the swsusp code to the second column, so that they won't
> fool diff -p.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
ACK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> Move the loop from thaw_processes() to a separate function and call it
> independently for kernel threads and user space processes so that the order
> of thawing tasks is clearly visible.
>
> Drop thaw_kernel_threads() which is never used.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> include/linux/freezer.h | 6 -----
> kernel/power/process.c | 55 +++++++++++++++++++++++++++---------------------
> 2 files changed, 33 insertions(+), 28 deletions(-)
>
> Index: linux-2.6.19-rc5-mm2/include/linux/freezer.h
> ===================================================================
> --- linux-2.6.19-rc5-mm2.orig/include/linux/freezer.h
> +++ linux-2.6.19-rc5-mm2/include/linux/freezer.h
> @@ -1,8 +1,5 @@
> /* Freezer declarations */
>
> -#define FREEZER_KERNEL_THREADS 0
> -#define FREEZER_ALL_THREADS 1
> -
> #ifdef CONFIG_PM
> /*
> * Check if a process has been frozen
> @@ -60,8 +57,7 @@ static inline void frozen_process(struct
>
> extern void refrigerator(void);
> extern int freeze_processes(void);
> -#define thaw_processes() do { thaw_some_processes(FREEZER_ALL_THREADS); } while(0)
> -#define thaw_kernel_threads() do { thaw_some_processes(FREEZER_KERNEL_THREADS); } while(0)
> +extern void thaw_processes(void);
>
> static inline int try_to_freeze(void)
> {
> Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
> ===================================================================
> --- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
> +++ linux-2.6.19-rc5-mm2/kernel/power/process.c
> @@ -20,6 +20,8 @@
> */
> #define TIMEOUT (20 * HZ)
>
> +#define FREEZER_KERNEL_THREADS 0
> +#define FREEZER_USER_SPACE 1
The variable is named "is_user_space"... so maybe the defines are not
strictly needed?
> + do_each_thread(g, p) {
> + if (!freezeable(p))
> + continue;
>
> + if (is_user_space(p)) {
> + if (!thaw_user_space)
> + continue;
> + } else {
> + if (thaw_user_space)
> + continue;
> + }
if (is_user_space(p) != thaw_user_space)
continue;
?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sat 2006-11-18 23:48:35, Rafael J. Wysocki wrote:
> Fix coding style in suspend.c.
ACK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> Move the loop from freeze_processes() to a separate function and call it
> independently for user space processes and kernel threads so that the order of
> freezing tasks is clearly visible.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> kernel/power/process.c | 88 ++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 58 insertions(+), 30 deletions(-)
>
> do_each_thread(g, p) {
> + if (is_user_space(p)) {
> + if(!freeze_user_space)
> + continue;
> + } else {
> + if(freeze_user_space)
> + continue;
> + }
Needs space between if and (, but lets use if (is_user_space() !=
freeze_user_space) trick here, too.
> +/**
> + * freeze_processes - tell processes to enter the refrigerator
> + *
> + * Returns 0 on success, or the number of processes that didn't freeze,
> + * although they were told to.
> + */
Above you point out to the broken calling convention. Perhaps this is
great time to fix it, too? Hmm, or maybe not. Patch looks good,
otherwise.
Pavel
> +int freeze_processes(void)
> +{
> + unsigned int nr_unfrozen;
> +
> + printk("Stopping tasks ... ");
> + nr_unfrozen = try_to_freeze_tasks(FREEZER_USER_SPACE);
> + if (nr_unfrozen)
> + return nr_unfrozen;
> +
> + sys_sync();
> + nr_unfrozen = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
> + if (nr_unfrozen)
> + return nr_unfrozen;
> +
> printk("done.\n");
> BUG_ON(in_atomic());
> return 0;
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi,
On Sunday, 19 November 2006 03:09, Pavel Machek wrote:
> Hi!
>
> > Move the loop from freeze_processes() to a separate function and call it
> > independently for user space processes and kernel threads so that the order of
> > freezing tasks is clearly visible.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > kernel/power/process.c | 88 ++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 58 insertions(+), 30 deletions(-)
> >
>
> > do_each_thread(g, p) {
> > + if (is_user_space(p)) {
> > + if(!freeze_user_space)
> > + continue;
> > + } else {
> > + if(freeze_user_space)
> > + continue;
> > + }
>
> Needs space between if and (, but lets use if (is_user_space() !=
> freeze_user_space) trick here, too.
OK
New version of the patch follows.
---
Move the loop from freeze_processes() to a separate function and call it
independently for user space processes and kernel threads so that the order of
freezing tasks is clearly visible.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/process.c | 84 +++++++++++++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 30 deletions(-)
Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
+++ linux-2.6.19-rc5-mm2/kernel/power/process.c
@@ -86,24 +86,23 @@ static inline int is_user_space(struct t
return p->mm && !(p->flags & PF_BORROWED_MM);
}
-/* 0 = success, else # of processes that we failed to stop */
-int freeze_processes(void)
+static unsigned int try_to_freeze_tasks(int freeze_user_space)
{
- int todo, nr_user, user_frozen;
- unsigned long start_time;
struct task_struct *g, *p;
+ unsigned long end_time;
+ unsigned int todo;
- printk("Stopping tasks... ");
- start_time = jiffies;
- user_frozen = 0;
+ end_time = jiffies + TIMEOUT;
do {
- nr_user = todo = 0;
+ todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!freezeable(p))
continue;
+
if (frozen(p))
continue;
+
if (p->state == TASK_TRACED &&
(frozen(p->parent) ||
p->parent->state == TASK_STOPPED)) {
@@ -111,51 +110,76 @@ int freeze_processes(void)
continue;
}
if (is_user_space(p)) {
+ if (!freeze_user_space)
+ continue;
+
/* Freeze the task unless there is a vfork
* completion pending
*/
if (!p->vfork_done)
freeze_process(p);
- nr_user++;
} else {
- /* Freeze only if the user space is frozen */
- if (user_frozen)
- freeze_process(p);
- todo++;
+ if (freeze_user_space)
+ continue;
+
+ freeze_process(p);
}
+ todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
- todo += nr_user;
- if (!user_frozen && !nr_user) {
- sys_sync();
- start_time = jiffies;
- }
- user_frozen = !nr_user;
yield(); /* Yield is okay here */
- if (todo && time_after(jiffies, start_time + TIMEOUT))
+ if (todo && time_after(jiffies, end_time))
break;
- } while(todo);
+ } while (todo);
- /* This does not unfreeze processes that are already frozen
- * (we have slightly ugly calling convention in that respect,
- * and caller must call thaw_processes() if something fails),
- * but it cleans up leftover PF_FREEZE requests.
- */
if (todo) {
+ /* This does not unfreeze processes that are already frozen
+ * (we have slightly ugly calling convention in that respect,
+ * and caller must call thaw_processes() if something fails),
+ * but it cleans up leftover PF_FREEZE requests.
+ */
printk("\n");
- printk(KERN_ERR "Stopping tasks timed out "
- "after %d seconds (%d tasks remaining):\n",
- TIMEOUT / HZ, todo);
+ printk(KERN_ERR "Stopping %s timed out after %d seconds "
+ "(%d tasks refusing to freeze):\n",
+ freeze_user_space ? "user space processes" :
+ "kernel threads",
+ TIMEOUT / HZ, todo);
read_lock(&tasklist_lock);
do_each_thread(g, p) {
+ if (is_user_space(p) == !freeze_user_space)
+ continue;
+
if (freezeable(p) && !frozen(p))
printk(KERN_ERR " %s\n", p->comm);
+
cancel_freezing(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
- return todo;
}
+ return todo;
+}
+
+/**
+ * freeze_processes - tell processes to enter the refrigerator
+ *
+ * Returns 0 on success, or the number of processes that didn't freeze,
+ * although they were told to.
+ */
+int freeze_processes(void)
+{
+ unsigned int nr_unfrozen;
+
+ printk("Stopping tasks ... ");
+ nr_unfrozen = try_to_freeze_tasks(FREEZER_USER_SPACE);
+ if (nr_unfrozen)
+ return nr_unfrozen;
+
+ sys_sync();
+ nr_unfrozen = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
+ if (nr_unfrozen)
+ return nr_unfrozen;
+
printk("done.\n");
BUG_ON(in_atomic());
return 0;
Hi,
On Sunday, 19 November 2006 03:04, Pavel Machek wrote:
> Hi!
>
> > Move the loop from thaw_processes() to a separate function and call it
> > independently for kernel threads and user space processes so that the order
> > of thawing tasks is clearly visible.
> >
> > Drop thaw_kernel_threads() which is never used.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > include/linux/freezer.h | 6 -----
> > kernel/power/process.c | 55 +++++++++++++++++++++++++++---------------------
> > 2 files changed, 33 insertions(+), 28 deletions(-)
> >
> > Index: linux-2.6.19-rc5-mm2/include/linux/freezer.h
> > ===================================================================
> > --- linux-2.6.19-rc5-mm2.orig/include/linux/freezer.h
> > +++ linux-2.6.19-rc5-mm2/include/linux/freezer.h
> > @@ -1,8 +1,5 @@
> > /* Freezer declarations */
> >
> > -#define FREEZER_KERNEL_THREADS 0
> > -#define FREEZER_ALL_THREADS 1
> > -
> > #ifdef CONFIG_PM
> > /*
> > * Check if a process has been frozen
> > @@ -60,8 +57,7 @@ static inline void frozen_process(struct
> >
> > extern void refrigerator(void);
> > extern int freeze_processes(void);
> > -#define thaw_processes() do { thaw_some_processes(FREEZER_ALL_THREADS); } while(0)
> > -#define thaw_kernel_threads() do { thaw_some_processes(FREEZER_KERNEL_THREADS); } while(0)
> > +extern void thaw_processes(void);
> >
> > static inline int try_to_freeze(void)
> > {
> > Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
> > ===================================================================
> > --- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
> > +++ linux-2.6.19-rc5-mm2/kernel/power/process.c
> > @@ -20,6 +20,8 @@
> > */
> > #define TIMEOUT (20 * HZ)
> >
> > +#define FREEZER_KERNEL_THREADS 0
> > +#define FREEZER_USER_SPACE 1
>
> The variable is named "is_user_space"... so maybe the defines are not
> strictly needed?
Not strlctly, but if I see
thaw_tasks(FREEZER_KERNEL_THREADS);
thaw_tasks(FREEZER_USER_SPACE);
I can figure out what's going on even without looking at thaw_tasks().
> > + do_each_thread(g, p) {
> > + if (!freezeable(p))
> > + continue;
> >
> > + if (is_user_space(p)) {
> > + if (!thaw_user_space)
> > + continue;
> > + } else {
> > + if (thaw_user_space)
> > + continue;
> > + }
>
> if (is_user_space(p) != thaw_user_space)
> continue;
Ah, good idea. In fact I prefer if (is_user_space(p) == !thaw_user_space),
because it will work even if thaw_user_space is different to 1 and 0.
Revised patch follows.
---
Move the loop from thaw_processes() to a separate function and call it
independently for kernel threads and user space processes so that the order
of thawing tasks is clearly visible.
Drop thaw_kernel_threads() which is never used.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/linux/freezer.h | 6 -----
kernel/power/process.c | 53 ++++++++++++++++++++++++++----------------------
2 files changed, 30 insertions(+), 29 deletions(-)
Index: linux-2.6.19-rc5-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.19-rc5-mm2.orig/include/linux/freezer.h
+++ linux-2.6.19-rc5-mm2/include/linux/freezer.h
@@ -1,8 +1,5 @@
/* Freezer declarations */
-#define FREEZER_KERNEL_THREADS 0
-#define FREEZER_ALL_THREADS 1
-
#ifdef CONFIG_PM
/*
* Check if a process has been frozen
@@ -60,8 +57,7 @@ static inline void frozen_process(struct
extern void refrigerator(void);
extern int freeze_processes(void);
-#define thaw_processes() do { thaw_some_processes(FREEZER_ALL_THREADS); } while(0)
-#define thaw_kernel_threads() do { thaw_some_processes(FREEZER_KERNEL_THREADS); } while(0)
+extern void thaw_processes(void);
static inline int try_to_freeze(void)
{
Index: linux-2.6.19-rc5-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.19-rc5-mm2.orig/kernel/power/process.c
+++ linux-2.6.19-rc5-mm2/kernel/power/process.c
@@ -20,6 +20,8 @@
*/
#define TIMEOUT (20 * HZ)
+#define FREEZER_KERNEL_THREADS 0
+#define FREEZER_USER_SPACE 1
static inline int freezeable(struct task_struct * p)
{
@@ -79,6 +81,11 @@ static void cancel_freezing(struct task_
}
}
+static inline int is_user_space(struct task_struct *p)
+{
+ return p->mm && !(p->flags & PF_BORROWED_MM);
+}
+
/* 0 = success, else # of processes that we failed to stop */
int freeze_processes(void)
{
@@ -103,10 +110,9 @@ int freeze_processes(void)
cancel_freezing(p);
continue;
}
- if (p->mm && !(p->flags & PF_BORROWED_MM)) {
- /* The task is a user-space one.
- * Freeze it unless there's a vfork completion
- * pending
+ if (is_user_space(p)) {
+ /* Freeze the task unless there is a vfork
+ * completion pending
*/
if (!p->vfork_done)
freeze_process(p);
@@ -155,31 +161,30 @@ int freeze_processes(void)
return 0;
}
-void thaw_some_processes(int all)
+static void thaw_tasks(int thaw_user_space)
{
struct task_struct *g, *p;
- int pass = 0; /* Pass 0 = Kernel space, 1 = Userspace */
- printk("Restarting tasks... ");
read_lock(&tasklist_lock);
- do {
- do_each_thread(g, p) {
- /*
- * is_user = 0 if kernel thread or borrowed mm,
- * 1 otherwise.
- */
- int is_user = !!(p->mm && !(p->flags & PF_BORROWED_MM));
- if (!freezeable(p) || (is_user != pass))
- continue;
- if (!thaw_process(p))
- printk(KERN_INFO
- "Strange, %s not stopped\n", p->comm);
- } while_each_thread(g, p);
-
- pass++;
- } while (pass < 2 && all);
-
+ do_each_thread(g, p) {
+ if (!freezeable(p))
+ continue;
+
+ if (is_user_space(p) == !thaw_user_space)
+ continue;
+
+ if (!thaw_process(p))
+ printk(KERN_WARNING " Strange, %s not stopped\n",
+ p->comm );
+ } while_each_thread(g, p);
read_unlock(&tasklist_lock);
+}
+
+void thaw_processes(void)
+{
+ printk("Restarting tasks ... ");
+ thaw_tasks(FREEZER_KERNEL_THREADS);
+ thaw_tasks(FREEZER_USER_SPACE);
schedule();
printk("done.\n");
}
Hi!
> Ah, good idea. In fact I prefer if (is_user_space(p) == !thaw_user_space),
> because it will work even if thaw_user_space is different to 1 and 0.
>
> Revised patch follows.
>
> ---
> Move the loop from thaw_processes() to a separate function and call it
> independently for kernel threads and user space processes so that the order
> of thawing tasks is clearly visible.
>
> Drop thaw_kernel_threads() which is never used.
ACK.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> > Needs space between if and (, but lets use if (is_user_space() !=
> > freeze_user_space) trick here, too.
>
> OK
>
> New version of the patch follows.
>
> ---
> Move the loop from freeze_processes() to a separate function and call it
> independently for user space processes and kernel threads so that the order of
> freezing tasks is clearly visible.
ACK.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, 19 Nov 2006 00:25:06 +0100 Rafael J. Wysocki wrote:
> On Sunday, 19 November 2006 00:06, Nigel Cunningham wrote:
> > Hi.
> >
> > On Sat, 2006-11-18 at 23:51 +0100, Rafael J. Wysocki wrote:
> > > Move all labels in the swsusp code to the second column, so that they won't
> > > fool diff -p.
> >
> > This sounds like working around brokenness in diff -p. Should/could a
> > patch be submitted to the diff maintainer instead?
>
> No. This feature of diff is actually documented.
>
> There was a discussion on LKML about it some time ago and the patch follows
> the conclusion.
There was discussion, and then both Jesper and I tried to
reproduce the problem with diff and could not do so.
Maybe it has already been fixed. (?)
---
~Randy
On Wednesday, 22 November 2006 23:32, Randy Dunlap wrote:
> On Sun, 19 Nov 2006 00:25:06 +0100 Rafael J. Wysocki wrote:
>
> > On Sunday, 19 November 2006 00:06, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Sat, 2006-11-18 at 23:51 +0100, Rafael J. Wysocki wrote:
> > > > Move all labels in the swsusp code to the second column, so that they won't
> > > > fool diff -p.
> > >
> > > This sounds like working around brokenness in diff -p. Should/could a
> > > patch be submitted to the diff maintainer instead?
> >
> > No. This feature of diff is actually documented.
> >
> > There was a discussion on LKML about it some time ago and the patch follows
> > the conclusion.
>
> There was discussion, and then both Jesper and I tried to
> reproduce the problem with diff and could not do so.
> Maybe it has already been fixed. (?)
Well, I'm not sure.
Greetings,
Rafael
--
You never change things by fighting the existing reality.
R. Buckminster Fuller