[RFC] Add the "icebox"

[RFC] Add the "icebox"

[Alan Stern]

Rafael:

This patch adds the icebox, for use by kernel threads that want to 
freeze themselves without using the freezer -- which is likely to crop 
up when the freezer gets eliminated.

It's straightforward enough.  It could be used as-is, for example for 
freezing workqueue threads.  Using it for other sorts of kernel threads 
seems to be rather difficult, requiring a fair amount of extra code 
that's hard to centralize since each thread will have to be woken up 
its own way.

Do you have any particular suggestions?

Alan Stern


Index: usb-2.6/include/linux/freezer.h
===================================================================
--- usb-2.6.orig/include/linux/freezer.h
+++ usb-2.6/include/linux/freezer.h
@@ -157,6 +157,12 @@ static inline void set_freezable(void)
 	} while (try_to_freeze());					\
 	__retval;							\
 })
+
+/*
+ * Kernel threads that want to freeze themselves go into the icebox.
+ */
+extern void icebox(void);
+
 #else /* !CONFIG_PM_SLEEP */
 static inline int frozen(struct task_struct *p) { return 0; }
 static inline int freezing(struct task_struct *p) { return 0; }
@@ -181,6 +187,7 @@ static inline void set_freezable(void) {
 #define wait_event_freezable_timeout(wq, condition, timeout)		\
 		wait_event_interruptible_timeout(wq, condition, timeout)
 
+static inline void icebox(void) {}
 #endif /* !CONFIG_PM_SLEEP */
 
 #endif	/* FREEZER_H_INCLUDED */
Index: usb-2.6/kernel/power/power.h
===================================================================
--- usb-2.6.orig/kernel/power/power.h
+++ usb-2.6/kernel/power/power.h
@@ -205,3 +205,8 @@ static inline int suspend_devices_and_en
 
 /* kernel/power/process.c */
 extern int pm_notifier_call_chain(unsigned long val);
+
+#ifdef CONFIG_PM_SLEEP
+extern void start_icebox(void);
+extern void stop_icebox(void);
+#endif
Index: usb-2.6/kernel/power/main.c
===================================================================
--- usb-2.6.orig/kernel/power/main.c
+++ usb-2.6/kernel/power/main.c
@@ -71,6 +71,8 @@ static int suspend_prepare(void)
 	if (!suspend_ops || !suspend_ops->enter)
 		return -EPERM;
 
+	start_icebox();
+
 	error = pm_notifier_call_chain(PM_SUSPEND_PREPARE);
 	if (error)
 		goto Finish;
@@ -98,6 +100,7 @@ static int suspend_prepare(void)
 	thaw_processes();
 	pm_restore_console();
  Finish:
+	stop_icebox();
 	pm_notifier_call_chain(PM_POST_SUSPEND);
 	return error;
 }
@@ -191,6 +194,7 @@ static void suspend_finish(void)
 {
 	thaw_processes();
 	pm_restore_console();
+	stop_icebox();
 	pm_notifier_call_chain(PM_POST_SUSPEND);
 }
 
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -389,6 +389,8 @@ int hibernate(void)
 		goto Unlock;
 	}
 
+	start_icebox();
+
 	error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
 	if (error)
 		goto Exit;
@@ -431,6 +433,7 @@ int hibernate(void)
  Finish:
 	free_basic_memory_bitmaps();
  Exit:
+	stop_icebox();
 	pm_notifier_call_chain(PM_POST_HIBERNATION);
 	atomic_inc(&snapshot_device_available);
  Unlock:
@@ -489,6 +492,8 @@ static int software_resume(void)
 		goto Unlock;
 	}
 
+	start_icebox();
+
 	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
 	if (error)
 		goto Finish;
@@ -516,6 +521,7 @@ static int software_resume(void)
  Done:
 	free_basic_memory_bitmaps();
  Finish:
+	stop_icebox();
 	pm_notifier_call_chain(PM_POST_RESTORE);
 	atomic_inc(&snapshot_device_available);
 	/* For success case, the suspend path will release the lock */
