linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
@ 2012-01-28 13:45 Rafael J. Wysocki
  2012-01-28 21:23 ` Nigel Cunningham
                   ` (6 more replies)
  0 siblings, 7 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-01-28 13:45 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Jan Kara, linux-fsdevel, Dave Chinner, Nigel Cunningham,
	Pavel Machek, Srivatsa S. Bhat

From: Rafael J. Wysocki <rjw@sisk.pl>

Freeze all filesystems during system suspend and (kernel-driven)
hibernation by calling freeze_supers() for all superblocks and thaw
them during the subsequent resume with the help of thaw_supers().

This makes filesystems stay in a consistent state in case something
goes wrong between system suspend (or hibernation) and the subsequent
resume (e.g. journal replays won't be necessary in those cases).  In
particular, this should help to solve a long-standing issue that, in
some cases, during resume from hibernation the boot loader causes the
journal to be replied for the filesystem containing the kernel image
and/or initrd causing it to become inconsistent with the information
stored in the hibernation image.

The user-space-driven hibernation (s2disk) is not covered by this
change, because the freezing of filesystems prevents s2disk from
accessing device special files it needs to do its job.

This change is based on earlier work by Nigel Cunningham.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h       |    3 +
 kernel/power/hibernate.c |   11 +++++--
 kernel/power/power.h     |   23 --------------
 kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 24 deletions(-)

Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h
+++ linux/include/linux/fs.h
@@ -210,6 +210,7 @@ struct inodes_stat_t {
 #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
 #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
 #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
+#define MS_FROZEN	(1<<25) /* Frozen filesystem */
 #define MS_NOSEC	(1<<28)
 #define MS_BORN		(1<<29)
 #define MS_ACTIVE	(1<<30)
@@ -2501,6 +2502,8 @@ extern void drop_super(struct super_bloc
 extern void iterate_supers(void (*)(struct super_block *, void *), void *);
 extern void iterate_supers_type(struct file_system_type *,
 			        void (*)(struct super_block *, void *), void *);
+extern int freeze_supers(void);
+extern void thaw_supers(void);
 
 extern int dcache_dir_open(struct inode *, struct file *);
 extern int dcache_dir_close(struct inode *, struct file *);
Index: linux/fs/super.c
===================================================================
--- linux.orig/fs/super.c
+++ linux/fs/super.c
@@ -594,6 +594,79 @@ void iterate_supers_type(struct file_sys
 EXPORT_SYMBOL(iterate_supers_type);
 
 /**
+ *	thaw_supers - call thaw_super() for all superblocks
+ */
+void thaw_supers(void)
+{
+	struct super_block *sb, *p = NULL;
+
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		if (list_empty(&sb->s_instances))
+			continue;
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+
+		if (sb->s_flags & MS_FROZEN) {
+			thaw_super(sb);
+			sb->s_flags &= ~MS_FROZEN;
+		}
+
+		spin_lock(&sb_lock);
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+}
+
+/**
+ *	freeze_supers - call freeze_super() for all superblocks
+ */
+int freeze_supers(void)
+{
+	struct super_block *sb, *p = NULL;
+	int error = 0;
+
+	spin_lock(&sb_lock);
+	/*
+	 * Freeze in reverse order so filesystems depending on others are
+	 * frozen in the right order (eg. loopback on ext3).
+	 */
+	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
+		if (list_empty(&sb->s_instances))
+			continue;
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+
+		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
+		    && !(sb->s_flags & MS_RDONLY)) {
+			error = freeze_super(sb);
+			if (!error)
+				sb->s_flags |= MS_FROZEN;
+		}
+
+		spin_lock(&sb_lock);
+		if (error)
+			break;
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+
+	if (error)
+		thaw_supers();
+
+	return error;
+}
+
+
+/**
  *	get_super - get the superblock of a device
  *	@bdev: device to get the superblock for
  *	
Index: linux/kernel/power/power.h
===================================================================
--- linux.orig/kernel/power/power.h
+++ linux/kernel/power/power.h
@@ -1,3 +1,4 @@
+#include <linux/fs.h>
 #include <linux/suspend.h>
 #include <linux/suspend_ioctls.h>
 #include <linux/utsname.h>
@@ -227,25 +228,3 @@ enum {
 #define TEST_MAX	(__TEST_AFTER_LAST - 1)
 
 extern int pm_test_level;
-
-#ifdef CONFIG_SUSPEND_FREEZER
-static inline int suspend_freeze_processes(void)
-{
-	int error = freeze_processes();
-	return error ? : freeze_kernel_threads();
-}
-
-static inline void suspend_thaw_processes(void)
-{
-	thaw_processes();
-}
-#else
-static inline int suspend_freeze_processes(void)
-{
-	return 0;
-}
-
-static inline void suspend_thaw_processes(void)
-{
-}
-#endif
Index: linux/kernel/power/suspend.c
===================================================================
--- linux.orig/kernel/power/suspend.c
+++ linux/kernel/power/suspend.c
@@ -29,6 +29,48 @@
 
 #include "power.h"
 
+#ifdef CONFIG_SUSPEND_FREEZER
+
+static inline int suspend_freeze_processes(void)
+{
+	int error;
+
+	error = freeze_processes();
+	if (error)
+		return error;
+
+	error = freeze_supers();
+	if (error) {
+		thaw_processes();
+		return error;
+	}
+
+	error = freeze_kernel_threads();
+	if (error)
+		thaw_supers();
+
+	return error;
+}
+
+static inline void suspend_thaw_processes(void)
+{
+	thaw_supers();
+	thaw_processes();
+}
+
+#else /* !CONFIG_SUSPEND_FREEZER */
+
+static inline int suspend_freeze_processes(void)
+{
+	return 0;
+}
+
+static inline void suspend_thaw_processes(void)
+{
+}
+
+#endif /* !CONFIG_SUSPEND_FREEZER */
+
 const char *const pm_states[PM_SUSPEND_MAX] = {
 	[PM_SUSPEND_STANDBY]	= "standby",
 	[PM_SUSPEND_MEM]	= "mem",
Index: linux/kernel/power/hibernate.c
===================================================================
--- linux.orig/kernel/power/hibernate.c
+++ linux/kernel/power/hibernate.c
@@ -626,12 +626,17 @@ int hibernate(void)
 	if (error)
 		goto Finish;
 
-	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
+	error = freeze_supers();
 	if (error)
 		goto Thaw;
+
+	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
+	if (error)
+		goto Thaw_fs;
+
 	if (freezer_test_done) {
 		freezer_test_done = false;
-		goto Thaw;
+		goto Thaw_fs;
 	}
 
 	if (in_suspend) {
@@ -655,6 +660,8 @@ int hibernate(void)
 		pr_debug("PM: Image restored successfully.\n");
 	}
 
+ Thaw_fs:
+	thaw_supers();
  Thaw:
 	thaw_processes();
  Finish:

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
@ 2012-01-28 21:23 ` Nigel Cunningham
  2012-01-29 15:55 ` Martin Steigerwald
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 37+ messages in thread
From: Nigel Cunningham @ 2012-01-28 21:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Pavel Machek, Srivatsa S. Bhat

Hi.

On 29/01/12 00:45, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image
> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.
> 
> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.
> 
> This change is based on earlier work by Nigel Cunningham.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h       |    3 +
>  kernel/power/hibernate.c |   11 +++++--
>  kernel/power/power.h     |   23 --------------
>  kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
>  5 files changed, 128 insertions(+), 24 deletions(-)
> 
> Index: linux/include/linux/fs.h
> ===================================================================
> --- linux.orig/include/linux/fs.h
> +++ linux/include/linux/fs.h
> @@ -210,6 +210,7 @@ struct inodes_stat_t {
>  #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
>  #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
>  #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
> +#define MS_FROZEN	(1<<25) /* Frozen filesystem */
>  #define MS_NOSEC	(1<<28)
>  #define MS_BORN		(1<<29)
>  #define MS_ACTIVE	(1<<30)
> @@ -2501,6 +2502,8 @@ extern void drop_super(struct super_bloc
>  extern void iterate_supers(void (*)(struct super_block *, void *), void *);
>  extern void iterate_supers_type(struct file_system_type *,
>  			        void (*)(struct super_block *, void *), void *);
> +extern int freeze_supers(void);
> +extern void thaw_supers(void);
>  
>  extern int dcache_dir_open(struct inode *, struct file *);
>  extern int dcache_dir_close(struct inode *, struct file *);
> Index: linux/fs/super.c
> ===================================================================
> --- linux.orig/fs/super.c
> +++ linux/fs/super.c
> @@ -594,6 +594,79 @@ void iterate_supers_type(struct file_sys
>  EXPORT_SYMBOL(iterate_supers_type);
>  
>  /**
> + *	thaw_supers - call thaw_super() for all superblocks
> + */
> +void thaw_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +
> +	spin_lock(&sb_lock);
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		if (list_empty(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_flags & MS_FROZEN) {
> +			thaw_super(sb);
> +			sb->s_flags &= ~MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +}

It might be helpful to explain why you delay calling __put_super (I
can't see why, though I realise that fs/super.c does it this way too),
and perhaps also where the balancing get is (it's not obvious that
s->count++ is the open coded get).

> +
> +/**
> + *	freeze_supers - call freeze_super() for all superblocks
> + */
> +int freeze_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +	int error = 0;
> +
> +	spin_lock(&sb_lock);
> +	/*
> +	 * Freeze in reverse order so filesystems depending on others are
> +	 * frozen in the right order (eg. loopback on ext3).
> +	 */
> +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> +		if (list_empty(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
> +		    && !(sb->s_flags & MS_RDONLY)) {
> +			error = freeze_super(sb);

No chance of a race against another caller of freeze_super, right?
(Userspace is already frozen and kernel threads aren't going to invoke
this without pushing). Might be good to document that there's no locking
in freeze_super anyway?

> +			if (!error)
> +				sb->s_flags |= MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (error)
> +			break;
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +
> +	if (error)
> +		thaw_supers();
> +
> +	return error;
> +}
> +
> +
> +/**
>   *	get_super - get the superblock of a device
>   *	@bdev: device to get the superblock for
>   *	
> Index: linux/kernel/power/power.h
> ===================================================================
> --- linux.orig/kernel/power/power.h
> +++ linux/kernel/power/power.h
> @@ -1,3 +1,4 @@
> +#include <linux/fs.h>
>  #include <linux/suspend.h>
>  #include <linux/suspend_ioctls.h>
>  #include <linux/utsname.h>
> @@ -227,25 +228,3 @@ enum {
>  #define TEST_MAX	(__TEST_AFTER_LAST - 1)
>  
>  extern int pm_test_level;
> -
> -#ifdef CONFIG_SUSPEND_FREEZER
> -static inline int suspend_freeze_processes(void)
> -{
> -	int error = freeze_processes();
> -	return error ? : freeze_kernel_threads();
> -}
> -
> -static inline void suspend_thaw_processes(void)
> -{
> -	thaw_processes();
> -}
> -#else
> -static inline int suspend_freeze_processes(void)
> -{
> -	return 0;
> -}
> -
> -static inline void suspend_thaw_processes(void)
> -{
> -}
> -#endif
> Index: linux/kernel/power/suspend.c
> ===================================================================
> --- linux.orig/kernel/power/suspend.c
> +++ linux/kernel/power/suspend.c
> @@ -29,6 +29,48 @@
>  
>  #include "power.h"
>  
> +#ifdef CONFIG_SUSPEND_FREEZER
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	int error;
> +
> +	error = freeze_processes();
> +	if (error)
> +		return error;
> +
> +	error = freeze_supers();
> +	if (error) {
> +		thaw_processes();
> +		return error;
> +	}
> +
> +	error = freeze_kernel_threads();
> +	if (error)
> +		thaw_supers();

Comment why there's no matching thaw_processes() as well?

That's all from me.

Regards,

Nigel

> +
> +	return error;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +	thaw_supers();
> +	thaw_processes();
> +}
> +
> +#else /* !CONFIG_SUSPEND_FREEZER */
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	return 0;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +}
> +
> +#endif /* !CONFIG_SUSPEND_FREEZER */
> +
>  const char *const pm_states[PM_SUSPEND_MAX] = {
>  	[PM_SUSPEND_STANDBY]	= "standby",
>  	[PM_SUSPEND_MEM]	= "mem",
> Index: linux/kernel/power/hibernate.c
> ===================================================================
> --- linux.orig/kernel/power/hibernate.c
> +++ linux/kernel/power/hibernate.c
> @@ -626,12 +626,17 @@ int hibernate(void)
>  	if (error)
>  		goto Finish;
>  
> -	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	error = freeze_supers();
>  	if (error)
>  		goto Thaw;
> +
> +	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	if (error)
> +		goto Thaw_fs;
> +
>  	if (freezer_test_done) {
>  		freezer_test_done = false;
> -		goto Thaw;
> +		goto Thaw_fs;
>  	}
>  
>  	if (in_suspend) {
> @@ -655,6 +660,8 @@ int hibernate(void)
>  		pr_debug("PM: Image restored successfully.\n");
>  	}
>  
> + Thaw_fs:
> +	thaw_supers();
>   Thaw:
>  	thaw_processes();
>   Finish:
> 

-- 
Evolution (n): A hypothetical process whereby improbable
events occur with alarming frequency, order arises from chaos, and
no one is given credit.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
  2012-01-28 21:23 ` Nigel Cunningham
@ 2012-01-29 15:55 ` Martin Steigerwald
  2012-01-29 19:53   ` Rafael J. Wysocki
  2012-01-29 16:28 ` Srivatsa S. Bhat
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 37+ messages in thread
From: Martin Steigerwald @ 2012-01-29 15:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

Am Samstag, 28. Januar 2012 schrieb Rafael J. Wysocki:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image

replied => replayed

> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.

Thanks,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
  2012-01-28 21:23 ` Nigel Cunningham
  2012-01-29 15:55 ` Martin Steigerwald
@ 2012-01-29 16:28 ` Srivatsa S. Bhat
  2012-01-29 19:53   ` Rafael J. Wysocki
  2012-01-30 20:00 ` Jan Kara
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 37+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-29 16:28 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Martin

On 01/28/2012 07:15 PM, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image

> and/or initrd causing it to become inconsistent with the information

> stored in the hibernation image.
> 
> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.
> 
> This change is based on earlier work by Nigel Cunningham.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h       |    3 +
>  kernel/power/hibernate.c |   11 +++++--
>  kernel/power/power.h     |   23 --------------
>  kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
>  5 files changed, 128 insertions(+), 24 deletions(-)
> 

...

> Index: linux/kernel/power/suspend.c
> ===================================================================
> --- linux.orig/kernel/power/suspend.c
> +++ linux/kernel/power/suspend.c
> @@ -29,6 +29,48 @@
> 
>  #include "power.h"
> 
> +#ifdef CONFIG_SUSPEND_FREEZER
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	int error;
> +
> +	error = freeze_processes();
> +	if (error)
> +		return error;
> +
> +	error = freeze_supers();
> +	if (error) {
> +		thaw_processes();
> +		return error;
> +	}
> +
> +	error = freeze_kernel_threads();
> +	if (error)
> +		thaw_supers();
> +


If freezing of kernel threads fails, freeze_kernel_threads() calls
thaw_processes(), which means, even userspace processes get thawed.
So, there would be a time-window in which userspace is thawed but the
filesystems are still frozen. That is not very desirable right?

If that is right, then modifying freeze_kernel_threads() to call
thaw_kernel_threads() instead of thaw_processes() would fix it
(and of course, we would need to explicitly call thaw_processes
above).

BTW, after your patch posted at https://lkml.org/lkml/2012/1/27/501,
I very much wanted to write a patch to convert the semantics of
freeze/thaw to something like:

freeze_processes() calls thaw_processes on error.
//Both touch only userspace processes.

freeze_kernel_threads() calls thaw_kernel_threads() on error.
//Both touch only kernel threads.

Of course, such a patch would need to do a lot of fixing up at several
places, but IMHO, it would really help make the overall code more logical
and easier to understand.

I can write it up and post it soon, but then you'll have to rebase
your patch (this one) on top of that. What do you say?

Regards,
Srivatsa S. Bhat

> +	return error;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +	thaw_supers();
> +	thaw_processes();
> +}
> +
> +#else /* !CONFIG_SUSPEND_FREEZER */
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	return 0;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +}
> +
> +#endif /* !CONFIG_SUSPEND_FREEZER */
> +
>  const char *const pm_states[PM_SUSPEND_MAX] = {
>  	[PM_SUSPEND_STANDBY]	= "standby",
>  	[PM_SUSPEND_MEM]	= "mem",
> Index: linux/kernel/power/hibernate.c
> ===================================================================
> --- linux.orig/kernel/power/hibernate.c
> +++ linux/kernel/power/hibernate.c
> @@ -626,12 +626,17 @@ int hibernate(void)
>  	if (error)
>  		goto Finish;
> 
> -	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	error = freeze_supers();
>  	if (error)
>  		goto Thaw;
> +
> +	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	if (error)
> +		goto Thaw_fs;
> +
>  	if (freezer_test_done) {
>  		freezer_test_done = false;
> -		goto Thaw;
> +		goto Thaw_fs;
>  	}
> 
>  	if (in_suspend) {
> @@ -655,6 +660,8 @@ int hibernate(void)
>  		pr_debug("PM: Image restored successfully.\n");
>  	}
> 
> + Thaw_fs:
> +	thaw_supers();
>   Thaw:
>  	thaw_processes();
>   Finish:
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-29 16:28 ` Srivatsa S. Bhat
@ 2012-01-29 19:53   ` Rafael J. Wysocki
  2012-01-30 23:24     ` Srivatsa S. Bhat
  0 siblings, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-01-29 19:53 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Martin

On Sunday, January 29, 2012, Srivatsa S. Bhat wrote:
> On 01/28/2012 07:15 PM, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Freeze all filesystems during system suspend and (kernel-driven)
> > hibernation by calling freeze_supers() for all superblocks and thaw
> > them during the subsequent resume with the help of thaw_supers().
> > 
> > This makes filesystems stay in a consistent state in case something
> > goes wrong between system suspend (or hibernation) and the subsequent
> > resume (e.g. journal replays won't be necessary in those cases).  In
> > particular, this should help to solve a long-standing issue that, in
> > some cases, during resume from hibernation the boot loader causes the
> > journal to be replied for the filesystem containing the kernel image
> 
> > and/or initrd causing it to become inconsistent with the information
> 
> > stored in the hibernation image.
> > 
> > The user-space-driven hibernation (s2disk) is not covered by this
> > change, because the freezing of filesystems prevents s2disk from
> > accessing device special files it needs to do its job.
> > 
> > This change is based on earlier work by Nigel Cunningham.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/fs.h       |    3 +
> >  kernel/power/hibernate.c |   11 +++++--
> >  kernel/power/power.h     |   23 --------------
> >  kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
> >  5 files changed, 128 insertions(+), 24 deletions(-)
> > 
> 
> ...
> 
> > Index: linux/kernel/power/suspend.c
> > ===================================================================
> > --- linux.orig/kernel/power/suspend.c
> > +++ linux/kernel/power/suspend.c
> > @@ -29,6 +29,48 @@
> > 
> >  #include "power.h"
> > 
> > +#ifdef CONFIG_SUSPEND_FREEZER
> > +
> > +static inline int suspend_freeze_processes(void)
> > +{
> > +	int error;
> > +
> > +	error = freeze_processes();
> > +	if (error)
> > +		return error;
> > +
> > +	error = freeze_supers();
> > +	if (error) {
> > +		thaw_processes();
> > +		return error;
> > +	}
> > +
> > +	error = freeze_kernel_threads();
> > +	if (error)
> > +		thaw_supers();
> > +
> 
> 
> If freezing of kernel threads fails, freeze_kernel_threads() calls
> thaw_processes(), which means, even userspace processes get thawed.
> So, there would be a time-window in which userspace is thawed but the
> filesystems are still frozen. That is not very desirable right?

No, it is not.  I overlooked that, thanks!

> If that is right, then modifying freeze_kernel_threads() to call
> thaw_kernel_threads() instead of thaw_processes() would fix it
> (and of course, we would need to explicitly call thaw_processes
> above).
> 
> BTW, after your patch posted at https://lkml.org/lkml/2012/1/27/501,
> I very much wanted to write a patch to convert the semantics of
> freeze/thaw to something like:
> 
> freeze_processes() calls thaw_processes on error.
> //Both touch only userspace processes.
> 
> freeze_kernel_threads() calls thaw_kernel_threads() on error.
> //Both touch only kernel threads.
> 
> Of course, such a patch would need to do a lot of fixing up at several
> places, but IMHO, it would really help make the overall code more logical
> and easier to understand.
> 
> I can write it up and post it soon, but then you'll have to rebase
> your patch (this one) on top of that. What do you say?

Please do that, it wouldn't be any problem for me to rebase the $subject
patch.

Thanks,
Rafael


> > +	return error;
> > +}
> > +
> > +static inline void suspend_thaw_processes(void)
> > +{
> > +	thaw_supers();
> > +	thaw_processes();
> > +}
> > +
> > +#else /* !CONFIG_SUSPEND_FREEZER */
> > +
> > +static inline int suspend_freeze_processes(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void suspend_thaw_processes(void)
> > +{
> > +}
> > +
> > +#endif /* !CONFIG_SUSPEND_FREEZER */
> > +
> >  const char *const pm_states[PM_SUSPEND_MAX] = {
> >  	[PM_SUSPEND_STANDBY]	= "standby",
> >  	[PM_SUSPEND_MEM]	= "mem",
> > Index: linux/kernel/power/hibernate.c
> > ===================================================================
> > --- linux.orig/kernel/power/hibernate.c
> > +++ linux/kernel/power/hibernate.c
> > @@ -626,12 +626,17 @@ int hibernate(void)
> >  	if (error)
> >  		goto Finish;
> > 
> > -	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> > +	error = freeze_supers();
> >  	if (error)
> >  		goto Thaw;
> > +
> > +	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> > +	if (error)
> > +		goto Thaw_fs;
> > +
> >  	if (freezer_test_done) {
> >  		freezer_test_done = false;
> > -		goto Thaw;
> > +		goto Thaw_fs;
> >  	}
> > 
> >  	if (in_suspend) {
> > @@ -655,6 +660,8 @@ int hibernate(void)
> >  		pr_debug("PM: Image restored successfully.\n");
> >  	}
> > 
> > + Thaw_fs:
> > +	thaw_supers();
> >   Thaw:
> >  	thaw_processes();
> >   Finish:
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-29 15:55 ` Martin Steigerwald
@ 2012-01-29 19:53   ` Rafael J. Wysocki
  0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-01-29 19:53 UTC (permalink / raw)
  To: Martin Steigerwald
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Sunday, January 29, 2012, Martin Steigerwald wrote:
> Am Samstag, 28. Januar 2012 schrieb Rafael J. Wysocki:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Freeze all filesystems during system suspend and (kernel-driven)
> > hibernation by calling freeze_supers() for all superblocks and thaw
> > them during the subsequent resume with the help of thaw_supers().
> > 
> > This makes filesystems stay in a consistent state in case something
> > goes wrong between system suspend (or hibernation) and the subsequent
> > resume (e.g. journal replays won't be necessary in those cases).  In
> > particular, this should help to solve a long-standing issue that, in
> > some cases, during resume from hibernation the boot loader causes the
> > journal to be replied for the filesystem containing the kernel image
> 
> replied => replayed

Right, thanks!

Rafael

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2012-01-29 16:28 ` Srivatsa S. Bhat
@ 2012-01-30 20:00 ` Jan Kara
  2012-01-30 21:05   ` Rafael J. Wysocki
  2012-02-01 13:36 ` Pavel Machek
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 37+ messages in thread
From: Jan Kara @ 2012-01-30 20:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat, Al Viro

On Sat 28-01-12 14:45:49, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image
> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.
> 
> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.
> 
> This change is based on earlier work by Nigel Cunningham.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h       |    3 +
>  kernel/power/hibernate.c |   11 +++++--
>  kernel/power/power.h     |   23 --------------
>  kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
>  5 files changed, 128 insertions(+), 24 deletions(-)
> 
> Index: linux/fs/super.c
> ===================================================================
> --- linux.orig/fs/super.c
> +++ linux/fs/super.c
> @@ -594,6 +594,79 @@ void iterate_supers_type(struct file_sys
>  EXPORT_SYMBOL(iterate_supers_type);
>  
>  /**
> + *	thaw_supers - call thaw_super() for all superblocks
> + */
> +void thaw_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +
> +	spin_lock(&sb_lock);
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		if (list_empty(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_flags & MS_FROZEN) {
> +			thaw_super(sb);
> +			sb->s_flags &= ~MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +}
  At least for thawing you can use iterate_supers() helper.

> +/**
> + *	freeze_supers - call freeze_super() for all superblocks
> + */
> +int freeze_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +	int error = 0;
> +
> +	spin_lock(&sb_lock);
> +	/*
> +	 * Freeze in reverse order so filesystems depending on others are
> +	 * frozen in the right order (eg. loopback on ext3).
> +	 */
  Ho, hum, are you sure the order in super_blocks list is the one you need?
Maybe it is but I'm not sure you are guaranteed it is. I've added Al to CC,
he'll likely have opinion on this...

> +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> +		if (list_empty(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
> +		    && !(sb->s_flags & MS_RDONLY)) {
> +			error = freeze_super(sb);
> +			if (!error)
> +				sb->s_flags |= MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (error)
> +			break;
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +
> +	if (error)
> +		thaw_supers();
> +
> +	return error;
> +}
> +
> +
> +/**
>   *	get_super - get the superblock of a device
>   *	@bdev: device to get the superblock for
>   *	

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-30 20:00 ` Jan Kara
@ 2012-01-30 21:05   ` Rafael J. Wysocki
  2012-01-30 21:10     ` Nigel Cunningham
  2012-01-30 23:58     ` Jan Kara
  0 siblings, 2 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-01-30 21:05 UTC (permalink / raw)
  To: Jan Kara
  Cc: Linux PM list, LKML, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat, Al Viro

On Monday, January 30, 2012, Jan Kara wrote:
> On Sat 28-01-12 14:45:49, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Freeze all filesystems during system suspend and (kernel-driven)
> > hibernation by calling freeze_supers() for all superblocks and thaw
> > them during the subsequent resume with the help of thaw_supers().
> > 
> > This makes filesystems stay in a consistent state in case something
> > goes wrong between system suspend (or hibernation) and the subsequent
> > resume (e.g. journal replays won't be necessary in those cases).  In
> > particular, this should help to solve a long-standing issue that, in
> > some cases, during resume from hibernation the boot loader causes the
> > journal to be replied for the filesystem containing the kernel image
> > and/or initrd causing it to become inconsistent with the information
> > stored in the hibernation image.
> > 
> > The user-space-driven hibernation (s2disk) is not covered by this
> > change, because the freezing of filesystems prevents s2disk from
> > accessing device special files it needs to do its job.
> > 
> > This change is based on earlier work by Nigel Cunningham.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  fs/super.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/fs.h       |    3 +
> >  kernel/power/hibernate.c |   11 +++++--
> >  kernel/power/power.h     |   23 --------------
> >  kernel/power/suspend.c   |   42 +++++++++++++++++++++++++++
> >  5 files changed, 128 insertions(+), 24 deletions(-)
> > 
> > Index: linux/fs/super.c
> > ===================================================================
> > --- linux.orig/fs/super.c
> > +++ linux/fs/super.c
> > @@ -594,6 +594,79 @@ void iterate_supers_type(struct file_sys
> >  EXPORT_SYMBOL(iterate_supers_type);
> >  
> >  /**
> > + *	thaw_supers - call thaw_super() for all superblocks
> > + */
> > +void thaw_supers(void)
> > +{
> > +	struct super_block *sb, *p = NULL;
> > +
> > +	spin_lock(&sb_lock);
> > +	list_for_each_entry(sb, &super_blocks, s_list) {
> > +		if (list_empty(&sb->s_instances))
> > +			continue;
> > +		sb->s_count++;
> > +		spin_unlock(&sb_lock);
> > +
> > +		if (sb->s_flags & MS_FROZEN) {
> > +			thaw_super(sb);
> > +			sb->s_flags &= ~MS_FROZEN;
> > +		}
> > +
> > +		spin_lock(&sb_lock);
> > +		if (p)
> > +			__put_super(p);
> > +		p = sb;
> > +	}
> > +	if (p)
> > +		__put_super(p);
> > +	spin_unlock(&sb_lock);
> > +}
>   At least for thawing you can use iterate_supers() helper.

OK

> > +/**
> > + *	freeze_supers - call freeze_super() for all superblocks
> > + */
> > +int freeze_supers(void)
> > +{
> > +	struct super_block *sb, *p = NULL;
> > +	int error = 0;
> > +
> > +	spin_lock(&sb_lock);
> > +	/*
> > +	 * Freeze in reverse order so filesystems depending on others are
> > +	 * frozen in the right order (eg. loopback on ext3).
> > +	 */
>   Ho, hum, are you sure the order in super_blocks list is the one you need?
> Maybe it is but I'm not sure you are guaranteed it is.

Well, is there any way I can get the right order?

> I've added Al to CC,
> he'll likely have opinion on this...
> 
> > +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> > +		if (list_empty(&sb->s_instances))
> > +			continue;
> > +		sb->s_count++;
> > +		spin_unlock(&sb_lock);
> > +
> > +		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
> > +		    && !(sb->s_flags & MS_RDONLY)) {
> > +			error = freeze_super(sb);
> > +			if (!error)
> > +				sb->s_flags |= MS_FROZEN;
> > +		}
> > +
> > +		spin_lock(&sb_lock);
> > +		if (error)
> > +			break;
> > +		if (p)
> > +			__put_super(p);
> > +		p = sb;
> > +	}
> > +	if (p)
> > +		__put_super(p);
> > +	spin_unlock(&sb_lock);
> > +
> > +	if (error)
> > +		thaw_supers();
> > +
> > +	return error;
> > +}
> > +
> > +
> > +/**
> >   *	get_super - get the superblock of a device
> >   *	@bdev: device to get the superblock for
> >   *	

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-30 21:05   ` Rafael J. Wysocki
@ 2012-01-30 21:10     ` Nigel Cunningham
  2012-01-31  0:03       ` Jan Kara
  2012-01-30 23:58     ` Jan Kara
  1 sibling, 1 reply; 37+ messages in thread
From: Nigel Cunningham @ 2012-01-30 21:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jan Kara, Linux PM list, LKML, linux-fsdevel, Dave Chinner,
	Pavel Machek, Srivatsa S. Bhat, Al Viro

Hi.

On 31/01/12 08:05, Rafael J. Wysocki wrote:
> On Monday, January 30, 2012, Jan Kara wrote:
>> On Sat 28-01-12 14:45:49, Rafael J. Wysocki wrote:
>>> +	/*
>>> +	 * Freeze in reverse order so filesystems depending on others are
>>> +	 * frozen in the right order (eg. loopback on ext3).
>>> +	 */
>>   Ho, hum, are you sure the order in super_blocks list is the one you need?
>> Maybe it is but I'm not sure you are guaranteed it is.
> 
> Well, is there any way I can get the right order?

Jan, what's the case where the reverse of mount order might be the wrong
order?

Regards,

Nigel
-- 
Evolution (n): A hypothetical process whereby improbable
events occur with alarming frequency, order arises from chaos, and
no one is given credit.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-29 19:53   ` Rafael J. Wysocki
@ 2012-01-30 23:24     ` Srivatsa S. Bhat
  0 siblings, 0 replies; 37+ messages in thread
From: Srivatsa S. Bhat @ 2012-01-30 23:24 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Martin, Al Viro

On 01/30/2012 01:23 AM, Rafael J. Wysocki wrote:

> On Sunday, January 29, 2012, Srivatsa S. Bhat wrote:

[...]

>>> Index: linux/kernel/power/suspend.c
>>> ===================================================================
>>> --- linux.orig/kernel/power/suspend.c
>>> +++ linux/kernel/power/suspend.c
>>> @@ -29,6 +29,48 @@
>>>
>>>  #include "power.h"
>>>
>>> +#ifdef CONFIG_SUSPEND_FREEZER
>>> +
>>> +static inline int suspend_freeze_processes(void)
>>> +{
>>> +	int error;
>>> +
>>> +	error = freeze_processes();
>>> +	if (error)
>>> +		return error;
>>> +
>>> +	error = freeze_supers();
>>> +	if (error) {
>>> +		thaw_processes();
>>> +		return error;
>>> +	}
>>> +
>>> +	error = freeze_kernel_threads();
>>> +	if (error)
>>> +		thaw_supers();
>>> +
>>
>>
>> If freezing of kernel threads fails, freeze_kernel_threads() calls
>> thaw_processes(), which means, even userspace processes get thawed.
>> So, there would be a time-window in which userspace is thawed but the
>> filesystems are still frozen. That is not very desirable right?
> 
> No, it is not.  I overlooked that, thanks!
> 
>> If that is right, then modifying freeze_kernel_threads() to call
>> thaw_kernel_threads() instead of thaw_processes() would fix it
>> (and of course, we would need to explicitly call thaw_processes
>> above).
>>
>> BTW, after your patch posted at https://lkml.org/lkml/2012/1/27/501,
>> I very much wanted to write a patch to convert the semantics of
>> freeze/thaw to something like:
>>
>> freeze_processes() calls thaw_processes on error.
>> //Both touch only userspace processes.
>>
>> freeze_kernel_threads() calls thaw_kernel_threads() on error.
>> //Both touch only kernel threads.
>>
>> Of course, such a patch would need to do a lot of fixing up at several
>> places, but IMHO, it would really help make the overall code more logical
>> and easier to understand.
>>
>> I can write it up and post it soon, but then you'll have to rebase
>> your patch (this one) on top of that. What do you say?
> 
> Please do that, it wouldn't be any problem for me to rebase the $subject
> patch.
> 


Hi Rafael,
I have posted the patchset (v2) at https://lkml.org/lkml/2012/1/30/530

 

Regards,
Srivatsa S. Bhat


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-30 21:05   ` Rafael J. Wysocki
  2012-01-30 21:10     ` Nigel Cunningham
@ 2012-01-30 23:58     ` Jan Kara
  1 sibling, 0 replies; 37+ messages in thread
From: Jan Kara @ 2012-01-30 23:58 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jan Kara, Linux PM list, LKML, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat, Al Viro

On Mon 30-01-12 22:05:33, Rafael J. Wysocki wrote:
> On Monday, January 30, 2012, Jan Kara wrote:
> > On Sat 28-01-12 14:45:49, Rafael J. Wysocki wrote:
> > > +/**
> > > + *	freeze_supers - call freeze_super() for all superblocks
> > > + */
> > > +int freeze_supers(void)
> > > +{
> > > +	struct super_block *sb, *p = NULL;
> > > +	int error = 0;
> > > +
> > > +	spin_lock(&sb_lock);
> > > +	/*
> > > +	 * Freeze in reverse order so filesystems depending on others are
> > > +	 * frozen in the right order (eg. loopback on ext3).
> > > +	 */
> >   Ho, hum, are you sure the order in super_blocks list is the one you need?
> > Maybe it is but I'm not sure you are guaranteed it is.
> 
> Well, is there any way I can get the right order?
  None I'm aware of :( I think your reverse order scanning is the best you
can do now as we don't track the "freeze dependency" in any way. I was just
warning that noone really guarantees this is the right order. So we should
probably at least document somewhere that super_blocks list order matters
for freeze_supers().

> > > +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> > > +		if (list_empty(&sb->s_instances))
> > > +			continue;
> > > +		sb->s_count++;
> > > +		spin_unlock(&sb_lock);
> > > +
> > > +		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
> > > +		    && !(sb->s_flags & MS_RDONLY)) {
> > > +			error = freeze_super(sb);
> > > +			if (!error)
> > > +				sb->s_flags |= MS_FROZEN;
> > > +		}
> > > +
> > > +		spin_lock(&sb_lock);
> > > +		if (error)
> > > +			break;
> > > +		if (p)
> > > +			__put_super(p);
> > > +		p = sb;
> > > +	}
> > > +	if (p)
> > > +		__put_super(p);
> > > +	spin_unlock(&sb_lock);
> > > +
> > > +	if (error)
> > > +		thaw_supers();
> > > +
> > > +	return error;
> > > +}
> > > +
> > > +
> > > +/**
> > >   *	get_super - get the superblock of a device
> > >   *	@bdev: device to get the superblock for
> > >   *	

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-30 21:10     ` Nigel Cunningham
@ 2012-01-31  0:03       ` Jan Kara
  0 siblings, 0 replies; 37+ messages in thread
From: Jan Kara @ 2012-01-31  0:03 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Rafael J. Wysocki, Jan Kara, Linux PM list, LKML, linux-fsdevel,
	Dave Chinner, Pavel Machek, Srivatsa S. Bhat, Al Viro

On Tue 31-01-12 08:10:37, Nigel Cunningham wrote:
> Hi.
> 
> On 31/01/12 08:05, Rafael J. Wysocki wrote:
> > On Monday, January 30, 2012, Jan Kara wrote:
> >> On Sat 28-01-12 14:45:49, Rafael J. Wysocki wrote:
> >>> +	/*
> >>> +	 * Freeze in reverse order so filesystems depending on others are
> >>> +	 * frozen in the right order (eg. loopback on ext3).
> >>> +	 */
> >>   Ho, hum, are you sure the order in super_blocks list is the one you need?
> >> Maybe it is but I'm not sure you are guaranteed it is.
> > 
> > Well, is there any way I can get the right order?
> 
> Jan, what's the case where the reverse of mount order might be the wrong
> order?
  After some though, I'm not able to come up with a counterexample. As I
wrote to Rafael, let's at least document that we rely on some ordering of
super_blocks list.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2012-01-30 20:00 ` Jan Kara
@ 2012-02-01 13:36 ` Pavel Machek
  2012-02-01 15:29   ` Alan Stern
  2012-02-02  3:17 ` Nigel Cunningham
  2012-02-17 15:41 ` Josh Boyer
  6 siblings, 1 reply; 37+ messages in thread
From: Pavel Machek @ 2012-02-01 13:36 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Srivatsa S. Bhat

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In

Good.

> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image
> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.

Ungood. Why is bootloader/initrd doing that? If it mounts filesystem
read/write, what is the guarantee that it will not change data on the
filesystem, breaking stuff?

Bootloaders should just not replay journals.

> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.

...so bootloaders need to be fixed, anyway.
									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] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-01 13:36 ` Pavel Machek
@ 2012-02-01 15:29   ` Alan Stern
  2012-02-10  2:52     ` Jamie Lokier
  0 siblings, 1 reply; 37+ messages in thread
From: Alan Stern @ 2012-02-01 15:29 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Srivatsa S. Bhat

On Wed, 1 Feb 2012, Pavel Machek wrote:

> > particular, this should help to solve a long-standing issue that, in
> > some cases, during resume from hibernation the boot loader causes the
> > journal to be replied for the filesystem containing the kernel image
> > and/or initrd causing it to become inconsistent with the information
> > stored in the hibernation image.
> 
> Ungood. Why is bootloader/initrd doing that? If it mounts filesystem
> read/write, what is the guarantee that it will not change data on the
> filesystem, breaking stuff?
> 
> Bootloaders should just not replay journals.
> 
> > The user-space-driven hibernation (s2disk) is not covered by this
> > change, because the freezing of filesystems prevents s2disk from
> > accessing device special files it needs to do its job.
> 
> ...so bootloaders need to be fixed, anyway.

I don't know about bootloaders, but from what I've heard, Linux fs
drivers (including those inside initrds) always replay the journal,
even if the filesystem is mounted read-only.  This could be considered
a bug in the filesystem code.

Alan Stern


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2012-02-01 13:36 ` Pavel Machek
@ 2012-02-02  3:17 ` Nigel Cunningham
  2012-02-17 15:41 ` Josh Boyer
  6 siblings, 0 replies; 37+ messages in thread
From: Nigel Cunningham @ 2012-02-02  3:17 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Pavel Machek, Srivatsa S. Bhat

Hi.

On 29/01/12 00:45, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
> 
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image
> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.
> 
> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.
> 
> This change is based on earlier work by Nigel Cunningham.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Acked-by: Nigel Cunningham <nigel@tuxonice.net>


(Or reviewed by or whatever :> I'm out of the loop too much, and don't
remember what the right one is!)

-- 
Evolution (n): A hypothetical process whereby improbable
events occur with alarming frequency, order arises from chaos, and
no one is given credit.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-01 15:29   ` Alan Stern
@ 2012-02-10  2:52     ` Jamie Lokier
  2012-02-10  9:03       ` Jan Kara
  0 siblings, 1 reply; 37+ messages in thread
From: Jamie Lokier @ 2012-02-10  2:52 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pavel Machek, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Dave Chinner, Nigel Cunningham, Srivatsa S. Bhat

Alan Stern wrote:
> On Wed, 1 Feb 2012, Pavel Machek wrote:
> 
> > > particular, this should help to solve a long-standing issue that, in
> > > some cases, during resume from hibernation the boot loader causes the
> > > journal to be replied for the filesystem containing the kernel image
> > > and/or initrd causing it to become inconsistent with the information
> > > stored in the hibernation image.
> > 
> > Ungood. Why is bootloader/initrd doing that? If it mounts filesystem
> > read/write, what is the guarantee that it will not change data on the
> > filesystem, breaking stuff?
> > 
> > Bootloaders should just not replay journals.
> > 
> > > The user-space-driven hibernation (s2disk) is not covered by this
> > > change, because the freezing of filesystems prevents s2disk from
> > > accessing device special files it needs to do its job.
> > 
> > ...so bootloaders need to be fixed, anyway.
> 
> I don't know about bootloaders, but from what I've heard, Linux fs
> drivers (including those inside initrds) always replay the journal,
> even if the filesystem is mounted read-only.  This could be considered
> a bug in the filesystem code.

Theoretically a filesystem might need replay for the bootloader to see
a non-corrupt image, even for just the files it uses.

For example if the last state was in the middle of updating the root
directory, the /boot entry in the root directory might not be reliably
found without replaying the journal.

However replaying a journal when mounted read-only should probably
track journalled blocks in memory only, not commit back to the storage.

All the best,
-- Jamie

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-10  2:52     ` Jamie Lokier
@ 2012-02-10  9:03       ` Jan Kara
  0 siblings, 0 replies; 37+ messages in thread
From: Jan Kara @ 2012-02-10  9:03 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: Alan Stern, Pavel Machek, Rafael J. Wysocki, Linux PM list, LKML,
	Jan Kara, linux-fsdevel, Dave Chinner, Nigel Cunningham,
	Srivatsa S. Bhat

On Fri 10-02-12 02:52:17, Jamie Lokier wrote:
> Alan Stern wrote:
> > On Wed, 1 Feb 2012, Pavel Machek wrote:
> > 
> > > > particular, this should help to solve a long-standing issue that, in
> > > > some cases, during resume from hibernation the boot loader causes the
> > > > journal to be replied for the filesystem containing the kernel image
> > > > and/or initrd causing it to become inconsistent with the information
> > > > stored in the hibernation image.
> > > 
> > > Ungood. Why is bootloader/initrd doing that? If it mounts filesystem
> > > read/write, what is the guarantee that it will not change data on the
> > > filesystem, breaking stuff?
> > > 
> > > Bootloaders should just not replay journals.
> > > 
> > > > The user-space-driven hibernation (s2disk) is not covered by this
> > > > change, because the freezing of filesystems prevents s2disk from
> > > > accessing device special files it needs to do its job.
> > > 
> > > ...so bootloaders need to be fixed, anyway.
> > 
> > I don't know about bootloaders, but from what I've heard, Linux fs
> > drivers (including those inside initrds) always replay the journal,
> > even if the filesystem is mounted read-only.  This could be considered
> > a bug in the filesystem code.
> 
> Theoretically a filesystem might need replay for the bootloader to see
> a non-corrupt image, even for just the files it uses.
> 
> For example if the last state was in the middle of updating the root
> directory, the /boot entry in the root directory might not be reliably
> found without replaying the journal.
> 
> However replaying a journal when mounted read-only should probably
> track journalled blocks in memory only, not commit back to the storage.
  Yes, that would be nice. Although memory constraints of the bootloader
might make it tricky.

  The reason why we don't have the functionality in kernel is not so much
about memory demands but more about the complexities of implementing the
caching - we'd have to create an equivalent of dm-snapshot of the device
from the mount code to trick generic code in pagecache to loading proper
replayed data).

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2012-02-02  3:17 ` Nigel Cunningham
@ 2012-02-17 15:41 ` Josh Boyer
  2012-02-17 18:33   ` Josh Boyer
  6 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2012-02-17 15:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Sat, Jan 28, 2012 at 8:45 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Freeze all filesystems during system suspend and (kernel-driven)
> hibernation by calling freeze_supers() for all superblocks and thaw
> them during the subsequent resume with the help of thaw_supers().
>
> This makes filesystems stay in a consistent state in case something
> goes wrong between system suspend (or hibernation) and the subsequent
> resume (e.g. journal replays won't be necessary in those cases).  In
> particular, this should help to solve a long-standing issue that, in
> some cases, during resume from hibernation the boot loader causes the
> journal to be replied for the filesystem containing the kernel image
> and/or initrd causing it to become inconsistent with the information
> stored in the hibernation image.
>
> The user-space-driven hibernation (s2disk) is not covered by this
> change, because the freezing of filesystems prevents s2disk from
> accessing device special files it needs to do its job.
>
> This change is based on earlier work by Nigel Cunningham.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Do you happen to have a version of this patch that has been rebased to the
latest 3.3 git tree?  I looked in the various archives and haven't found one
yet.  I'd like to get this into Fedora rawhide to see if some of the hibernate
issues we've been seeing clear up.

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-17 15:41 ` Josh Boyer
@ 2012-02-17 18:33   ` Josh Boyer
  2012-02-17 20:59     ` Rafael J. Wysocki
  0 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2012-02-17 18:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Fri, Feb 17, 2012 at 10:41 AM, Josh Boyer <jwboyer@gmail.com> wrote:
> On Sat, Jan 28, 2012 at 8:45 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> From: Rafael J. Wysocki <rjw@sisk.pl>
>>
>> Freeze all filesystems during system suspend and (kernel-driven)
>> hibernation by calling freeze_supers() for all superblocks and thaw
>> them during the subsequent resume with the help of thaw_supers().
>>
>> This makes filesystems stay in a consistent state in case something
>> goes wrong between system suspend (or hibernation) and the subsequent
>> resume (e.g. journal replays won't be necessary in those cases).  In
>> particular, this should help to solve a long-standing issue that, in
>> some cases, during resume from hibernation the boot loader causes the
>> journal to be replied for the filesystem containing the kernel image
>> and/or initrd causing it to become inconsistent with the information
>> stored in the hibernation image.
>>
>> The user-space-driven hibernation (s2disk) is not covered by this
>> change, because the freezing of filesystems prevents s2disk from
>> accessing device special files it needs to do its job.
>>
>> This change is based on earlier work by Nigel Cunningham.
>>
>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> Do you happen to have a version of this patch that has been rebased to the
> latest 3.3 git tree?  I looked in the various archives and haven't found one
> yet.  I'd like to get this into Fedora rawhide to see if some of the hibernate
> issues we've been seeing clear up.

I got impatient and decided to tackle this myself.  If others would like to
look at the forward port below, I'd appreciate it.

josh

commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date:   Fri Feb 17 12:42:08 2012 -0500

    Freeze all filesystems during system suspend and (kernel-driven)
    hibernation by calling freeze_supers() for all superblocks and thaw
    them during the subsequent resume with the help of thaw_supers().

    This makes filesystems stay in a consistent state in case something
    goes wrong between system suspend (or hibernation) and the subsequent
    resume (e.g. journal replays won't be necessary in those cases).  In
    particular, this should help to solve a long-standing issue that, in
    some cases, during resume from hibernation the boot loader causes the
    journal to be replied for the filesystem containing the kernel image
    and/or initrd causing it to become inconsistent with the information
    stored in the hibernation image.

    The user-space-driven hibernation (s2disk) is not covered by this
    change, because the freezing of filesystems prevents s2disk from
    accessing device special files it needs to do its job.

    This change is based on earlier work by Nigel Cunningham.

    Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

    Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>

diff --git a/fs/super.c b/fs/super.c
index 6015c02..c8057fa 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -594,6 +594,79 @@ void iterate_supers_type(struct file_system_type *type,
 EXPORT_SYMBOL(iterate_supers_type);

 /**
+ *	thaw_supers - call thaw_super() for all superblocks
+ */
+void thaw_supers(void)
+{
+	struct super_block *sb, *p = NULL;
+
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		if (hlist_unhashed(&sb->s_instances))
+			continue;
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+
+		if (sb->s_flags & MS_FROZEN) {
+			thaw_super(sb);
+			sb->s_flags &= ~MS_FROZEN;
+		}
+
+		spin_lock(&sb_lock);
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+}
+
+/**
+ *	freeze_supers - call freeze_super() for all superblocks
+ */
+int freeze_supers(void)
+{
+	struct super_block *sb, *p = NULL;
+	int error = 0;
+
+	spin_lock(&sb_lock);
+	/*
+	 * Freeze in reverse order so filesystems depending on others are
+	 * frozen in the right order (eg. loopback on ext3).
+	 */
+	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
+		if (hlist_unhashed(&sb->s_instances))
+			continue;
+		sb->s_count++;
+		spin_unlock(&sb_lock);
+
+		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
+		    && !(sb->s_flags & MS_RDONLY)) {
+			error = freeze_super(sb);
+			if (!error)
+				sb->s_flags |= MS_FROZEN;
+		}
+
+		spin_lock(&sb_lock);
+		if (error)
+			break;
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+
+	if (error)
+		thaw_supers();
+
+	return error;
+}
+
+
+/**
  *	get_super - get the superblock of a device
  *	@bdev: device to get the superblock for
  *	
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 386da09..a164f4a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -210,6 +210,7 @@ struct inodes_stat_t {
 #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
 #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
 #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
+#define MS_FROZEN	(1<<25) /* Frozen filesystem */
 #define MS_NOSEC	(1<<28)
 #define MS_BORN		(1<<29)
 #define MS_ACTIVE	(1<<30)
@@ -2501,6 +2502,8 @@ extern void drop_super(struct super_block *sb);
 extern void iterate_supers(void (*)(struct super_block *, void *), void *);
 extern void iterate_supers_type(struct file_system_type *,
 			        void (*)(struct super_block *, void *), void *);
+extern int freeze_supers(void);
+extern void thaw_supers(void);

 extern int dcache_dir_open(struct inode *, struct file *);
 extern int dcache_dir_close(struct inode *, struct file *);
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 6d6d288..492fc62 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -626,12 +626,17 @@ int hibernate(void)
 	if (error)
 		goto Finish;

-	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
+	error = freeze_supers();
 	if (error)
 		goto Thaw;
+
+	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
+	if (error)
+		goto Thaw_fs;
+
 	if (freezer_test_done) {
 		freezer_test_done = false;
-		goto Thaw;
+		goto Thaw_fs;
 	}

 	if (in_suspend) {
@@ -655,6 +660,8 @@ int hibernate(void)
 		pr_debug("PM: Image restored successfully.\n");
 	}

+ Thaw_fs:
+	thaw_supers();
  Thaw:
 	thaw_processes();
  Finish:
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 21724ee..40d6f64 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -1,3 +1,4 @@
+#include <linux/fs.h>
 #include <linux/suspend.h>
 #include <linux/suspend_ioctls.h>
 #include <linux/utsname.h>
@@ -227,45 +228,3 @@ enum {
 #define TEST_MAX	(__TEST_AFTER_LAST - 1)

 extern int pm_test_level;
-
-#ifdef CONFIG_SUSPEND_FREEZER
-static inline int suspend_freeze_processes(void)
-{
-	int error;
-
-	error = freeze_processes();
-
-	/*
-	 * freeze_processes() automatically thaws every task if freezing
-	 * fails. So we need not do anything extra upon error.
-	 */
-	if (error)
-		goto Finish;
-
-	error = freeze_kernel_threads();
-
-	/*
-	 * freeze_kernel_threads() thaws only kernel threads upon freezing
-	 * failure. So we have to thaw the userspace tasks ourselves.
-	 */
-	if (error)
-		thaw_processes();
-
- Finish:
-	return error;
-}
-
-static inline void suspend_thaw_processes(void)
-{
-	thaw_processes();
-}
-#else
-static inline int suspend_freeze_processes(void)
-{
-	return 0;
-}
-
-static inline void suspend_thaw_processes(void)
-{
-}
-#endif
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 4fd51be..5f51fc7 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -29,6 +29,62 @@

 #include "power.h"

+#ifdef CONFIG_SUSPEND_FREEZER
+
+static inline int suspend_freeze_processes(void)
+{
+	int error;
+
+	error = freeze_processes();
+
+	/*
+	 * freeze_processes() automatically thaws every task if freezing
+	 * fails. So we need not do anything extra upon error.
+	 */
+
+	if (error)
+		goto Finish;
+
+	error = freeze_supers();
+	if (error) {
+		thaw_processes();
+		goto Finish;
+	}
+
+	error = freeze_kernel_threads();
+
+	/*
+	 * freeze_kernel_threads() thaws only kernel threads upon freezing
+	 * failure. So we have to thaw the userspace tasks ourselves.
+	 */
+	if (error) {
+		thaw_supers();
+		thaw_processes();
+	}
+
+Finish:
+	return error;
+}
+
+static inline void suspend_thaw_processes(void)
+{
+	thaw_supers();
+	thaw_processes();
+}
+
+#else /* !CONFIG_SUSPEND_FREEZER */
+
+static inline int suspend_freeze_processes(void)
+{
+	return 0;
+}
+
+static inline void suspend_thaw_processes(void)
+{
+}
+
+#endif /* !CONFIG_SUSPEND_FREEZER */
+
 const char *const pm_states[PM_SUSPEND_MAX] = {
 	[PM_SUSPEND_STANDBY]	= "standby",
 	[PM_SUSPEND_MEM]	= "mem",

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-17 18:33   ` Josh Boyer
@ 2012-02-17 20:59     ` Rafael J. Wysocki
  2012-05-25 16:55       ` Josh Boyer
  0 siblings, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-02-17 20:59 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Friday, February 17, 2012, Josh Boyer wrote:
> On Fri, Feb 17, 2012 at 10:41 AM, Josh Boyer <jwboyer@gmail.com> wrote:
> > On Sat, Jan 28, 2012 at 8:45 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> From: Rafael J. Wysocki <rjw@sisk.pl>
> >>
> >> Freeze all filesystems during system suspend and (kernel-driven)
> >> hibernation by calling freeze_supers() for all superblocks and thaw
> >> them during the subsequent resume with the help of thaw_supers().
> >>
> >> This makes filesystems stay in a consistent state in case something
> >> goes wrong between system suspend (or hibernation) and the subsequent
> >> resume (e.g. journal replays won't be necessary in those cases).  In
> >> particular, this should help to solve a long-standing issue that, in
> >> some cases, during resume from hibernation the boot loader causes the
> >> journal to be replied for the filesystem containing the kernel image
> >> and/or initrd causing it to become inconsistent with the information
> >> stored in the hibernation image.
> >>
> >> The user-space-driven hibernation (s2disk) is not covered by this
> >> change, because the freezing of filesystems prevents s2disk from
> >> accessing device special files it needs to do its job.
> >>
> >> This change is based on earlier work by Nigel Cunningham.
> >>
> >> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Do you happen to have a version of this patch that has been rebased to the
> > latest 3.3 git tree?  I looked in the various archives and haven't found one
> > yet.  I'd like to get this into Fedora rawhide to see if some of the hibernate
> > issues we've been seeing clear up.
> 
> I got impatient and decided to tackle this myself.  If others would like to
> look at the forward port below, I'd appreciate it.

It looks good at first sight, thanks for rebasing!

Rafael


> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
> Author: Rafael J. Wysocki <rjw@sisk.pl>
> Date:   Fri Feb 17 12:42:08 2012 -0500
> 
>     Freeze all filesystems during system suspend and (kernel-driven)
>     hibernation by calling freeze_supers() for all superblocks and thaw
>     them during the subsequent resume with the help of thaw_supers().
> 
>     This makes filesystems stay in a consistent state in case something
>     goes wrong between system suspend (or hibernation) and the subsequent
>     resume (e.g. journal replays won't be necessary in those cases).  In
>     particular, this should help to solve a long-standing issue that, in
>     some cases, during resume from hibernation the boot loader causes the
>     journal to be replied for the filesystem containing the kernel image
>     and/or initrd causing it to become inconsistent with the information
>     stored in the hibernation image.
> 
>     The user-space-driven hibernation (s2disk) is not covered by this
>     change, because the freezing of filesystems prevents s2disk from
>     accessing device special files it needs to do its job.
> 
>     This change is based on earlier work by Nigel Cunningham.
> 
>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
> 
> diff --git a/fs/super.c b/fs/super.c
> index 6015c02..c8057fa 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -594,6 +594,79 @@ void iterate_supers_type(struct file_system_type *type,
>  EXPORT_SYMBOL(iterate_supers_type);
> 
>  /**
> + *	thaw_supers - call thaw_super() for all superblocks
> + */
> +void thaw_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +
> +	spin_lock(&sb_lock);
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		if (hlist_unhashed(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_flags & MS_FROZEN) {
> +			thaw_super(sb);
> +			sb->s_flags &= ~MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +}
> +
> +/**
> + *	freeze_supers - call freeze_super() for all superblocks
> + */
> +int freeze_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +	int error = 0;
> +
> +	spin_lock(&sb_lock);
> +	/*
> +	 * Freeze in reverse order so filesystems depending on others are
> +	 * frozen in the right order (eg. loopback on ext3).
> +	 */
> +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> +		if (hlist_unhashed(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +		spin_unlock(&sb_lock);
> +
> +		if (sb->s_root && sb->s_frozen != SB_FREEZE_TRANS
> +		    && !(sb->s_flags & MS_RDONLY)) {
> +			error = freeze_super(sb);
> +			if (!error)
> +				sb->s_flags |= MS_FROZEN;
> +		}
> +
> +		spin_lock(&sb_lock);
> +		if (error)
> +			break;
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +
> +	if (error)
> +		thaw_supers();
> +
> +	return error;
> +}
> +
> +
> +/**
>   *	get_super - get the superblock of a device
>   *	@bdev: device to get the superblock for
>   *	
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 386da09..a164f4a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -210,6 +210,7 @@ struct inodes_stat_t {
>  #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
>  #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
>  #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
> +#define MS_FROZEN	(1<<25) /* Frozen filesystem */
>  #define MS_NOSEC	(1<<28)
>  #define MS_BORN		(1<<29)
>  #define MS_ACTIVE	(1<<30)
> @@ -2501,6 +2502,8 @@ extern void drop_super(struct super_block *sb);
>  extern void iterate_supers(void (*)(struct super_block *, void *), void *);
>  extern void iterate_supers_type(struct file_system_type *,
>  			        void (*)(struct super_block *, void *), void *);
> +extern int freeze_supers(void);
> +extern void thaw_supers(void);
> 
>  extern int dcache_dir_open(struct inode *, struct file *);
>  extern int dcache_dir_close(struct inode *, struct file *);
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 6d6d288..492fc62 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -626,12 +626,17 @@ int hibernate(void)
>  	if (error)
>  		goto Finish;
> 
> -	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	error = freeze_supers();
>  	if (error)
>  		goto Thaw;
> +
> +	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> +	if (error)
> +		goto Thaw_fs;
> +
>  	if (freezer_test_done) {
>  		freezer_test_done = false;
> -		goto Thaw;
> +		goto Thaw_fs;
>  	}
> 
>  	if (in_suspend) {
> @@ -655,6 +660,8 @@ int hibernate(void)
>  		pr_debug("PM: Image restored successfully.\n");
>  	}
> 
> + Thaw_fs:
> +	thaw_supers();
>   Thaw:
>  	thaw_processes();
>   Finish:
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 21724ee..40d6f64 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -1,3 +1,4 @@
> +#include <linux/fs.h>
>  #include <linux/suspend.h>
>  #include <linux/suspend_ioctls.h>
>  #include <linux/utsname.h>
> @@ -227,45 +228,3 @@ enum {
>  #define TEST_MAX	(__TEST_AFTER_LAST - 1)
> 
>  extern int pm_test_level;
> -
> -#ifdef CONFIG_SUSPEND_FREEZER
> -static inline int suspend_freeze_processes(void)
> -{
> -	int error;
> -
> -	error = freeze_processes();
> -
> -	/*
> -	 * freeze_processes() automatically thaws every task if freezing
> -	 * fails. So we need not do anything extra upon error.
> -	 */
> -	if (error)
> -		goto Finish;
> -
> -	error = freeze_kernel_threads();
> -
> -	/*
> -	 * freeze_kernel_threads() thaws only kernel threads upon freezing
> -	 * failure. So we have to thaw the userspace tasks ourselves.
> -	 */
> -	if (error)
> -		thaw_processes();
> -
> - Finish:
> -	return error;
> -}
> -
> -static inline void suspend_thaw_processes(void)
> -{
> -	thaw_processes();
> -}
> -#else
> -static inline int suspend_freeze_processes(void)
> -{
> -	return 0;
> -}
> -
> -static inline void suspend_thaw_processes(void)
> -{
> -}
> -#endif
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 4fd51be..5f51fc7 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -29,6 +29,62 @@
> 
>  #include "power.h"
> 
> +#ifdef CONFIG_SUSPEND_FREEZER
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	int error;
> +
> +	error = freeze_processes();
> +
> +	/*
> +	 * freeze_processes() automatically thaws every task if freezing
> +	 * fails. So we need not do anything extra upon error.
> +	 */
> +
> +	if (error)
> +		goto Finish;
> +
> +	error = freeze_supers();
> +	if (error) {
> +		thaw_processes();
> +		goto Finish;
> +	}
> +
> +	error = freeze_kernel_threads();
> +
> +	/*
> +	 * freeze_kernel_threads() thaws only kernel threads upon freezing
> +	 * failure. So we have to thaw the userspace tasks ourselves.
> +	 */
> +	if (error) {
> +		thaw_supers();
> +		thaw_processes();
> +	}
> +
> +Finish:
> +	return error;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +	thaw_supers();
> +	thaw_processes();
> +}
> +
> +#else /* !CONFIG_SUSPEND_FREEZER */
> +
> +static inline int suspend_freeze_processes(void)
> +{
> +	return 0;
> +}
> +
> +static inline void suspend_thaw_processes(void)
> +{
> +}
> +
> +#endif /* !CONFIG_SUSPEND_FREEZER */
> +
>  const char *const pm_states[PM_SUSPEND_MAX] = {
>  	[PM_SUSPEND_STANDBY]	= "standby",
>  	[PM_SUSPEND_MEM]	= "mem",
> 
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-02-17 20:59     ` Rafael J. Wysocki
@ 2012-05-25 16:55       ` Josh Boyer
  2012-05-25 19:13         ` Rafael J. Wysocki
  0 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2012-05-25 16:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
>> Author: Rafael J. Wysocki <rjw@sisk.pl>
>> Date:   Fri Feb 17 12:42:08 2012 -0500
>>
>>     Freeze all filesystems during system suspend and (kernel-driven)
>>     hibernation by calling freeze_supers() for all superblocks and thaw
>>     them during the subsequent resume with the help of thaw_supers().
>>
>>     This makes filesystems stay in a consistent state in case something
>>     goes wrong between system suspend (or hibernation) and the subsequent
>>     resume (e.g. journal replays won't be necessary in those cases).  In
>>     particular, this should help to solve a long-standing issue that, in
>>     some cases, during resume from hibernation the boot loader causes the
>>     journal to be replied for the filesystem containing the kernel image
>>     and/or initrd causing it to become inconsistent with the information
>>     stored in the hibernation image.
>>
>>     The user-space-driven hibernation (s2disk) is not covered by this
>>     change, because the freezing of filesystems prevents s2disk from
>>     accessing device special files it needs to do its job.
>>
>>     This change is based on earlier work by Nigel Cunningham.
>>
>>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>>
>>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>

Did this patch ever wind up going anywhere?  Fedora has it sitting in
our tree with a comment that says "rebase" and I don't see it in the
linux-next tree at all.

Did if fall through the cracks or was it NAKed somewhere?

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-05-25 16:55       ` Josh Boyer
@ 2012-05-25 19:13         ` Rafael J. Wysocki
  2013-12-17 16:03           ` Josh Boyer
  0 siblings, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki @ 2012-05-25 19:13 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Friday, May 25, 2012, Josh Boyer wrote:
> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
> >> Date:   Fri Feb 17 12:42:08 2012 -0500
> >>
> >>     Freeze all filesystems during system suspend and (kernel-driven)
> >>     hibernation by calling freeze_supers() for all superblocks and thaw
> >>     them during the subsequent resume with the help of thaw_supers().
> >>
> >>     This makes filesystems stay in a consistent state in case something
> >>     goes wrong between system suspend (or hibernation) and the subsequent
> >>     resume (e.g. journal replays won't be necessary in those cases).  In
> >>     particular, this should help to solve a long-standing issue that, in
> >>     some cases, during resume from hibernation the boot loader causes the
> >>     journal to be replied for the filesystem containing the kernel image
> >>     and/or initrd causing it to become inconsistent with the information
> >>     stored in the hibernation image.
> >>
> >>     The user-space-driven hibernation (s2disk) is not covered by this
> >>     change, because the freezing of filesystems prevents s2disk from
> >>     accessing device special files it needs to do its job.
> >>
> >>     This change is based on earlier work by Nigel Cunningham.
> >>
> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> >>
> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
> 
> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> our tree with a comment that says "rebase" and I don't see it in the
> linux-next tree at all.
> 
> Did if fall through the cracks or was it NAKed somewhere?

No, it wasn't in principle. There were some comments I haven't addressed yet.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2012-05-25 19:13         ` Rafael J. Wysocki
@ 2013-12-17 16:03           ` Josh Boyer
  2013-12-17 16:04             ` Josh Boyer
  0 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2013-12-17 16:03 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday, May 25, 2012, Josh Boyer wrote:
>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
>> >>
>> >>     Freeze all filesystems during system suspend and (kernel-driven)
>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
>> >>     them during the subsequent resume with the help of thaw_supers().
>> >>
>> >>     This makes filesystems stay in a consistent state in case something
>> >>     goes wrong between system suspend (or hibernation) and the subsequent
>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
>> >>     particular, this should help to solve a long-standing issue that, in
>> >>     some cases, during resume from hibernation the boot loader causes the
>> >>     journal to be replied for the filesystem containing the kernel image
>> >>     and/or initrd causing it to become inconsistent with the information
>> >>     stored in the hibernation image.
>> >>
>> >>     The user-space-driven hibernation (s2disk) is not covered by this
>> >>     change, because the freezing of filesystems prevents s2disk from
>> >>     accessing device special files it needs to do its job.
>> >>
>> >>     This change is based on earlier work by Nigel Cunningham.
>> >>
>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> >>
>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
>>
>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
>> our tree with a comment that says "rebase" and I don't see it in the
>> linux-next tree at all.
>>
>> Did if fall through the cracks or was it NAKed somewhere?
>
> No, it wasn't in principle. There were some comments I haven't addressed yet.

Dredging up a really old thread, sorry.

We're still carrying this patch along in Fedora.  Should we drop it at
this point, or is it still eventually going to head upstream?

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-17 16:03           ` Josh Boyer
@ 2013-12-17 16:04             ` Josh Boyer
  2013-12-17 23:08               ` Pavel Machek
  2013-12-18  0:52               ` Rafael J. Wysocki
  0 siblings, 2 replies; 37+ messages in thread
From: Josh Boyer @ 2013-12-17 16:04 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Jan Kara, linux-fsdevel, Dave Chinner,
	Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> On Friday, May 25, 2012, Josh Boyer wrote:
>>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
>>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
>>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
>>> >>
>>> >>     Freeze all filesystems during system suspend and (kernel-driven)
>>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
>>> >>     them during the subsequent resume with the help of thaw_supers().
>>> >>
>>> >>     This makes filesystems stay in a consistent state in case something
>>> >>     goes wrong between system suspend (or hibernation) and the subsequent
>>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
>>> >>     particular, this should help to solve a long-standing issue that, in
>>> >>     some cases, during resume from hibernation the boot loader causes the
>>> >>     journal to be replied for the filesystem containing the kernel image
>>> >>     and/or initrd causing it to become inconsistent with the information
>>> >>     stored in the hibernation image.
>>> >>
>>> >>     The user-space-driven hibernation (s2disk) is not covered by this
>>> >>     change, because the freezing of filesystems prevents s2disk from
>>> >>     accessing device special files it needs to do its job.
>>> >>
>>> >>     This change is based on earlier work by Nigel Cunningham.
>>> >>
>>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>>> >>
>>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
>>>
>>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
>>> our tree with a comment that says "rebase" and I don't see it in the
>>> linux-next tree at all.
>>>
>>> Did if fall through the cracks or was it NAKed somewhere?
>>
>> No, it wasn't in principle. There were some comments I haven't addressed yet.
>
> Dredging up a really old thread, sorry.
>
> We're still carrying this patch along in Fedora.  Should we drop it at
> this point, or is it still eventually going to head upstream?

Fixed Rafael's email address.  (Double sorry.)

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-17 16:04             ` Josh Boyer
@ 2013-12-17 23:08               ` Pavel Machek
  2013-12-17 23:31                 ` Dave Chinner
  2013-12-18  0:52               ` Rafael J. Wysocki
  1 sibling, 1 reply; 37+ messages in thread
From: Pavel Machek @ 2013-12-17 23:08 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Srivatsa S. Bhat

Hi!

> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> >>> our tree with a comment that says "rebase" and I don't see it in the
> >>> linux-next tree at all.
> >>>
> >>> Did if fall through the cracks or was it NAKed somewhere?
> >>
> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> >
> > Dredging up a really old thread, sorry.
> >
> > We're still carrying this patch along in Fedora.  Should we drop it at
> > this point, or is it still eventually going to head upstream?

I'd say drop.

									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] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-17 23:08               ` Pavel Machek
@ 2013-12-17 23:31                 ` Dave Chinner
  2013-12-18  0:01                   ` Pavel Machek
  0 siblings, 1 reply; 37+ messages in thread
From: Dave Chinner @ 2013-12-17 23:31 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

On Wed, Dec 18, 2013 at 12:08:43AM +0100, Pavel Machek wrote:
> Hi!
> 
> > >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> > >>> our tree with a comment that says "rebase" and I don't see it in the
> > >>> linux-next tree at all.
> > >>>
> > >>> Did if fall through the cracks or was it NAKed somewhere?
> > >>
> > >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> > >
> > > Dredging up a really old thread, sorry.
> > >
> > > We're still carrying this patch along in Fedora.  Should we drop it at
> > > this point, or is it still eventually going to head upstream?
> 
> I'd say drop.

I disagree - given the problem it is resolving leads to silent
filesystem corruption, this patch should be considered somewhat of a
priority to push...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-17 23:31                 ` Dave Chinner
@ 2013-12-18  0:01                   ` Pavel Machek
  2013-12-18 12:39                     ` Dave Chinner
  0 siblings, 1 reply; 37+ messages in thread
From: Pavel Machek @ 2013-12-18  0:01 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

On Wed 2013-12-18 10:31:52, Dave Chinner wrote:
> On Wed, Dec 18, 2013 at 12:08:43AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> > > >>> our tree with a comment that says "rebase" and I don't see it in the
> > > >>> linux-next tree at all.
> > > >>>
> > > >>> Did if fall through the cracks or was it NAKed somewhere?
> > > >>
> > > >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> > > >
> > > > Dredging up a really old thread, sorry.
> > > >
> > > > We're still carrying this patch along in Fedora.  Should we drop it at
> > > > this point, or is it still eventually going to head upstream?
> > 
> > I'd say drop.
> 
> I disagree - given the problem it is resolving leads to silent
> filesystem corruption, this patch should be considered somewhat of a
> priority to push...

Umm. Ok, I forgot what it does, really.

So... for few years now suspend corrupts data on XFS? And Fedora has
the fix but it is not in mainline? That does not sound right...

									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] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-17 16:04             ` Josh Boyer
  2013-12-17 23:08               ` Pavel Machek
@ 2013-12-18  0:52               ` Rafael J. Wysocki
  2013-12-18  1:00                 ` Josh Boyer
  1 sibling, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki @ 2013-12-18  0:52 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Tuesday, December 17, 2013 11:04:46 AM Josh Boyer wrote:
> On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> > On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> On Friday, May 25, 2012, Josh Boyer wrote:
> >>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
> >>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
> >>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
> >>> >>
> >>> >>     Freeze all filesystems during system suspend and (kernel-driven)
> >>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
> >>> >>     them during the subsequent resume with the help of thaw_supers().
> >>> >>
> >>> >>     This makes filesystems stay in a consistent state in case something
> >>> >>     goes wrong between system suspend (or hibernation) and the subsequent
> >>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
> >>> >>     particular, this should help to solve a long-standing issue that, in
> >>> >>     some cases, during resume from hibernation the boot loader causes the
> >>> >>     journal to be replied for the filesystem containing the kernel image
> >>> >>     and/or initrd causing it to become inconsistent with the information
> >>> >>     stored in the hibernation image.
> >>> >>
> >>> >>     The user-space-driven hibernation (s2disk) is not covered by this
> >>> >>     change, because the freezing of filesystems prevents s2disk from
> >>> >>     accessing device special files it needs to do its job.
> >>> >>
> >>> >>     This change is based on earlier work by Nigel Cunningham.
> >>> >>
> >>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> >>> >>
> >>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
> >>>
> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> >>> our tree with a comment that says "rebase" and I don't see it in the
> >>> linux-next tree at all.
> >>>
> >>> Did if fall through the cracks or was it NAKed somewhere?
> >>
> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> >
> > Dredging up a really old thread, sorry.
> >
> > We're still carrying this patch along in Fedora.  Should we drop it at
> > this point, or is it still eventually going to head upstream?
> 
> Fixed Rafael's email address.  (Double sorry.)

No biggie.

I just hadn't got sufficient response for that patch at the time it was
submitted, so I guess it would be good to resubmit it.  Please feel free to
do that if you want.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18  0:52               ` Rafael J. Wysocki
@ 2013-12-18  1:00                 ` Josh Boyer
  2013-12-18  1:16                   ` Rafael J. Wysocki
  0 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2013-12-18  1:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Dave Chinner, Nigel Cunningham, Pavel Machek,
	Srivatsa S. Bhat

On Tue, Dec 17, 2013 at 7:52 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, December 17, 2013 11:04:46 AM Josh Boyer wrote:
>> On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>> > On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> On Friday, May 25, 2012, Josh Boyer wrote:
>> >>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
>> >>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
>> >>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
>> >>> >>
>> >>> >>     Freeze all filesystems during system suspend and (kernel-driven)
>> >>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
>> >>> >>     them during the subsequent resume with the help of thaw_supers().
>> >>> >>
>> >>> >>     This makes filesystems stay in a consistent state in case something
>> >>> >>     goes wrong between system suspend (or hibernation) and the subsequent
>> >>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
>> >>> >>     particular, this should help to solve a long-standing issue that, in
>> >>> >>     some cases, during resume from hibernation the boot loader causes the
>> >>> >>     journal to be replied for the filesystem containing the kernel image
>> >>> >>     and/or initrd causing it to become inconsistent with the information
>> >>> >>     stored in the hibernation image.
>> >>> >>
>> >>> >>     The user-space-driven hibernation (s2disk) is not covered by this
>> >>> >>     change, because the freezing of filesystems prevents s2disk from
>> >>> >>     accessing device special files it needs to do its job.
>> >>> >>
>> >>> >>     This change is based on earlier work by Nigel Cunningham.
>> >>> >>
>> >>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> >>> >>
>> >>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
>> >>>
>> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
>> >>> our tree with a comment that says "rebase" and I don't see it in the
>> >>> linux-next tree at all.
>> >>>
>> >>> Did if fall through the cracks or was it NAKed somewhere?
>> >>
>> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
>> >
>> > Dredging up a really old thread, sorry.
>> >
>> > We're still carrying this patch along in Fedora.  Should we drop it at
>> > this point, or is it still eventually going to head upstream?
>>
>> Fixed Rafael's email address.  (Double sorry.)
>
> No biggie.
>
> I just hadn't got sufficient response for that patch at the time it was
> submitted, so I guess it would be good to resubmit it.  Please feel free to
> do that if you want.

You want me to resend a patch you authored back to you?  I mean, I can
do that but it seems a bit strange.  All I did was rebase what you
wrote to a newer kernel version.

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18  1:00                 ` Josh Boyer
@ 2013-12-18  1:16                   ` Rafael J. Wysocki
  2013-12-18 12:31                     ` Josh Boyer
  0 siblings, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki @ 2013-12-18  1:16 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Tuesday, December 17, 2013 08:00:55 PM Josh Boyer wrote:
> On Tue, Dec 17, 2013 at 7:52 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, December 17, 2013 11:04:46 AM Josh Boyer wrote:
> >> On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> >> > On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> On Friday, May 25, 2012, Josh Boyer wrote:
> >> >>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
> >> >>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
> >> >>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
> >> >>> >>
> >> >>> >>     Freeze all filesystems during system suspend and (kernel-driven)
> >> >>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
> >> >>> >>     them during the subsequent resume with the help of thaw_supers().
> >> >>> >>
> >> >>> >>     This makes filesystems stay in a consistent state in case something
> >> >>> >>     goes wrong between system suspend (or hibernation) and the subsequent
> >> >>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
> >> >>> >>     particular, this should help to solve a long-standing issue that, in
> >> >>> >>     some cases, during resume from hibernation the boot loader causes the
> >> >>> >>     journal to be replied for the filesystem containing the kernel image
> >> >>> >>     and/or initrd causing it to become inconsistent with the information
> >> >>> >>     stored in the hibernation image.
> >> >>> >>
> >> >>> >>     The user-space-driven hibernation (s2disk) is not covered by this
> >> >>> >>     change, because the freezing of filesystems prevents s2disk from
> >> >>> >>     accessing device special files it needs to do its job.
> >> >>> >>
> >> >>> >>     This change is based on earlier work by Nigel Cunningham.
> >> >>> >>
> >> >>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> >> >>> >>
> >> >>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
> >> >>>
> >> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> >> >>> our tree with a comment that says "rebase" and I don't see it in the
> >> >>> linux-next tree at all.
> >> >>>
> >> >>> Did if fall through the cracks or was it NAKed somewhere?
> >> >>
> >> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> >> >
> >> > Dredging up a really old thread, sorry.
> >> >
> >> > We're still carrying this patch along in Fedora.  Should we drop it at
> >> > this point, or is it still eventually going to head upstream?
> >>
> >> Fixed Rafael's email address.  (Double sorry.)
> >
> > No biggie.
> >
> > I just hadn't got sufficient response for that patch at the time it was
> > submitted, so I guess it would be good to resubmit it.  Please feel free to
> > do that if you want.
> 
> You want me to resend a patch you authored back to you?  I mean, I can
> do that but it seems a bit strange.  All I did was rebase what you
> wrote to a newer kernel version.

Well, you can send it to me in private then and I'll resubmit. :-)

I just don't have any recent version of it handy.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18  1:16                   ` Rafael J. Wysocki
@ 2013-12-18 12:31                     ` Josh Boyer
  2013-12-18 21:40                       ` Rafael J. Wysocki
  0 siblings, 1 reply; 37+ messages in thread
From: Josh Boyer @ 2013-12-18 12:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Tue, Dec 17, 2013 at 8:16 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, December 17, 2013 08:00:55 PM Josh Boyer wrote:
>> On Tue, Dec 17, 2013 at 7:52 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Tuesday, December 17, 2013 11:04:46 AM Josh Boyer wrote:
>> >> On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
>> >> > On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> On Friday, May 25, 2012, Josh Boyer wrote:
>> >> >>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
>> >> >>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
>> >> >>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
>> >> >>> >>
>> >> >>> >>     Freeze all filesystems during system suspend and (kernel-driven)
>> >> >>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
>> >> >>> >>     them during the subsequent resume with the help of thaw_supers().
>> >> >>> >>
>> >> >>> >>     This makes filesystems stay in a consistent state in case something
>> >> >>> >>     goes wrong between system suspend (or hibernation) and the subsequent
>> >> >>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
>> >> >>> >>     particular, this should help to solve a long-standing issue that, in
>> >> >>> >>     some cases, during resume from hibernation the boot loader causes the
>> >> >>> >>     journal to be replied for the filesystem containing the kernel image
>> >> >>> >>     and/or initrd causing it to become inconsistent with the information
>> >> >>> >>     stored in the hibernation image.
>> >> >>> >>
>> >> >>> >>     The user-space-driven hibernation (s2disk) is not covered by this
>> >> >>> >>     change, because the freezing of filesystems prevents s2disk from
>> >> >>> >>     accessing device special files it needs to do its job.
>> >> >>> >>
>> >> >>> >>     This change is based on earlier work by Nigel Cunningham.
>> >> >>> >>
>> >> >>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> >> >>> >>
>> >> >>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
>> >> >>>
>> >> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
>> >> >>> our tree with a comment that says "rebase" and I don't see it in the
>> >> >>> linux-next tree at all.
>> >> >>>
>> >> >>> Did if fall through the cracks or was it NAKed somewhere?
>> >> >>
>> >> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
>> >> >
>> >> > Dredging up a really old thread, sorry.
>> >> >
>> >> > We're still carrying this patch along in Fedora.  Should we drop it at
>> >> > this point, or is it still eventually going to head upstream?
>> >>
>> >> Fixed Rafael's email address.  (Double sorry.)
>> >
>> > No biggie.
>> >
>> > I just hadn't got sufficient response for that patch at the time it was
>> > submitted, so I guess it would be good to resubmit it.  Please feel free to
>> > do that if you want.
>>
>> You want me to resend a patch you authored back to you?  I mean, I can
>> do that but it seems a bit strange.  All I did was rebase what you
>> wrote to a newer kernel version.
>
> Well, you can send it to me in private then and I'll resubmit. :-)

Done.  Thanks.

josh

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18  0:01                   ` Pavel Machek
@ 2013-12-18 12:39                     ` Dave Chinner
  2013-12-18 14:08                       ` Pavel Machek
  0 siblings, 1 reply; 37+ messages in thread
From: Dave Chinner @ 2013-12-18 12:39 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

On Wed, Dec 18, 2013 at 01:01:28AM +0100, Pavel Machek wrote:
> On Wed 2013-12-18 10:31:52, Dave Chinner wrote:
> > On Wed, Dec 18, 2013 at 12:08:43AM +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> > > > >>> our tree with a comment that says "rebase" and I don't see it in the
> > > > >>> linux-next tree at all.
> > > > >>>
> > > > >>> Did if fall through the cracks or was it NAKed somewhere?
> > > > >>
> > > > >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> > > > >
> > > > > Dredging up a really old thread, sorry.
> > > > >
> > > > > We're still carrying this patch along in Fedora.  Should we drop it at
> > > > > this point, or is it still eventually going to head upstream?
> > > 
> > > I'd say drop.
> > 
> > I disagree - given the problem it is resolving leads to silent
> > filesystem corruption, this patch should be considered somewhat of a
> > priority to push...
> 
> Umm. Ok, I forgot what it does, really.

It ensures that the filesystem is in an quiescent state both in
memory and on disk, and it cannot be modified in memory or on disk
whilst the suspend image is being generated, or by log recovery
after a resume before the suspended image has been restored.

> So... for few years now suspend corrupts data on XFS? And Fedora has
> the fix but it is not in mainline? That does not sound right...

The issues freezing the filesystem before the suspend image is
created affect every journalled filesystem linux supports, be
it XFS, ext4, reiser, btrfs, etc.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18 12:39                     ` Dave Chinner
@ 2013-12-18 14:08                       ` Pavel Machek
  2013-12-19  0:22                         ` Dave Chinner
  0 siblings, 1 reply; 37+ messages in thread
From: Pavel Machek @ 2013-12-18 14:08 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

> > > I disagree - given the problem it is resolving leads to silent
> > > filesystem corruption, this patch should be considered somewhat of a
> > > priority to push...
> > 
> > Umm. Ok, I forgot what it does, really.
> 
> It ensures that the filesystem is in an quiescent state both in
> memory and on disk, and it cannot be modified in memory or on disk
> whilst the suspend image is being generated, or by log recovery
> after a resume before the suspended image has been restored.

If someone attempts to run log recovery before resume, that's a bug
and yes, it will corrupt filesystems. (Including ext3). Don't do that.

Documentation/power/swsusp.txt:

 * BIG FAT WARNING
   *********************************************************
 *
 * If you touch anything on disk between suspend and resume...
 *                              ...kiss your data goodbye.

> > So... for few years now suspend corrupts data on XFS? And Fedora has
> > the fix but it is not in mainline? That does not sound right...
> 
> The issues freezing the filesystem before the suspend image is
> created affect every journalled filesystem linux supports, be
> it XFS, ext4, reiser, btrfs, etc.

Did not it have some problems with ext3?

Regards,
									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] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18 12:31                     ` Josh Boyer
@ 2013-12-18 21:40                       ` Rafael J. Wysocki
  0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki @ 2013-12-18 21:40 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Rafael J. Wysocki, Linux PM list, LKML, Jan Kara, linux-fsdevel,
	Dave Chinner, Nigel Cunningham, Pavel Machek, Srivatsa S. Bhat

On Wednesday, December 18, 2013 07:31:59 AM Josh Boyer wrote:
> On Tue, Dec 17, 2013 at 8:16 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, December 17, 2013 08:00:55 PM Josh Boyer wrote:
> >> On Tue, Dec 17, 2013 at 7:52 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> > On Tuesday, December 17, 2013 11:04:46 AM Josh Boyer wrote:
> >> >> On Tue, Dec 17, 2013 at 11:03 AM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> >> >> > On Fri, May 25, 2012 at 3:13 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> >> On Friday, May 25, 2012, Josh Boyer wrote:
> >> >> >>> On Fri, Feb 17, 2012 at 3:59 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> >>> >> commit b94887bbc0621e1e8402e7f0ec4bc3adf46c9a6e
> >> >> >>> >> Author: Rafael J. Wysocki <rjw@sisk.pl>
> >> >> >>> >> Date:   Fri Feb 17 12:42:08 2012 -0500
> >> >> >>> >>
> >> >> >>> >>     Freeze all filesystems during system suspend and (kernel-driven)
> >> >> >>> >>     hibernation by calling freeze_supers() for all superblocks and thaw
> >> >> >>> >>     them during the subsequent resume with the help of thaw_supers().
> >> >> >>> >>
> >> >> >>> >>     This makes filesystems stay in a consistent state in case something
> >> >> >>> >>     goes wrong between system suspend (or hibernation) and the subsequent
> >> >> >>> >>     resume (e.g. journal replays won't be necessary in those cases).  In
> >> >> >>> >>     particular, this should help to solve a long-standing issue that, in
> >> >> >>> >>     some cases, during resume from hibernation the boot loader causes the
> >> >> >>> >>     journal to be replied for the filesystem containing the kernel image
> >> >> >>> >>     and/or initrd causing it to become inconsistent with the information
> >> >> >>> >>     stored in the hibernation image.
> >> >> >>> >>
> >> >> >>> >>     The user-space-driven hibernation (s2disk) is not covered by this
> >> >> >>> >>     change, because the freezing of filesystems prevents s2disk from
> >> >> >>> >>     accessing device special files it needs to do its job.
> >> >> >>> >>
> >> >> >>> >>     This change is based on earlier work by Nigel Cunningham.
> >> >> >>> >>
> >> >> >>> >>     Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> >> >> >>> >>
> >> >> >>> >>     Rebased to 3.3-rc3 by Josh Boyer <jwboyer@redhat.com>
> >> >> >>>
> >> >> >>> Did this patch ever wind up going anywhere?  Fedora has it sitting in
> >> >> >>> our tree with a comment that says "rebase" and I don't see it in the
> >> >> >>> linux-next tree at all.
> >> >> >>>
> >> >> >>> Did if fall through the cracks or was it NAKed somewhere?
> >> >> >>
> >> >> >> No, it wasn't in principle. There were some comments I haven't addressed yet.
> >> >> >
> >> >> > Dredging up a really old thread, sorry.
> >> >> >
> >> >> > We're still carrying this patch along in Fedora.  Should we drop it at
> >> >> > this point, or is it still eventually going to head upstream?
> >> >>
> >> >> Fixed Rafael's email address.  (Double sorry.)
> >> >
> >> > No biggie.
> >> >
> >> > I just hadn't got sufficient response for that patch at the time it was
> >> > submitted, so I guess it would be good to resubmit it.  Please feel free to
> >> > do that if you want.
> >>
> >> You want me to resend a patch you authored back to you?  I mean, I can
> >> do that but it seems a bit strange.  All I did was rebase what you
> >> wrote to a newer kernel version.
> >
> > Well, you can send it to me in private then and I'll resubmit. :-)
> 
> Done.  Thanks.

OK, thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-18 14:08                       ` Pavel Machek
@ 2013-12-19  0:22                         ` Dave Chinner
  2013-12-21 23:33                           ` Pavel Machek
  0 siblings, 1 reply; 37+ messages in thread
From: Dave Chinner @ 2013-12-19  0:22 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

On Wed, Dec 18, 2013 at 03:08:42PM +0100, Pavel Machek wrote:
> > > > I disagree - given the problem it is resolving leads to silent
> > > > filesystem corruption, this patch should be considered somewhat of a
> > > > priority to push...
> > > 
> > > Umm. Ok, I forgot what it does, really.
> > 
> > It ensures that the filesystem is in an quiescent state both in
> > memory and on disk, and it cannot be modified in memory or on disk
> > whilst the suspend image is being generated, or by log recovery
> > after a resume before the suspended image has been restored.
> 
> If someone attempts to run log recovery before resume, that's a bug
> and yes, it will corrupt filesystems. (Including ext3). Don't do that.

Freezing the filesystem prevents that accidental mount of the
filesystem from being an issue. It fixes a bug that:

> Documentation/power/swsusp.txt:
> 
>  * BIG FAT WARNING
>    *********************************************************
>  *
>  * If you touch anything on disk between suspend and resume...
>  *                              ...kiss your data goodbye.

Makes this a whole lot less dangerous.

> > > So... for few years now suspend corrupts data on XFS? And Fedora has
> > > the fix but it is not in mainline? That does not sound right...
> > 
> > The issues freezing the filesystem before the suspend image is
> > created affect every journalled filesystem linux supports, be
> > it XFS, ext4, reiser, btrfs, etc.
> 
> Did not it have some problems with ext3?

Please read more carefully: "affect every journalled filesystem
linux supports". So, ext3 is affected because it's a journalling
filesystem.  I didn't list every journalled filesystem Linux
supports - "etc" means there are more examples that aren't
explicitly listed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-19  0:22                         ` Dave Chinner
@ 2013-12-21 23:33                           ` Pavel Machek
  2013-12-23  3:48                             ` Dave Chinner
  0 siblings, 1 reply; 37+ messages in thread
From: Pavel Machek @ 2013-12-21 23:33 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

Hi!

> > > > > I disagree - given the problem it is resolving leads to silent
> > > > > filesystem corruption, this patch should be considered somewhat of a
> > > > > priority to push...
> > > > 
> > > > Umm. Ok, I forgot what it does, really.
> > > 
> > > It ensures that the filesystem is in an quiescent state both in
> > > memory and on disk, and it cannot be modified in memory or on disk
> > > whilst the suspend image is being generated, or by log recovery
> > > after a resume before the suspended image has been restored.
> > 
> > If someone attempts to run log recovery before resume, that's a bug
> > and yes, it will corrupt filesystems. (Including ext3). Don't do that.
> 
> Freezing the filesystem prevents that accidental mount of the
> filesystem from being an issue. It fixes a bug that:

Can you elaborate on that?

If you do read-write mount of that filesystem, surely filesystem
metadata will differ from what the filesystem expects. You'll still
get data corruption AFAICT.

Read-only mount... maybe that will get slightly better -- there'll be
no journal to play back. But what happens to superblock information
such as "last mount time"? Mount counts?

> > Documentation/power/swsusp.txt:
> > 
> >  * BIG FAT WARNING
> >    *********************************************************
> >  *
> >  * If you touch anything on disk between suspend and resume...
> >  *                              ...kiss your data goodbye.
> 
> Makes this a whole lot less dangerous.

Do you claim that it is now safe to mount (rw) and access filesystem
between suspend and resume?

> > > > So... for few years now suspend corrupts data on XFS? And Fedora has
> > > > the fix but it is not in mainline? That does not sound right...
> > > 
> > > The issues freezing the filesystem before the suspend image is
> > > created affect every journalled filesystem linux supports, be
> > > it XFS, ext4, reiser, btrfs, etc.
> > 
> > Did not it have some problems with ext3?
> 
> Please read more carefully: "affect every journalled filesystem
> linux supports". So, ext3 is affected because it's a journalling
> filesystem.  I didn't list every journalled filesystem Linux
> supports - "etc" means there are more examples that aren't
> explicitly listed.

Sorry for being unclear. My memory says this patch caused some
regression with ext3. Is my memory wrong? Or was the regression fixed
in the meantime?

(Clearly, ext3 has the same issues with journal replays as btrfs. But
note that mounting filesystem between suspend and resume is not safe
even on simple filesystems such as ext2).

Thanks,
								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] 37+ messages in thread

* Re: [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation
  2013-12-21 23:33                           ` Pavel Machek
@ 2013-12-23  3:48                             ` Dave Chinner
  0 siblings, 0 replies; 37+ messages in thread
From: Dave Chinner @ 2013-12-23  3:48 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Josh Boyer, Rafael J. Wysocki, Linux PM list, LKML, Jan Kara,
	linux-fsdevel, Nigel Cunningham, Srivatsa S. Bhat

On Sun, Dec 22, 2013 at 12:33:18AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > > > I disagree - given the problem it is resolving leads to silent
> > > > > > filesystem corruption, this patch should be considered somewhat of a
> > > > > > priority to push...
> > > > > 
> > > > > Umm. Ok, I forgot what it does, really.
> > > > 
> > > > It ensures that the filesystem is in an quiescent state both in
> > > > memory and on disk, and it cannot be modified in memory or on disk
> > > > whilst the suspend image is being generated, or by log recovery
> > > > after a resume before the suspended image has been restored.
> > > 
> > > If someone attempts to run log recovery before resume, that's a bug
> > > and yes, it will corrupt filesystems. (Including ext3). Don't do that.
> > 
> > Freezing the filesystem prevents that accidental mount of the
> > filesystem from being an issue. It fixes a bug that:
> 
> Can you elaborate on that?
> 
> If you do read-write mount of that filesystem, surely filesystem
> metadata will differ from what the filesystem expects. You'll still
> get data corruption AFAICT.

Only if you modify stuff. That's not what we are protecting against,
it's avoiding the automatic journal replay that you can't avoid if
you accidentally mount the filesystem.

> Read-only mount... maybe that will get slightly better -- there'll be
> no journal to play back. But what happens to superblock information
> such as "last mount time"? Mount counts?

If metadata is being modified on a read only mount outside of
journal replay, then the filesystem needs fixing.

> 
> > > Documentation/power/swsusp.txt:
> > > 
> > >  * BIG FAT WARNING
> > >    *********************************************************
> > >  *
> > >  * If you touch anything on disk between suspend and resume...
> > >  *                              ...kiss your data goodbye.
> > 
> > Makes this a whole lot less dangerous.
> 
> Do you claim that it is now safe to mount (rw) and access filesystem
> between suspend and resume?

No, I didn't claim that. "less dangerous" is still dangerous, just
less so than it was before.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2013-12-23  3:48 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-28 13:45 [RFC][PATCH] PM / Sleep: Freeze filesystems during system suspend/hibernation Rafael J. Wysocki
2012-01-28 21:23 ` Nigel Cunningham
2012-01-29 15:55 ` Martin Steigerwald
2012-01-29 19:53   ` Rafael J. Wysocki
2012-01-29 16:28 ` Srivatsa S. Bhat
2012-01-29 19:53   ` Rafael J. Wysocki
2012-01-30 23:24     ` Srivatsa S. Bhat
2012-01-30 20:00 ` Jan Kara
2012-01-30 21:05   ` Rafael J. Wysocki
2012-01-30 21:10     ` Nigel Cunningham
2012-01-31  0:03       ` Jan Kara
2012-01-30 23:58     ` Jan Kara
2012-02-01 13:36 ` Pavel Machek
2012-02-01 15:29   ` Alan Stern
2012-02-10  2:52     ` Jamie Lokier
2012-02-10  9:03       ` Jan Kara
2012-02-02  3:17 ` Nigel Cunningham
2012-02-17 15:41 ` Josh Boyer
2012-02-17 18:33   ` Josh Boyer
2012-02-17 20:59     ` Rafael J. Wysocki
2012-05-25 16:55       ` Josh Boyer
2012-05-25 19:13         ` Rafael J. Wysocki
2013-12-17 16:03           ` Josh Boyer
2013-12-17 16:04             ` Josh Boyer
2013-12-17 23:08               ` Pavel Machek
2013-12-17 23:31                 ` Dave Chinner
2013-12-18  0:01                   ` Pavel Machek
2013-12-18 12:39                     ` Dave Chinner
2013-12-18 14:08                       ` Pavel Machek
2013-12-19  0:22                         ` Dave Chinner
2013-12-21 23:33                           ` Pavel Machek
2013-12-23  3:48                             ` Dave Chinner
2013-12-18  0:52               ` Rafael J. Wysocki
2013-12-18  1:00                 ` Josh Boyer
2013-12-18  1:16                   ` Rafael J. Wysocki
2013-12-18 12:31                     ` Josh Boyer
2013-12-18 21:40                       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).