Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend
@ 2019-12-02 17:05 Jonas Meurer
  2019-12-02 17:07 ` [PATCH 1/2] PM: Add a switch for disabling/enabling sync() before, suspend Jonas Meurer
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jonas Meurer @ 2019-12-02 17:05 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Tim Dittler,
	Yannik Sembritzki

Hello,

Introduce a new run-time sysfs switch to disable/enable sync() before
system suspend. This is useful to avoid races and deadlocks if block
devices have been suspended before, e.g. by 'cryptsetup luksSuspend'.

The second patch changes the behaviour of build-time switch
'CONFIG_SUSPEND_SKIP_SYNC' accordingly, using the build-time switch value
as default for our new run-time switch '/sys/power/sync_on_suspend'.

Jonas Meurer (2):
  PM: Add a switch for disabling/enabling sync() before suspend
  PM: CONFIG_SUSPEND_SKIP_SYNC sets default for '/sys/power/sync_on_suspend'

 Documentation/ABI/testing/sysfs-power | 15 +++++++++++++++
 include/linux/suspend.h               |  2 ++
 kernel/power/Kconfig                  |  5 ++++-
 kernel/power/main.c                   | 33 +++++++++++++++++++++++++++++++++
 kernel/power/suspend.c                |  2 +-
 5 files changed, 55 insertions(+), 2 deletions(-)

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

* [PATCH 1/2] PM: Add a switch for disabling/enabling sync() before, suspend
  2019-12-02 17:05 [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
@ 2019-12-02 17:07 ` Jonas Meurer
  2019-12-02 17:07 ` [PATCH 2/2] PM: CONFIG_SUSPEND_SKIP_SYNC sets default for, '/sys/power/sync_on_suspend' Jonas Meurer
  2019-12-11 15:59 ` [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
  2 siblings, 0 replies; 5+ messages in thread
From: Jonas Meurer @ 2019-12-02 17:07 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Tim Dittler,
	Yannik Sembritzki

The switch allows to enable or disable the final sync() from the suspend.c
Linux Kernel system suspend implementation. This is useful to avoid race
conditions if block devices have been suspended before. Be aware that you
have to take care of sync() yourself before suspending the system if you
disable it here.

Signed-off-by: Jonas Meurer <jonas@freesources.org>
---
 Documentation/ABI/testing/sysfs-power | 14 ++++++++++++
 include/linux/suspend.h               |  2 ++
 kernel/power/main.c                   | 33 +++++++++++++++++++++++++++
 kernel/power/suspend.c                |  2 +-
 4 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
index 6f87b9dd384b..f164a364e89a 100644
--- a/Documentation/ABI/testing/sysfs-power
+++ b/Documentation/ABI/testing/sysfs-power
@@ -407,3 +407,17 @@ Contact:	Kalesh Singh <kaleshsingh96@gmail.com>
 Description:
 		The /sys/power/suspend_stats/last_failed_step file contains
 		the last failed step in the suspend/resume path.
+
+What:		/sys/power/sync_on_suspend
+Date:		October 2019
+Contact:	Jonas Meurer <jonas@freesources.org>
+Description:
+		This file controls the switch to enable or disable the final
+		sync() before system suspend. This is useful to avoid race
+		conditions if block devices have been suspended before. Be
+		aware that you have to take care of sync() yourself before
+		suspending the system if you disable it here.
+
+		Writing a "1" (default) to this file enables the sync() and
+		writing a "0" disables it. Reads from the file return the
+		current value.
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 6fc8843f1c9e..4a230c2f1c31 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -329,6 +329,7 @@ extern void arch_suspend_disable_irqs(void);
 extern void arch_suspend_enable_irqs(void);
 
 extern int pm_suspend(suspend_state_t state);
+extern bool sync_on_suspend_enabled;
 #else /* !CONFIG_SUSPEND */
 #define suspend_valid_only_mem	NULL
 
@@ -342,6 +343,7 @@ static inline bool pm_suspend_default_s2idle(void) { return false; }
 
 static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
 static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
+static inline bool sync_on_suspend_enabled(void) { return true; }
 static inline bool idle_should_enter_s2idle(void) { return false; }
 static inline void __init pm_states_init(void) {}
 static inline void s2idle_set_ops(const struct platform_s2idle_ops *ops) {}
diff --git a/kernel/power/main.c b/kernel/power/main.c
index e26de7af520b..7d9d579a8e6f 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -190,6 +190,38 @@ static ssize_t mem_sleep_store(struct kobject *kobj, struct kobj_attribute *attr
 }
 
 power_attr(mem_sleep);
+
+/*
+ * sync_on_suspend: invoke ksys_sync_helper() before suspend.
+ *
+ * show() returns whether ksys_sync_helper() is invoked before suspend.
+ * store() accepts 0 or 1.  0 disables ksys_sync_helper() and 1 enables it.
+ */
+bool sync_on_suspend_enabled = true;
+
+static ssize_t sync_on_suspend_show(struct kobject *kobj,
+				   struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", sync_on_suspend_enabled);
+}
+
+static ssize_t sync_on_suspend_store(struct kobject *kobj,
+				    struct kobj_attribute *attr,
+				    const char *buf, size_t n)
+{
+	unsigned long val;
+
+	if (kstrtoul(buf, 10, &val))
+		return -EINVAL;
+
+	if (val > 1)
+		return -EINVAL;
+
+	sync_on_suspend_enabled = !!val;
+	return n;
+}
+
+power_attr(sync_on_suspend);
 #endif /* CONFIG_SUSPEND */
 
 #ifdef CONFIG_PM_SLEEP_DEBUG
@@ -855,6 +887,7 @@ static struct attribute * g[] = {
 	&wakeup_count_attr.attr,
 #ifdef CONFIG_SUSPEND
 	&mem_sleep_attr.attr,
+	&sync_on_suspend_attr.attr,
 #endif
 #ifdef CONFIG_PM_AUTOSLEEP
 	&autosleep_attr.attr,
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index f3b7239f1892..503d56419a69 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -564,7 +564,7 @@ static int enter_state(suspend_state_t state)
 	if (state == PM_SUSPEND_TO_IDLE)
 		s2idle_begin();
 
-	if (!IS_ENABLED(CONFIG_SUSPEND_SKIP_SYNC)) {
+	if (!IS_ENABLED(CONFIG_SUSPEND_SKIP_SYNC) && sync_on_suspend_enabled) {
 		trace_suspend_resume(TPS("sync_filesystems"), 0, true);
 		ksys_sync_helper();
 		trace_suspend_resume(TPS("sync_filesystems"), 0, false);
-- 
2.20.1

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

* [PATCH 2/2] PM: CONFIG_SUSPEND_SKIP_SYNC sets default for, '/sys/power/sync_on_suspend'
  2019-12-02 17:05 [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
  2019-12-02 17:07 ` [PATCH 1/2] PM: Add a switch for disabling/enabling sync() before, suspend Jonas Meurer
@ 2019-12-02 17:07 ` Jonas Meurer
  2019-12-11 15:59 ` [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
  2 siblings, 0 replies; 5+ messages in thread
From: Jonas Meurer @ 2019-12-02 17:07 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Tim Dittler,
	Yannik Sembritzki

Slightly change the behaviour of build-time switch CONFIG_SUSPEND_SKIP_SYNC:
Make it configure the default for '/sys/power/sync_on_suspend', now that we
have a run-time switch for it.

Signed-off-by: Jonas Meurer <jonas@freesources.org>
---
 Documentation/ABI/testing/sysfs-power | 7 ++++---
 kernel/power/Kconfig                  | 5 ++++-
 kernel/power/main.c                   | 2 +-
 kernel/power/suspend.c                | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
index f164a364e89a..783b3ce8eb06 100644
--- a/Documentation/ABI/testing/sysfs-power
+++ b/Documentation/ABI/testing/sysfs-power
@@ -418,6 +418,7 @@ Description:
 		aware that you have to take care of sync() yourself before
 		suspending the system if you disable it here.
 
-		Writing a "1" (default) to this file enables the sync() and
-		writing a "0" disables it. Reads from the file return the
-		current value.
+		Writing a "1" to this file enables the sync() and writing a
+		"0" disables it. Reads from the file return the current value.
+		The default is "1" but can be configured with the build-time
+		config flag "SUSPEND_SKIP_SYNC".
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index d3667b4075c1..7cbfbeacd68a 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -27,7 +27,10 @@ config SUSPEND_SKIP_SYNC
 	  Skip the kernel sys_sync() before freezing user processes.
 	  Some systems prefer not to pay this cost on every invocation
 	  of suspend, or they are content with invoking sync() from
-	  user-space before invoking suspend.  Say Y if that's your case.
+	  user-space before invoking suspend.  There's a run-time switch
+	  at '/sys/power/sync_on_suspend' to configure this behaviour.
+	  This setting changes the default for the run-tim switch. Say Y
+	  to change the default to disable the kernel sys_sync().
 
 config HIBERNATE_CALLBACKS
 	bool
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 7d9d579a8e6f..69b7a8aeca3b 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -197,7 +197,7 @@ power_attr(mem_sleep);
  * show() returns whether ksys_sync_helper() is invoked before suspend.
  * store() accepts 0 or 1.  0 disables ksys_sync_helper() and 1 enables it.
  */
-bool sync_on_suspend_enabled = true;
+bool sync_on_suspend_enabled = !IS_ENABLED(CONFIG_SUSPEND_SKIP_SYNC);
 
 static ssize_t sync_on_suspend_show(struct kobject *kobj,
 				   struct kobj_attribute *attr, char *buf)
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 503d56419a69..2c47280fbfc7 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -564,7 +564,7 @@ static int enter_state(suspend_state_t state)
 	if (state == PM_SUSPEND_TO_IDLE)
 		s2idle_begin();
 
-	if (!IS_ENABLED(CONFIG_SUSPEND_SKIP_SYNC) && sync_on_suspend_enabled) {
+	if (sync_on_suspend_enabled) {
 		trace_suspend_resume(TPS("sync_filesystems"), 0, true);
 		ksys_sync_helper();
 		trace_suspend_resume(TPS("sync_filesystems"), 0, false);
-- 
2.20.1


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

* Re: [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend
  2019-12-02 17:05 [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
  2019-12-02 17:07 ` [PATCH 1/2] PM: Add a switch for disabling/enabling sync() before, suspend Jonas Meurer
  2019-12-02 17:07 ` [PATCH 2/2] PM: CONFIG_SUSPEND_SKIP_SYNC sets default for, '/sys/power/sync_on_suspend' Jonas Meurer
@ 2019-12-11 15:59 ` Jonas Meurer
  2019-12-12 18:00   ` Rafael J. Wysocki
  2 siblings, 1 reply; 5+ messages in thread
From: Jonas Meurer @ 2019-12-11 15:59 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Tim Dittler,
	Yannik Sembritzki

Hello,

Jonas Meurer:
> Hello,
> 
> Introduce a new run-time sysfs switch to disable/enable sync() before
> system suspend. This is useful to avoid races and deadlocks if block
> devices have been suspended before, e.g. by 'cryptsetup luksSuspend'.
> 
> The second patch changes the behaviour of build-time switch
> 'CONFIG_SUSPEND_SKIP_SYNC' accordingly, using the build-time switch value
> as default for our new run-time switch '/sys/power/sync_on_suspend'.
> 
> Jonas Meurer (2):
>   PM: Add a switch for disabling/enabling sync() before suspend
>   PM: CONFIG_SUSPEND_SKIP_SYNC sets default for '/sys/power/sync_on_suspend'
> 
>  Documentation/ABI/testing/sysfs-power | 15 +++++++++++++++
>  include/linux/suspend.h               |  2 ++
>  kernel/power/Kconfig                  |  5 ++++-
>  kernel/power/main.c                   | 33 +++++++++++++++++++++++++++++++++
>  kernel/power/suspend.c                |  2 +-
>  5 files changed, 55 insertions(+), 2 deletions(-)

Any chance to get a review/comment on this patch? What's the next
logical steps to get it merged?

Cheers
 jonas


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

* Re: [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend
  2019-12-11 15:59 ` [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
@ 2019-12-12 18:00   ` Rafael J. Wysocki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2019-12-12 18:00 UTC (permalink / raw)
  To: Jonas Meurer
  Cc: linux-pm, linux-kernel, Pavel Machek, Len Brown, Tim Dittler,
	Yannik Sembritzki

On Wednesday, December 11, 2019 4:59:29 PM CET Jonas Meurer wrote:
> Hello,
> 
> Jonas Meurer:
> > Hello,
> > 
> > Introduce a new run-time sysfs switch to disable/enable sync() before
> > system suspend. This is useful to avoid races and deadlocks if block
> > devices have been suspended before, e.g. by 'cryptsetup luksSuspend'.
> > 
> > The second patch changes the behaviour of build-time switch
> > 'CONFIG_SUSPEND_SKIP_SYNC' accordingly, using the build-time switch value
> > as default for our new run-time switch '/sys/power/sync_on_suspend'.
> > 
> > Jonas Meurer (2):
> >   PM: Add a switch for disabling/enabling sync() before suspend
> >   PM: CONFIG_SUSPEND_SKIP_SYNC sets default for '/sys/power/sync_on_suspend'
> > 
> >  Documentation/ABI/testing/sysfs-power | 15 +++++++++++++++
> >  include/linux/suspend.h               |  2 ++
> >  kernel/power/Kconfig                  |  5 ++++-
> >  kernel/power/main.c                   | 33 +++++++++++++++++++++++++++++++++
> >  kernel/power/suspend.c                |  2 +-
> >  5 files changed, 55 insertions(+), 2 deletions(-)
> 
> Any chance to get a review/comment on this patch? What's the next
> logical steps to get it merged?

This is not a super-high priority patchset IMO, but it is in my list of things
to look at.

Thanks!




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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 17:05 [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
2019-12-02 17:07 ` [PATCH 1/2] PM: Add a switch for disabling/enabling sync() before, suspend Jonas Meurer
2019-12-02 17:07 ` [PATCH 2/2] PM: CONFIG_SUSPEND_SKIP_SYNC sets default for, '/sys/power/sync_on_suspend' Jonas Meurer
2019-12-11 15:59 ` [PATCH 0/2] PM: Add a switch for disabling/enabling sync() before suspend Jonas Meurer
2019-12-12 18:00   ` Rafael J. Wysocki

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git