Index: usb-2.6/kernel/power/user.c
===================================================================
--- usb-2.6.orig/kernel/power/user.c
+++ usb-2.6/kernel/power/user.c
@@ -45,6 +45,7 @@ static int snapshot_open(struct inode *i
 {
 	struct snapshot_data *data;
 	int error;
+	unsigned long cancel;
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
 		return -EBUSY;
@@ -61,21 +62,22 @@ static int snapshot_open(struct inode *i
 	data = &snapshot_state;
 	filp->private_data = data;
 	memset(&data->handle, 0, sizeof(struct snapshot_handle));
+	start_icebox();
 	if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
 		data->swap = swsusp_resume_device ?
 			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
 		data->mode = O_RDONLY;
 		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
-		if (error)
-			pm_notifier_call_chain(PM_POST_RESTORE);
+		cancel = PM_POST_RESTORE;
 	} else {
 		data->swap = -1;
 		data->mode = O_WRONLY;
 		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
-		if (error)
-			pm_notifier_call_chain(PM_POST_HIBERNATION);
+		cancel = PM_POST_HIBERNATION;
 	}
 	if (error) {
+		stop_icebox();
+		pm_notifier_call_chain(cancel);
 		atomic_inc(&snapshot_device_available);
 		return error;
 	}
@@ -99,6 +101,7 @@ static int snapshot_release(struct inode
 		thaw_processes();
 		mutex_unlock(&pm_mutex);
 	}
+	stop_icebox();
 	pm_notifier_call_chain(data->mode == O_WRONLY ?
 			PM_POST_HIBERNATION : PM_POST_RESTORE);
 	atomic_inc(&snapshot_device_available);
Index: usb-2.6/kernel/power/process.c
===================================================================
--- usb-2.6.orig/kernel/power/process.c
+++ usb-2.6/kernel/power/process.c
@@ -14,6 +14,8 @@
 #include <linux/syscalls.h>
 #include <linux/freezer.h>
 
+#include "power.h"
+
 /* 
  * Timeout for stopping processes
  */
@@ -305,3 +307,49 @@ int pm_notifier_call_chain(unsigned long
 	return (blocking_notifier_call_chain(&pm_chain_head, val, NULL)
 			== NOTIFY_BAD) ? -EINVAL : 0;
 }
+
+/*
+ * Routines for kernel threads that want to freeze themselves
+ */
+static DECLARE_WAIT_QUEUE_HEAD(icebox_wait_queue_head);
+static int icebox_active;
+
+/**
+ * start_icebox -- activate the icebox
+ *
+ * Kernel power code should call this routine before sending the
+ * PM_HIBERNATION_PREPARE or PM_SUSPEND_PREPARE notifications.
+ */
+void start_icebox(void)
+{
+	icebox_active = 1;
+}
+
+/**
+ * stop_icebox -- deactivate the icebox and awaken waiting threads
+ *
+ * Kernel power code should call this routine before sending the
+ * PM_POST_HIBERNATION or PM_POST_SUSPEND notifications.
+ */
+void stop_icebox(void)
+{
+	icebox_active = 0;
+	wake_up_all(&icebox_wait_queue_head);
+}
+
+/**
+ * icebox -- place for kernel threads to wait during suspend or hibernation
+ *
+ * Threads can call this routine at any time.  It will return immediately
+ * unless a system suspend or hibernation has started and the icebox is
+ * active, in which case it won't return until the suspend/hibernation
+ * is over.
+ *
+ * Freezable kernel threads should use this routine rather than relying on
+ * the freezer.
+ */
+void icebox(void)
+{
+	wait_event(icebox_wait_queue_head, !icebox_active);
+}
+EXPORT_SYMBOL_GPL(icebox);

Re: [RFC] Add the "icebox"

[Rafael J. Wysocki]

On Monday, 5 of November 2007, Alan Stern wrote:
> Rafael:
> 
> This patch adds the icebox, for use by kernel threads that want to 
> freeze themselves without using the freezer -- which is likely to crop 
> up when the freezer gets eliminated.
> 
> It's straightforward enough.  It could be used as-is, for example for 
> freezing workqueue threads.  Using it for other sorts of kernel threads 
> seems to be rather difficult, requiring a fair amount of extra code 
> that's hard to centralize since each thread will have to be woken up 
> its own way.
> 
> Do you have any particular suggestions?

I like the idea, but I think that start_icebox() should be called after
freeze_processes().  Similarly, IMO it's better to call stop_icebox() before
thaw_processes().

Greetings,
Rafael


> Index: usb-2.6/include/linux/freezer.h
> ===================================================================
> --- usb-2.6.orig/include/linux/freezer.h
> +++ usb-2.6/include/linux/freezer.h
> @@ -157,6 +157,12 @@ static inline void set_freezable(void)
>  	} while (try_to_freeze());					\
>  	__retval;							\
>  })
> +
> +/*
> + * Kernel threads that want to freeze themselves go into the icebox.
> + */
> +extern void icebox(void);
> +
>  #else /* !CONFIG_PM_SLEEP */
>  static inline int frozen(struct task_struct *p) { return 0; }
>  static inline int freezing(struct task_struct *p) { return 0; }
> @@ -181,6 +187,7 @@ static inline void set_freezable(void) {
>  #define wait_event_freezable_timeout(wq, condition, timeout)		\
>  		wait_event_interruptible_timeout(wq, condition, timeout)
>  
> +static inline void icebox(void) {}
>  #endif /* !CONFIG_PM_SLEEP */
>  
>  #endif	/* FREEZER_H_INCLUDED */
> Index: usb-2.6/kernel/power/power.h
> ===================================================================
> --- usb-2.6.orig/kernel/power/power.h
> +++ usb-2.6/kernel/power/power.h
> @@ -205,3 +205,8 @@ static inline int suspend_devices_and_en
>  
>  /* kernel/power/process.c */
>  extern int pm_notifier_call_chain(unsigned long val);
> +
> +#ifdef CONFIG_PM_SLEEP
> +extern void start_icebox(void);
> +extern void stop_icebox(void);
> +#endif
> Index: usb-2.6/kernel/power/main.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/main.c
> +++ usb-2.6/kernel/power/main.c
> @@ -71,6 +71,8 @@ static int suspend_prepare(void)
>  	if (!suspend_ops || !suspend_ops->enter)
>  		return -EPERM;
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_SUSPEND_PREPARE);
>  	if (error)
>  		goto Finish;
> @@ -98,6 +100,7 @@ static int suspend_prepare(void)
>  	thaw_processes();
>  	pm_restore_console();
>   Finish:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_SUSPEND);
>  	return error;
>  }
> @@ -191,6 +194,7 @@ static void suspend_finish(void)
>  {
>  	thaw_processes();
>  	pm_restore_console();
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_SUSPEND);
>  }
>  
> Index: usb-2.6/kernel/power/disk.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/disk.c
> +++ usb-2.6/kernel/power/disk.c
> @@ -389,6 +389,8 @@ int hibernate(void)
>  		goto Unlock;
>  	}
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
>  	if (error)
>  		goto Exit;
> @@ -431,6 +433,7 @@ int hibernate(void)
>   Finish:
>  	free_basic_memory_bitmaps();
>   Exit:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_HIBERNATION);
>  	atomic_inc(&snapshot_device_available);
>   Unlock:
> @@ -489,6 +492,8 @@ static int software_resume(void)
>  		goto Unlock;
>  	}
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
>  	if (error)
>  		goto Finish;
> @@ -516,6 +521,7 @@ static int software_resume(void)
>   Done:
>  	free_basic_memory_bitmaps();
>   Finish:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_RESTORE);
>  	atomic_inc(&snapshot_device_available);
>  	/* For success case, the suspend path will release the lock */
> Index: usb-2.6/kernel/power/user.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/user.c
> +++ usb-2.6/kernel/power/user.c
> @@ -45,6 +45,7 @@ static int snapshot_open(struct inode *i
>  {
>  	struct snapshot_data *data;
>  	int error;
> +	unsigned long cancel;
>  
>  	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
>  		return -EBUSY;
> @@ -61,21 +62,22 @@ static int snapshot_open(struct inode *i
>  	data = &snapshot_state;
>  	filp->private_data = data;
>  	memset(&data->handle, 0, sizeof(struct snapshot_handle));
> +	start_icebox();
>  	if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
>  		data->swap = swsusp_resume_device ?
>  			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
>  		data->mode = O_RDONLY;
>  		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
> -		if (error)
> -			pm_notifier_call_chain(PM_POST_RESTORE);
> +		cancel = PM_POST_RESTORE;
>  	} else {
>  		data->swap = -1;
>  		data->mode = O_WRONLY;
>  		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
> -		if (error)
> -			pm_notifier_call_chain(PM_POST_HIBERNATION);
> +		cancel = PM_POST_HIBERNATION;
>  	}
>  	if (error) {
> +		stop_icebox();
> +		pm_notifier_call_chain(cancel);
>  		atomic_inc(&snapshot_device_available);
>  		return error;
>  	}
> @@ -99,6 +101,7 @@ static int snapshot_release(struct inode
>  		thaw_processes();
>  		mutex_unlock(&pm_mutex);
>  	}
> +	stop_icebox();
>  	pm_notifier_call_chain(data->mode == O_WRONLY ?
>  			PM_POST_HIBERNATION : PM_POST_RESTORE);
>  	atomic_inc(&snapshot_device_available);
> Index: usb-2.6/kernel/power/process.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/process.c
> +++ usb-2.6/kernel/power/process.c
> @@ -14,6 +14,8 @@
>  #include <linux/syscalls.h>
>  #include <linux/freezer.h>
>  
> +#include "power.h"
> +
>  /* 
>   * Timeout for stopping processes
>   */
> @@ -305,3 +307,49 @@ int pm_notifier_call_chain(unsigned long
>  	return (blocking_notifier_call_chain(&pm_chain_head, val, NULL)
>  			== NOTIFY_BAD) ? -EINVAL : 0;
>  }
> +
> +/*
> + * Routines for kernel threads that want to freeze themselves
> + */
> +static DECLARE_WAIT_QUEUE_HEAD(icebox_wait_queue_head);
> +static int icebox_active;
> +
> +/**
> + * start_icebox -- activate the icebox
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_HIBERNATION_PREPARE or PM_SUSPEND_PREPARE notifications.
> + */
> +void start_icebox(void)
> +{
> +	icebox_active = 1;
> +}
> +
> +/**
> + * stop_icebox -- deactivate the icebox and awaken waiting threads
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_POST_HIBERNATION or PM_POST_SUSPEND notifications.
> + */
> +void stop_icebox(void)
> +{
> +	icebox_active = 0;
> +	wake_up_all(&icebox_wait_queue_head);
> +}
> +
> +/**
> + * icebox -- place for kernel threads to wait during suspend or hibernation
> + *
> + * Threads can call this routine at any time.  It will return immediately
> + * unless a system suspend or hibernation has started and the icebox is
> + * active, in which case it won't return until the suspend/hibernation
> + * is over.
> + *
> + * Freezable kernel threads should use this routine rather than relying on
> + * the freezer.
> + */
> +void icebox(void)
> +{
> +	wait_event(icebox_wait_queue_head, !icebox_active);
> +}
> +EXPORT_SYMBOL_GPL(icebox);
> 
> 
> 



-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC] Add the "icebox"

[Alan Stern]

On Mon, 5 Nov 2007, Rafael J. Wysocki wrote:

> On Monday, 5 of November 2007, Alan Stern wrote:
> > Rafael:
> > 
> > This patch adds the icebox, for use by kernel threads that want to 
> > freeze themselves without using the freezer -- which is likely to crop 
> > up when the freezer gets eliminated.
> > 
> > It's straightforward enough.  It could be used as-is, for example for 
> > freezing workqueue threads.  Using it for other sorts of kernel threads 
> > seems to be rather difficult, requiring a fair amount of extra code 
> > that's hard to centralize since each thread will have to be woken up 
> > its own way.
> > 
> > Do you have any particular suggestions?
> 
> I like the idea, but I think that start_icebox() should be called after
> freeze_processes().  Similarly, IMO it's better to call stop_icebox() before
> thaw_processes().

The same thought occurred to me.  But consider: If at some point we do 
remove the freezer, then freezable kernel threads will rely on the 
PM_HIBERNATION_PREPARE notification to know when they should go into 
the icebox.  Hence the icebox has to be started before the 
notifications are sent, and that means before freeze_processes.  (In 
fact, once the freezer is gone it won't even make sense to ask whether 
start_icebox comes before or after freeze_processes!)

The experience of working on this has made we wonder whether we really 
should get rid of the freezer entirely.  Even if it is kept merely for 
freezing the handful of kernel threads (not user tasks!) that want to 
be frozen, it's still likely to be more efficient that having a bunch
of independent notifier routines, each sending a particular thread to 
the icebox.  If we do keep the freezer then maybe it's okay to start 
the icebox after the notications are sent.

Alan Stern

Re: [RFC] Add the "icebox"

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 512 bytes --]


> This patch adds the icebox, for use by kernel threads that want to 
> freeze themselves without using the freezer -- which is likely to crop 
> up when the freezer gets eliminated.

There obviously isn't a way to wait for all tasks that want to enter the
icebox since there's no task flag for this, will tasks need to add extra
locking? Or otherwise how do we make sure that a task hasn't *just* gone
past icebox() and starts executing the code that the icebox() is
supposed to prevent?

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Add the "icebox"

[Alan Stern]

On Wed, 7 Nov 2007, Johannes Berg wrote:

> > This patch adds the icebox, for use by kernel threads that want to 
> > freeze themselves without using the freezer -- which is likely to crop 
> > up when the freezer gets eliminated.
> 
> There obviously isn't a way to wait for all tasks that want to enter the
> icebox since there's no task flag for this, will tasks need to add extra
> locking? Or otherwise how do we make sure that a task hasn't *just* gone
> past icebox() and starts executing the code that the icebox() is
> supposed to prevent?

The idea was that kernel threads would have a PM-notifier routine and
would be sent to the icebox when the routine gets a
PM_{HIBERNATION|SUSPEND|RESTORE}_PREPARE message.  Presumably the
notifier routine wouldn't return until the thread was safely on its way
to the icebox.

You are correct that this would require some additional locking, plus 
the extra notifier routine, plus ways to handle timeouts, plus maybe 
some other stuff.  That's why I said in my earlier message that 
keeping the freezer around for these sorts of threads would be a good 
idea -- it would avoid all this new overhead.

The icebox would still be useful, however, for user tasks that try to
do I/O after a system-sleep transition has begun.  Right now this can't
happen, thanks to the freezer (except that it _can_ happen on PPC where
the freezer isn't used for suspend).  Once the freezer no longer
affects user tasks, kernel drivers will have to do _something_ when a
user task submits an I/O request during a system sleep.  That
"something" will be to go to the icebox.

Alan Stern

Re: [RFC] Add the "icebox"

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 1393 bytes --]


> The idea was that kernel threads would have a PM-notifier routine and
> would be sent to the icebox when the routine gets a
> PM_{HIBERNATION|SUSPEND|RESTORE}_PREPARE message.  Presumably the
> notifier routine wouldn't return until the thread was safely on its way
> to the icebox.
> 
> You are correct that this would require some additional locking, plus 
> the extra notifier routine, plus ways to handle timeouts, plus maybe 
> some other stuff.  That's why I said in my earlier message that 
> keeping the freezer around for these sorts of threads would be a good 
> idea -- it would avoid all this new overhead.

Ok, makes sense, thanks for the explanation.

> The icebox would still be useful, however, for user tasks that try to
> do I/O after a system-sleep transition has begun.  Right now this can't
> happen, thanks to the freezer (except that it _can_ happen on PPC where
> the freezer isn't used for suspend).  Once the freezer no longer
> affects user tasks, kernel drivers will have to do _something_ when a
> user task submits an I/O request during a system sleep.  That
> "something" will be to go to the icebox.

Right, ok. I wish I could change powerpc, I've been maintaining a patch
for months now, but Paul seems to be dead set against it. Not only does
that fix these things, but also we get to use the /sys/power/state
API...

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Add the "icebox"

[Alan Stern]

On Wed, 7 Nov 2007, Johannes Berg wrote:

> > The icebox would still be useful, however, for user tasks that try to
> > do I/O after a system-sleep transition has begun.  Right now this can't
> > happen, thanks to the freezer (except that it _can_ happen on PPC where
> > the freezer isn't used for suspend).  Once the freezer no longer
> > affects user tasks, kernel drivers will have to do _something_ when a
> > user task submits an I/O request during a system sleep.  That
> > "something" will be to go to the icebox.
> 
> Right, ok. I wish I could change powerpc, I've been maintaining a patch
> for months now, but Paul seems to be dead set against it. Not only does
> that fix these things, but also we get to use the /sys/power/state
> API...

This is just the start.  Nothing will _really_ get fixed until lots of 
drivers actually begin to use the icebox.

Alan Stern

Re: [RFC] Add the "icebox"

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 490 bytes --]


> > Right, ok. I wish I could change powerpc, I've been maintaining a patch
> > for months now, but Paul seems to be dead set against it. Not only does
> > that fix these things, but also we get to use the /sys/power/state
> > API...
> 
> This is just the start.  Nothing will _really_ get fixed until lots of 
> drivers actually begin to use the icebox.

No, I mean the patch that makes powerpc use the generic API fixes things
because it makes it use the freezer.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Add the "icebox"

[Alan Stern]

On Mon, 5 Nov 2007, Rafael J. Wysocki wrote:

> I like the idea, but I think that start_icebox() should be called after
> freeze_processes().  Similarly, IMO it's better to call stop_icebox() before
> thaw_processes().

Okay.  On the assumption that freezable kernel threads will continue to 
use the freezer and won't try to use the icebox, I have updated the 
patch.  Other comments?

Alan Stern

P.S.: What about mutual exclusion in kernel/power/user.c?  If we stop 
freezing user tasks, there's nothing to prevent more than one task 
holding an open file reference to the snapshot file and submitting 
ioctls concurrently.  Maybe the regions covered by pm_mutex should be 
expanded.



Index: usb-2.6/include/linux/freezer.h
===================================================================
--- usb-2.6.orig/include/linux/freezer.h
+++ usb-2.6/include/linux/freezer.h
@@ -157,6 +157,12 @@ static inline void set_freezable(void)
 	} while (try_to_freeze());					\
 	__retval;							\
 })
+
+/*
+ * Kernel threads that want to freeze themselves go into the icebox.
+ */
+extern void icebox(void);
+
 #else /* !CONFIG_PM_SLEEP */
 static inline int frozen(struct task_struct *p) { return 0; }
 static inline int freezing(struct task_struct *p) { return 0; }
@@ -181,6 +187,7 @@ static inline void set_freezable(void) {
 #define wait_event_freezable_timeout(wq, condition, timeout)		\
 		wait_event_interruptible_timeout(wq, condition, timeout)
 
+static inline void icebox(void) {}
 #endif /* !CONFIG_PM_SLEEP */
 
 #endif	/* FREEZER_H_INCLUDED */
Index: usb-2.6/kernel/power/power.h
===================================================================
--- usb-2.6.orig/kernel/power/power.h
+++ usb-2.6/kernel/power/power.h
@@ -207,3 +207,9 @@ static inline int suspend_devices_and_en
 /* kernel/power/main.c */
 extern int pm_notifier_call_chain(unsigned long val);
 #endif
+
+#ifdef CONFIG_PM_SLEEP
+/* kerne/power/process.c */
+extern void start_icebox(void);
+extern void stop_icebox(void);
+#endif
Index: usb-2.6/kernel/power/main.c
===================================================================
--- usb-2.6.orig/kernel/power/main.c
+++ usb-2.6/kernel/power/main.c
@@ -108,6 +108,8 @@ static int suspend_prepare(void)
 		goto Thaw;
 	}
 
+	start_icebox();
+
 	free_pages = global_page_state(NR_FREE_PAGES);
 	if (free_pages < FREE_PAGE_NUMBER) {
 		pr_debug("PM: free some memory\n");
@@ -120,6 +122,7 @@ static int suspend_prepare(void)
 	if (!error)
 		return 0;
 
+	stop_icebox();
  Thaw:
 	thaw_processes();
 	pm_restore_console();
@@ -215,6 +218,7 @@ int suspend_devices_and_enter(suspend_st
  */
 static void suspend_finish(void)
 {
+	stop_icebox();
 	thaw_processes();
 	pm_restore_console();
 	pm_notifier_call_chain(PM_POST_SUSPEND);
Index: usb-2.6/kernel/power/disk.c
===================================================================
--- usb-2.6.orig/kernel/power/disk.c
+++ usb-2.6/kernel/power/disk.c
@@ -406,6 +406,8 @@ int hibernate(void)
 	if (error)
 		goto Finish;
 
+	start_icebox();
+
 	if (hibernation_mode == HIBERNATION_TESTPROC) {
 		printk("swsusp debug: Waiting for 5 seconds.\n");
 		mdelay(5000);
@@ -427,6 +429,7 @@ int hibernate(void)
 		swsusp_free();
 	}
  Thaw:
+	stop_icebox();
 	unprepare_processes();
  Finish:
 	free_basic_memory_bitmaps();
@@ -504,6 +507,8 @@ static int software_resume(void)
 		goto Done;
 	}
 
+	start_icebox();
+
 	pr_debug("PM: Reading swsusp image.\n");
 
 	error = swsusp_read(&flags);
@@ -512,6 +517,7 @@ static int software_resume(void)
 
 	printk(KERN_ERR "PM: Restore failed, recovering.\n");
 	swsusp_free();
+	stop_icebox();
 	unprepare_processes();
  Done:
 	free_basic_memory_bitmaps();
Index: usb-2.6/kernel/power/user.c
===================================================================
--- usb-2.6.orig/kernel/power/user.c
+++ usb-2.6/kernel/power/user.c
@@ -172,13 +172,16 @@ static int snapshot_ioctl(struct inode *
 		if (error)
 			thaw_processes();
 		mutex_unlock(&pm_mutex);
-		if (!error)
+		if (!error) {
 			data->frozen = 1;
+			start_icebox();
+		}
 		break;
 
 	case SNAPSHOT_UNFREEZE:
 		if (!data->frozen || data->ready)
 			break;
+		stop_icebox();
 		mutex_lock(&pm_mutex);
 		thaw_processes();
 		mutex_unlock(&pm_mutex);
Index: usb-2.6/kernel/power/process.c
===================================================================
--- usb-2.6.orig/kernel/power/process.c
+++ usb-2.6/kernel/power/process.c
@@ -14,6 +14,8 @@
 #include <linux/syscalls.h>
 #include <linux/freezer.h>
 
+#include "power.h"
+
 /* 
  * Timeout for stopping processes
  */
@@ -283,3 +285,52 @@ void thaw_processes(void)
 }
 
 EXPORT_SYMBOL(refrigerator);
+
+/*
+ * Routines for kernel threads that want to freeze themselves in the icebox
+ */
+static DECLARE_WAIT_QUEUE_HEAD(icebox_wait_queue_head);
+static int icebox_active;
+
+/**
+ * start_icebox -- activate the icebox
+ *
+ * Kernel power code should call this routine before sending the
+ * PM_HIBERNATION_PREPARE, PM_SUSPEND_PREPARE, or PM_RESTORE_PREPARE
+ * notifications.
+ */
+void start_icebox(void)
+{
+	icebox_active = 1;
+}
+
+/**
+ * stop_icebox -- deactivate the icebox and awaken waiting threads
+ *
+ * Kernel power code should call this routine before sending the
+ * PM_POST_HIBERNATION, PM_POST_SUSPEND, or PM_POST_RESTORE notifications.
+ */
+void stop_icebox(void)
+{
+	icebox_active = 0;
+	wake_up_all(&icebox_wait_queue_head);
+}
+
+/**
+ * icebox -- place for kernel threads to wait during suspend or hibernation
+ *
+ * Tasks can call this routine at any time.  It will return immediately
+ * unless a system suspend or hibernation has started and the icebox is
+ * active, in which case it won't return until the suspend/hibernation
+ * is over.
+ *
+ * This routine should be used by drivers that want to delay user-initiated
+ * I/O until a system sleep is over.  Kernel threads should not use it
+ * in place of the freezer, because the icebox doesn't get activated until
+ * after freezing is complete.
+ */
+void icebox(void)
+{
+	wait_event(icebox_wait_queue_head, !icebox_active);
+}
+EXPORT_SYMBOL_GPL(icebox);

Re: [RFC] Add the "icebox"

[Mark Gross]

On Mon, Nov 05, 2007 at 04:27:47PM -0500, Alan Stern wrote:
> Rafael:
> 
> This patch adds the icebox, for use by kernel threads that want to 
> freeze themselves without using the freezer -- which is likely to crop 
> up when the freezer gets eliminated.

Why can't we have a runque per cpu that never gets scheduled and we simply
migrate the treads to those?  no more yield loops.

--mgross

> 
> It's straightforward enough.  It could be used as-is, for example for 
> freezing workqueue threads.  Using it for other sorts of kernel threads 
> seems to be rather difficult, requiring a fair amount of extra code 
> that's hard to centralize since each thread will have to be woken up 
> its own way.
> 
> Do you have any particular suggestions?
> 
> Alan Stern
> 
> 
> Index: usb-2.6/include/linux/freezer.h
> ===================================================================
> --- usb-2.6.orig/include/linux/freezer.h
> +++ usb-2.6/include/linux/freezer.h
> @@ -157,6 +157,12 @@ static inline void set_freezable(void)
>  	} while (try_to_freeze());					\
>  	__retval;							\
>  })
> +
> +/*
> + * Kernel threads that want to freeze themselves go into the icebox.
> + */
> +extern void icebox(void);
> +
>  #else /* !CONFIG_PM_SLEEP */
>  static inline int frozen(struct task_struct *p) { return 0; }
>  static inline int freezing(struct task_struct *p) { return 0; }
> @@ -181,6 +187,7 @@ static inline void set_freezable(void) {
>  #define wait_event_freezable_timeout(wq, condition, timeout)		\
>  		wait_event_interruptible_timeout(wq, condition, timeout)
>  
> +static inline void icebox(void) {}
>  #endif /* !CONFIG_PM_SLEEP */
>  
>  #endif	/* FREEZER_H_INCLUDED */
> Index: usb-2.6/kernel/power/power.h
> ===================================================================
> --- usb-2.6.orig/kernel/power/power.h
> +++ usb-2.6/kernel/power/power.h
> @@ -205,3 +205,8 @@ static inline int suspend_devices_and_en
>  
>  /* kernel/power/process.c */
>  extern int pm_notifier_call_chain(unsigned long val);
> +
> +#ifdef CONFIG_PM_SLEEP
> +extern void start_icebox(void);
> +extern void stop_icebox(void);
> +#endif
> Index: usb-2.6/kernel/power/main.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/main.c
> +++ usb-2.6/kernel/power/main.c
> @@ -71,6 +71,8 @@ static int suspend_prepare(void)
>  	if (!suspend_ops || !suspend_ops->enter)
>  		return -EPERM;
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_SUSPEND_PREPARE);
>  	if (error)
>  		goto Finish;
> @@ -98,6 +100,7 @@ static int suspend_prepare(void)
>  	thaw_processes();
>  	pm_restore_console();
>   Finish:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_SUSPEND);
>  	return error;
>  }
> @@ -191,6 +194,7 @@ static void suspend_finish(void)
>  {
>  	thaw_processes();
>  	pm_restore_console();
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_SUSPEND);
>  }
>  
> Index: usb-2.6/kernel/power/disk.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/disk.c
> +++ usb-2.6/kernel/power/disk.c
> @@ -389,6 +389,8 @@ int hibernate(void)
>  		goto Unlock;
>  	}
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
>  	if (error)
>  		goto Exit;
> @@ -431,6 +433,7 @@ int hibernate(void)
>   Finish:
>  	free_basic_memory_bitmaps();
>   Exit:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_HIBERNATION);
>  	atomic_inc(&snapshot_device_available);
>   Unlock:
> @@ -489,6 +492,8 @@ static int software_resume(void)
>  		goto Unlock;
>  	}
>  
> +	start_icebox();
> +
>  	error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
>  	if (error)
>  		goto Finish;
> @@ -516,6 +521,7 @@ static int software_resume(void)
>   Done:
>  	free_basic_memory_bitmaps();
>   Finish:
> +	stop_icebox();
>  	pm_notifier_call_chain(PM_POST_RESTORE);
>  	atomic_inc(&snapshot_device_available);
>  	/* For success case, the suspend path will release the lock */
> Index: usb-2.6/kernel/power/user.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/user.c
> +++ usb-2.6/kernel/power/user.c
> @@ -45,6 +45,7 @@ static int snapshot_open(struct inode *i
>  {
>  	struct snapshot_data *data;
>  	int error;
> +	unsigned long cancel;
>  
>  	if (!atomic_add_unless(&snapshot_device_available, -1, 0))
>  		return -EBUSY;
> @@ -61,21 +62,22 @@ static int snapshot_open(struct inode *i
>  	data = &snapshot_state;
>  	filp->private_data = data;
>  	memset(&data->handle, 0, sizeof(struct snapshot_handle));
> +	start_icebox();
>  	if ((filp->f_flags & O_ACCMODE) == O_RDONLY) {
>  		data->swap = swsusp_resume_device ?
>  			swap_type_of(swsusp_resume_device, 0, NULL) : -1;
>  		data->mode = O_RDONLY;
>  		error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
> -		if (error)
> -			pm_notifier_call_chain(PM_POST_RESTORE);
> +		cancel = PM_POST_RESTORE;
>  	} else {
>  		data->swap = -1;
>  		data->mode = O_WRONLY;
>  		error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
> -		if (error)
> -			pm_notifier_call_chain(PM_POST_HIBERNATION);
> +		cancel = PM_POST_HIBERNATION;
>  	}
>  	if (error) {
> +		stop_icebox();
> +		pm_notifier_call_chain(cancel);
>  		atomic_inc(&snapshot_device_available);
>  		return error;
>  	}
> @@ -99,6 +101,7 @@ static int snapshot_release(struct inode
>  		thaw_processes();
>  		mutex_unlock(&pm_mutex);
>  	}
> +	stop_icebox();
>  	pm_notifier_call_chain(data->mode == O_WRONLY ?
>  			PM_POST_HIBERNATION : PM_POST_RESTORE);
>  	atomic_inc(&snapshot_device_available);
> Index: usb-2.6/kernel/power/process.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/process.c
> +++ usb-2.6/kernel/power/process.c
> @@ -14,6 +14,8 @@
>  #include <linux/syscalls.h>
>  #include <linux/freezer.h>
>  
> +#include "power.h"
> +
>  /* 
>   * Timeout for stopping processes
>   */
> @@ -305,3 +307,49 @@ int pm_notifier_call_chain(unsigned long
>  	return (blocking_notifier_call_chain(&pm_chain_head, val, NULL)
>  			== NOTIFY_BAD) ? -EINVAL : 0;
>  }
> +
> +/*
> + * Routines for kernel threads that want to freeze themselves
> + */
> +static DECLARE_WAIT_QUEUE_HEAD(icebox_wait_queue_head);
> +static int icebox_active;
> +
> +/**
> + * start_icebox -- activate the icebox
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_HIBERNATION_PREPARE or PM_SUSPEND_PREPARE notifications.
> + */
> +void start_icebox(void)
> +{
> +	icebox_active = 1;
> +}
> +
> +/**
> + * stop_icebox -- deactivate the icebox and awaken waiting threads
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_POST_HIBERNATION or PM_POST_SUSPEND notifications.
> + */
> +void stop_icebox(void)
> +{
> +	icebox_active = 0;
> +	wake_up_all(&icebox_wait_queue_head);
> +}
> +
> +/**
> + * icebox -- place for kernel threads to wait during suspend or hibernation
> + *
> + * Threads can call this routine at any time.  It will return immediately
> + * unless a system suspend or hibernation has started and the icebox is
> + * active, in which case it won't return until the suspend/hibernation
> + * is over.
> + *
> + * Freezable kernel threads should use this routine rather than relying on
> + * the freezer.
> + */
> +void icebox(void)
> +{
> +	wait_event(icebox_wait_queue_head, !icebox_active);
> +}
> +EXPORT_SYMBOL_GPL(icebox);
> 
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [RFC] Add the "icebox"

[Alan Stern]

On Wed, 7 Nov 2007, Mark Gross wrote:

> On Mon, Nov 05, 2007 at 04:27:47PM -0500, Alan Stern wrote:
> > Rafael:
> > 
> > This patch adds the icebox, for use by kernel threads that want to 
> > freeze themselves without using the freezer -- which is likely to crop 
> > up when the freezer gets eliminated.
> 
> Why can't we have a runque per cpu that never gets scheduled and we simply
> migrate the treads to those?  no more yield loops.

Indeed.  Why stop there?  Why not eliminate wait_event() entirely, or
rather, re-implement it as you suggest?  :-)

Alan Stern

Re: [RFC] Add the "icebox"

[Alan Stern]

On Wed, 7 Nov 2007, Rafael J. Wysocki wrote:

> > P.S.: What about mutual exclusion in kernel/power/user.c?  If we stop 
> > freezing user tasks, there's nothing to prevent more than one task 
> > holding an open file reference to the snapshot file and submitting 
> > ioctls concurrently.  Maybe the regions covered by pm_mutex should be 
> > expanded.
> 
> /dev/snapshot can only be open once at a time.

What if a process opens /dev/snapshot and then forks a child?  Can't
the parent and child processes make concurrent ioctl calls?

Alan Stern

Re: [RFC] Add the "icebox"

[Rafael J. Wysocki]

On Wednesday, 7 of November 2007, Alan Stern wrote:
> On Mon, 5 Nov 2007, Rafael J. Wysocki wrote:
> 
> > I like the idea, but I think that start_icebox() should be called after
> > freeze_processes().  Similarly, IMO it's better to call stop_icebox() before
> > thaw_processes().
> 
> Okay.  On the assumption that freezable kernel threads will continue to 
> use the freezer and won't try to use the icebox, I have updated the 
> patch.  Other comments?

OK by me.

> Alan Stern
> 
> P.S.: What about mutual exclusion in kernel/power/user.c?  If we stop 
> freezing user tasks, there's nothing to prevent more than one task 
> holding an open file reference to the snapshot file and submitting 
> ioctls concurrently.  Maybe the regions covered by pm_mutex should be 
> expanded.

/dev/snapshot can only be open once at a time.

Greetings,
Rafael


> Index: usb-2.6/include/linux/freezer.h
> ===================================================================
> --- usb-2.6.orig/include/linux/freezer.h
> +++ usb-2.6/include/linux/freezer.h
> @@ -157,6 +157,12 @@ static inline void set_freezable(void)
>  	} while (try_to_freeze());					\
>  	__retval;							\
>  })
> +
> +/*
> + * Kernel threads that want to freeze themselves go into the icebox.
> + */
> +extern void icebox(void);
> +
>  #else /* !CONFIG_PM_SLEEP */
>  static inline int frozen(struct task_struct *p) { return 0; }
>  static inline int freezing(struct task_struct *p) { return 0; }
> @@ -181,6 +187,7 @@ static inline void set_freezable(void) {
>  #define wait_event_freezable_timeout(wq, condition, timeout)		\
>  		wait_event_interruptible_timeout(wq, condition, timeout)
>  
> +static inline void icebox(void) {}
>  #endif /* !CONFIG_PM_SLEEP */
>  
>  #endif	/* FREEZER_H_INCLUDED */
> Index: usb-2.6/kernel/power/power.h
> ===================================================================
> --- usb-2.6.orig/kernel/power/power.h
> +++ usb-2.6/kernel/power/power.h
> @@ -207,3 +207,9 @@ static inline int suspend_devices_and_en
>  /* kernel/power/main.c */
>  extern int pm_notifier_call_chain(unsigned long val);
>  #endif
> +
> +#ifdef CONFIG_PM_SLEEP
> +/* kerne/power/process.c */
> +extern void start_icebox(void);
> +extern void stop_icebox(void);
> +#endif
> Index: usb-2.6/kernel/power/main.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/main.c
> +++ usb-2.6/kernel/power/main.c
> @@ -108,6 +108,8 @@ static int suspend_prepare(void)
>  		goto Thaw;
>  	}
>  
> +	start_icebox();
> +
>  	free_pages = global_page_state(NR_FREE_PAGES);
>  	if (free_pages < FREE_PAGE_NUMBER) {
>  		pr_debug("PM: free some memory\n");
> @@ -120,6 +122,7 @@ static int suspend_prepare(void)
>  	if (!error)
>  		return 0;
>  
> +	stop_icebox();
>   Thaw:
>  	thaw_processes();
>  	pm_restore_console();
> @@ -215,6 +218,7 @@ int suspend_devices_and_enter(suspend_st
>   */
>  static void suspend_finish(void)
>  {
> +	stop_icebox();
>  	thaw_processes();
>  	pm_restore_console();
>  	pm_notifier_call_chain(PM_POST_SUSPEND);
> Index: usb-2.6/kernel/power/disk.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/disk.c
> +++ usb-2.6/kernel/power/disk.c
> @@ -406,6 +406,8 @@ int hibernate(void)
>  	if (error)
>  		goto Finish;
>  
> +	start_icebox();
> +
>  	if (hibernation_mode == HIBERNATION_TESTPROC) {
>  		printk("swsusp debug: Waiting for 5 seconds.\n");
>  		mdelay(5000);
> @@ -427,6 +429,7 @@ int hibernate(void)
>  		swsusp_free();
>  	}
>   Thaw:
> +	stop_icebox();
>  	unprepare_processes();
>   Finish:
>  	free_basic_memory_bitmaps();
> @@ -504,6 +507,8 @@ static int software_resume(void)
>  		goto Done;
>  	}
>  
> +	start_icebox();
> +
>  	pr_debug("PM: Reading swsusp image.\n");
>  
>  	error = swsusp_read(&flags);
> @@ -512,6 +517,7 @@ static int software_resume(void)
>  
>  	printk(KERN_ERR "PM: Restore failed, recovering.\n");
>  	swsusp_free();
> +	stop_icebox();
>  	unprepare_processes();
>   Done:
>  	free_basic_memory_bitmaps();
> Index: usb-2.6/kernel/power/user.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/user.c
> +++ usb-2.6/kernel/power/user.c
> @@ -172,13 +172,16 @@ static int snapshot_ioctl(struct inode *
>  		if (error)
>  			thaw_processes();
>  		mutex_unlock(&pm_mutex);
> -		if (!error)
> +		if (!error) {
>  			data->frozen = 1;
> +			start_icebox();
> +		}
>  		break;
>  
>  	case SNAPSHOT_UNFREEZE:
>  		if (!data->frozen || data->ready)
>  			break;
> +		stop_icebox();
>  		mutex_lock(&pm_mutex);
>  		thaw_processes();
>  		mutex_unlock(&pm_mutex);
> Index: usb-2.6/kernel/power/process.c
> ===================================================================
> --- usb-2.6.orig/kernel/power/process.c
> +++ usb-2.6/kernel/power/process.c
> @@ -14,6 +14,8 @@
>  #include <linux/syscalls.h>
>  #include <linux/freezer.h>
>  
> +#include "power.h"
> +
>  /* 
>   * Timeout for stopping processes
>   */
> @@ -283,3 +285,52 @@ void thaw_processes(void)
>  }
>  
>  EXPORT_SYMBOL(refrigerator);
> +
> +/*
> + * Routines for kernel threads that want to freeze themselves in the icebox
> + */
> +static DECLARE_WAIT_QUEUE_HEAD(icebox_wait_queue_head);
> +static int icebox_active;
> +
> +/**
> + * start_icebox -- activate the icebox
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_HIBERNATION_PREPARE, PM_SUSPEND_PREPARE, or PM_RESTORE_PREPARE
> + * notifications.
> + */
> +void start_icebox(void)
> +{
> +	icebox_active = 1;
> +}
> +
> +/**
> + * stop_icebox -- deactivate the icebox and awaken waiting threads
> + *
> + * Kernel power code should call this routine before sending the
> + * PM_POST_HIBERNATION, PM_POST_SUSPEND, or PM_POST_RESTORE notifications.
> + */
> +void stop_icebox(void)
> +{
> +	icebox_active = 0;
> +	wake_up_all(&icebox_wait_queue_head);
> +}
> +
> +/**
> + * icebox -- place for kernel threads to wait during suspend or hibernation
> + *
> + * Tasks can call this routine at any time.  It will return immediately
> + * unless a system suspend or hibernation has started and the icebox is
> + * active, in which case it won't return until the suspend/hibernation
> + * is over.
> + *
> + * This routine should be used by drivers that want to delay user-initiated
> + * I/O until a system sleep is over.  Kernel threads should not use it
> + * in place of the freezer, because the icebox doesn't get activated until
> + * after freezing is complete.
> + */
> +void icebox(void)
> +{
> +	wait_event(icebox_wait_queue_head, !icebox_active);
> +}
> +EXPORT_SYMBOL_GPL(icebox);
> 

Re: [RFC] Add the "icebox"

[Rafael J. Wysocki]

On Wednesday, 7 of November 2007, Alan Stern wrote:
> On Wed, 7 Nov 2007, Rafael J. Wysocki wrote:
> 
> > > P.S.: What about mutual exclusion in kernel/power/user.c?  If we stop 
> > > freezing user tasks, there's nothing to prevent more than one task 
> > > holding an open file reference to the snapshot file and submitting 
> > > ioctls concurrently.  Maybe the regions covered by pm_mutex should be 
> > > expanded.
> > 
> > /dev/snapshot can only be open once at a time.
> 
> What if a process opens /dev/snapshot and then forks a child?  Can't
> the parent and child processes make concurrent ioctl calls?

Well, that would be shooting yourself in a foot.

You can do many stupid things as root and this one is probably not the worst of
them after all. ;-)

Greetings,
Rafael