PM / suspend / s2idle: avoiding splats on RT

PM / suspend / s2idle: avoiding splats on RT

[Sebastian Andrzej Siewior]

Patch #1 is a repost with the s2idle bits included. I tested via 
	echo s2idle > mem_sleep && echo mem > state 

and I woke up the machine via the power on button. Patches #2+ were
additionally required with s2idle.

Sebastian

[PATCH 1/4] PM / suspend: Prevent might sleep splats

[Sebastian Andrzej Siewior]

From: Thomas Gleixner <tglx@linutronix.de>

timekeeping suspend/resume calls read_persistent_clock() which takes
rtc_lock. That results in might sleep warnings because at that point
we run with interrupts disabled.

We cannot convert rtc_lock to a raw spinlock as that would trigger
other might sleep warnings.

As a workaround we disable the might sleep warnings by setting
system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
restoring it to SYSTEM_RUNNING afer sysdev_resume().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[bigeasy: cover s2idle]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/kernel.h   | 1 +
 kernel/power/hibernate.c | 7 +++++++
 kernel/power/suspend.c   | 6 ++++++
 3 files changed, 14 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 6a1eb0b0aad9..7aed92624531 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -542,6 +542,7 @@ extern enum system_states {
 	SYSTEM_HALT,
 	SYSTEM_POWER_OFF,
 	SYSTEM_RESTART,
+	SYSTEM_SUSPEND,
 } system_state;
 
 /* This cannot be an enum because some may be used in assembly source. */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 5454cc639a8d..9c85c7822383 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -287,6 +287,8 @@ static int create_image(int platform_mode)
 
 	local_irq_disable();
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (error) {
 		pr_err("Some system devices failed to power down, aborting hibernation\n");
@@ -317,6 +319,7 @@ static int create_image(int platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -445,6 +448,7 @@ static int resume_target_kernel(bool platform_mode)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 
 	error = syscore_suspend();
 	if (error)
@@ -478,6 +482,7 @@ static int resume_target_kernel(bool platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -563,6 +568,7 @@ int hibernation_platform_enter(void)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 	syscore_suspend();
 	if (pm_wakeup_pending()) {
 		error = -EAGAIN;
@@ -575,6 +581,7 @@ int hibernation_platform_enter(void)
 
  Power_up:
 	syscore_resume();
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 4c10be0f4843..9108a99878bf 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -110,6 +110,7 @@ static void s2idle_loop(void)
 {
 	pm_pr_dbg("suspend-to-idle\n");
 
+	system_state = SYSTEM_SUSPEND;
 	for (;;) {
 		int error;
 
@@ -148,6 +149,7 @@ static void s2idle_loop(void)
 
 		pm_wakeup_clear(false);
 	}
+	system_state = SYSTEM_RUNNING;
 
 	pm_pr_dbg("resume from suspend-to-idle\n");
 }
@@ -428,6 +430,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 	arch_suspend_disable_irqs();
 	BUG_ON(!irqs_disabled());
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (!error) {
 		*wakeup = pm_wakeup_pending();
@@ -443,6 +447,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 		syscore_resume();
 	}
 
+	system_state = SYSTEM_RUNNING;
+
 	arch_suspend_enable_irqs();
 	BUG_ON(irqs_disabled());
 
-- 
2.17.0

[PATCH 2/4] PM / wakeup: Make events_lock a RAW_SPINLOCK

[Sebastian Andrzej Siewior]

The `events_lock' is acquired during suspend while interrupts are
disabled even on RT. The lock is taken only for a very brief moment.
Make it a RAW lock which avoids "sleeping while atomic" warnings on RT.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/base/power/wakeup.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index ea01621ed769..2e76fbcba76e 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -57,7 +57,7 @@ static void split_counters(unsigned int *cnt, unsigned int *inpr)
 /* A preserved old value of the events counter. */
 static unsigned int saved_count;
 
-static DEFINE_SPINLOCK(events_lock);
+static DEFINE_RAW_SPINLOCK(events_lock);
 
 static void pm_wakeup_timer_fn(struct timer_list *t);
 
@@ -185,9 +185,9 @@ void wakeup_source_add(struct wakeup_source *ws)
 	ws->active = false;
 	ws->last_time = ktime_get();
 
-	spin_lock_irqsave(&events_lock, flags);
+	raw_spin_lock_irqsave(&events_lock, flags);
 	list_add_rcu(&ws->entry, &wakeup_sources);
-	spin_unlock_irqrestore(&events_lock, flags);
+	raw_spin_unlock_irqrestore(&events_lock, flags);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_add);
 
@@ -202,9 +202,9 @@ void wakeup_source_remove(struct wakeup_source *ws)
 	if (WARN_ON(!ws))
 		return;
 
-	spin_lock_irqsave(&events_lock, flags);
+	raw_spin_lock_irqsave(&events_lock, flags);
 	list_del_rcu(&ws->entry);
-	spin_unlock_irqrestore(&events_lock, flags);
+	raw_spin_unlock_irqrestore(&events_lock, flags);
 	synchronize_srcu(&wakeup_srcu);
 }
 EXPORT_SYMBOL_GPL(wakeup_source_remove);
@@ -843,7 +843,7 @@ bool pm_wakeup_pending(void)
 	unsigned long flags;
 	bool ret = false;
 
-	spin_lock_irqsave(&events_lock, flags);
+	raw_spin_lock_irqsave(&events_lock, flags);
 	if (events_check_enabled) {
 		unsigned int cnt, inpr;
 
@@ -851,7 +851,7 @@ bool pm_wakeup_pending(void)
 		ret = (cnt != saved_count || inpr > 0);
 		events_check_enabled = !ret;
 	}
-	spin_unlock_irqrestore(&events_lock, flags);
+	raw_spin_unlock_irqrestore(&events_lock, flags);
 
 	if (ret) {
 		pr_info("PM: Wakeup pending, aborting suspend\n");
@@ -940,13 +940,13 @@ bool pm_save_wakeup_count(unsigned int count)
 	unsigned long flags;
 
 	events_check_enabled = false;
-	spin_lock_irqsave(&events_lock, flags);
+	raw_spin_lock_irqsave(&events_lock, flags);
 	split_counters(&cnt, &inpr);
 	if (cnt == count && inpr == 0) {
 		saved_count = count;
 		events_check_enabled = true;
 	}
-	spin_unlock_irqrestore(&events_lock, flags);
+	raw_spin_unlock_irqrestore(&events_lock, flags);
 	return events_check_enabled;
 }
 
-- 
2.17.0

[PATCH 3/4] PM / s2idle: Make s2idle_wait_head swait based

[Sebastian Andrzej Siewior]

s2idle_wait_head is used during s2idle with interrupts disabled even on
RT. There is no "custom" wake up function so swait could be used instead
which is also lower weight compared to the wait_queue.
Make s2idle_wait_head a swait_queue_head.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/power/suspend.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 9108a99878bf..f865aa934835 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -27,6 +27,7 @@
 #include <linux/export.h>
 #include <linux/suspend.h>
 #include <linux/syscore_ops.h>
+#include <linux/swait.h>
 #include <linux/ftrace.h>
 #include <trace/events/power.h>
 #include <linux/compiler.h>
@@ -57,7 +58,7 @@ EXPORT_SYMBOL_GPL(pm_suspend_global_flags);
 
 static const struct platform_suspend_ops *suspend_ops;
 static const struct platform_s2idle_ops *s2idle_ops;
-static DECLARE_WAIT_QUEUE_HEAD(s2idle_wait_head);
+static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
 
 enum s2idle_states __read_mostly s2idle_state;
 static DEFINE_SPINLOCK(s2idle_lock);
@@ -91,8 +92,8 @@ static void s2idle_enter(void)
 	/* Push all the CPUs into the idle loop. */
 	wake_up_all_idle_cpus();
 	/* Make the current CPU wait so it can enter the idle loop too. */
-	wait_event(s2idle_wait_head,
-		   s2idle_state == S2IDLE_STATE_WAKE);
+	swait_event(s2idle_wait_head,
+		    s2idle_state == S2IDLE_STATE_WAKE);
 
 	cpuidle_pause();
 	put_online_cpus();
@@ -161,7 +162,7 @@ void s2idle_wake(void)
 	spin_lock_irqsave(&s2idle_lock, flags);
 	if (s2idle_state > S2IDLE_STATE_NONE) {
 		s2idle_state = S2IDLE_STATE_WAKE;
-		wake_up(&s2idle_wait_head);
+		swake_up(&s2idle_wait_head);
 	}
 	spin_unlock_irqrestore(&s2idle_lock, flags);
 }
-- 
2.17.0

[PATCH 4/4] PM / wakeup: Make s2idle_lock a RAW_SPINLOCK

[Sebastian Andrzej Siewior]

The `s2idle_lock' is acquired during suspend while interrupts are
disabled even on RT. The lock is acquired for short sections only.
Make it a RAW lock which avoids "sleeping while atomic" warnings on RT.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/power/suspend.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index f865aa934835..ca2abb5e5109 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -61,7 +61,7 @@ static const struct platform_s2idle_ops *s2idle_ops;
 static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
 
 enum s2idle_states __read_mostly s2idle_state;
-static DEFINE_SPINLOCK(s2idle_lock);
+static DEFINE_RAW_SPINLOCK(s2idle_lock);
 
 void s2idle_set_ops(const struct platform_s2idle_ops *ops)
 {
@@ -79,12 +79,12 @@ static void s2idle_enter(void)
 {
 	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_TO_IDLE, true);
 
-	spin_lock_irq(&s2idle_lock);
+	raw_spin_lock_irq(&s2idle_lock);
 	if (pm_wakeup_pending())
 		goto out;
 
 	s2idle_state = S2IDLE_STATE_ENTER;
-	spin_unlock_irq(&s2idle_lock);
+	raw_spin_unlock_irq(&s2idle_lock);
 
 	get_online_cpus();
 	cpuidle_resume();
@@ -98,11 +98,11 @@ static void s2idle_enter(void)
 	cpuidle_pause();
 	put_online_cpus();
 
-	spin_lock_irq(&s2idle_lock);
+	raw_spin_lock_irq(&s2idle_lock);
 
  out:
 	s2idle_state = S2IDLE_STATE_NONE;
-	spin_unlock_irq(&s2idle_lock);
+	raw_spin_unlock_irq(&s2idle_lock);
 
 	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_TO_IDLE, false);
 }
@@ -159,12 +159,12 @@ void s2idle_wake(void)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&s2idle_lock, flags);
+	raw_spin_lock_irqsave(&s2idle_lock, flags);
 	if (s2idle_state > S2IDLE_STATE_NONE) {
 		s2idle_state = S2IDLE_STATE_WAKE;
 		swake_up(&s2idle_wait_head);
 	}
-	spin_unlock_irqrestore(&s2idle_lock, flags);
+	raw_spin_unlock_irqrestore(&s2idle_lock, flags);
 }
 EXPORT_SYMBOL_GPL(s2idle_wake);
 
-- 
2.17.0

Re: [PATCH 1/4] PM / suspend: Prevent might sleep splats

[Rafael J. Wysocki]

On Fri, May 25, 2018 at 11:46 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> timekeeping suspend/resume calls read_persistent_clock() which takes
> rtc_lock. That results in might sleep warnings because at that point
> we run with interrupts disabled.
>
> We cannot convert rtc_lock to a raw spinlock as that would trigger
> other might sleep warnings.
>
> As a workaround we disable the might sleep warnings by setting
> system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
> restoring it to SYSTEM_RUNNING afer sysdev_resume().
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> [bigeasy: cover s2idle]
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/kernel.h   | 1 +
>  kernel/power/hibernate.c | 7 +++++++
>  kernel/power/suspend.c   | 6 ++++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 6a1eb0b0aad9..7aed92624531 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -542,6 +542,7 @@ extern enum system_states {
>         SYSTEM_HALT,
>         SYSTEM_POWER_OFF,
>         SYSTEM_RESTART,
> +       SYSTEM_SUSPEND,
>  } system_state;
>
>  /* This cannot be an enum because some may be used in assembly source. */
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 5454cc639a8d..9c85c7822383 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -287,6 +287,8 @@ static int create_image(int platform_mode)
>
>         local_irq_disable();
>
> +       system_state = SYSTEM_SUSPEND;
> +
>         error = syscore_suspend();
>         if (error) {
>                 pr_err("Some system devices failed to power down, aborting hibernation\n");
> @@ -317,6 +319,7 @@ static int create_image(int platform_mode)
>         syscore_resume();
>
>   Enable_irqs:
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> @@ -445,6 +448,7 @@ static int resume_target_kernel(bool platform_mode)
>                 goto Enable_cpus;
>
>         local_irq_disable();
> +       system_state = SYSTEM_SUSPEND;
>
>         error = syscore_suspend();
>         if (error)
> @@ -478,6 +482,7 @@ static int resume_target_kernel(bool platform_mode)
>         syscore_resume();
>
>   Enable_irqs:
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> @@ -563,6 +568,7 @@ int hibernation_platform_enter(void)
>                 goto Enable_cpus;
>
>         local_irq_disable();
> +       system_state = SYSTEM_SUSPEND;
>         syscore_suspend();
>         if (pm_wakeup_pending()) {
>                 error = -EAGAIN;
> @@ -575,6 +581,7 @@ int hibernation_platform_enter(void)
>
>   Power_up:
>         syscore_resume();
> +       system_state = SYSTEM_RUNNING;
>         local_irq_enable();
>
>   Enable_cpus:
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 4c10be0f4843..9108a99878bf 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -110,6 +110,7 @@ static void s2idle_loop(void)
>  {
>         pm_pr_dbg("suspend-to-idle\n");
>
> +       system_state = SYSTEM_SUSPEND;

And here's the ambiguity.

For the other sleep variants SYSTEM_SUSPEND means "all devices have
been suspended, interrupts are off and there's one CPU active".

Here, device suspend has not even completed entirely.

I guess you could set system_state to SYSTEM_SUSPEND right before
calling timekeeping_suspend() in tick_freeze() and then back to
SYSTEM_RUNNING after timekeeping_resume() has returned in
tick_unfreeze(), but talk to Thomas about that first. :-)

Re: PM / suspend / s2idle: avoiding splats on RT

[Rafael J. Wysocki]

On Fri, May 25, 2018 at 11:46 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> Patch #1 is a repost with the s2idle bits included. I tested via
>         echo s2idle > mem_sleep && echo mem > state
>
> and I woke up the machine via the power on button. Patches #2+ were
> additionally required with s2idle.

I quite don't like the first one for reasons explained in the reply to it.

The others are fine by me and I can queue them up if you want me to do that.

Thanks,
Rafael

Re: PM / suspend / s2idle: avoiding splats on RT

[Sebastian Andrzej Siewior]

On 2018-05-25 12:05:31 [+0200], Rafael J. Wysocki wrote:
> On Fri, May 25, 2018 at 11:46 AM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > Patch #1 is a repost with the s2idle bits included. I tested via
> >         echo s2idle > mem_sleep && echo mem > state
> >
> > and I woke up the machine via the power on button. Patches #2+ were
> > additionally required with s2idle.
> 
> I quite don't like the first one for reasons explained in the reply to it.
> 
> The others are fine by me and I can queue them up if you want me to do that.

Yes, please. I will talk to tglx regarding #1 and get back to you.

> Thanks,
> Rafael

Sebastian

Re: PM / suspend / s2idle: avoiding splats on RT

[Rafael J. Wysocki]

On Fri, May 25, 2018 at 12:12 PM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> On 2018-05-25 12:05:31 [+0200], Rafael J. Wysocki wrote:
>> On Fri, May 25, 2018 at 11:46 AM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>> > Patch #1 is a repost with the s2idle bits included. I tested via
>> >         echo s2idle > mem_sleep && echo mem > state
>> >
>> > and I woke up the machine via the power on button. Patches #2+ were
>> > additionally required with s2idle.
>>
>> I quite don't like the first one for reasons explained in the reply to it.
>>
>> The others are fine by me and I can queue them up if you want me to do that.
>
> Yes, please. I will talk to tglx regarding #1 and get back to you.

OK

[PATCH 1/4 v2] PM / suspend: Prevent might sleep splats

[Sebastian Andrzej Siewior]

From: Thomas Gleixner <tglx@linutronix.de>

timekeeping suspend/resume calls read_persistent_clock() which takes
rtc_lock. That results in might sleep warnings because at that point
we run with interrupts disabled.

We cannot convert rtc_lock to a raw spinlock as that would trigger
other might sleep warnings.

As a workaround we disable the might sleep warnings by setting
system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
restoring it to SYSTEM_RUNNING afer sysdev_resume(). There is no lock
contention because hibernate / suspend to RAM is single-CPU at this
point.

In s2idle's case the system_state is set to SYSTEM_SUSPEND before
timekeeping_suspend() which is invoked by the last CPU. In the resume
case it set back to SYSTEM_RUNNING after timekeeping_resume() which is
invoked by the first CPU in the resume case. The other CPUs will block
on tick_freeze_lock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[bigeasy: cover s2idle in tick_freeze() / tick_unfreeze()]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: Moved the s2idle case into tick_freeze()/unfreeze() as suggested
       by Rafael (tglx was okay with this).

 include/linux/kernel.h    | 1 +
 kernel/power/hibernate.c  | 7 +++++++
 kernel/power/suspend.c    | 4 ++++
 kernel/time/tick-common.c | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 6a1eb0b0aad9..7aed92624531 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -542,6 +542,7 @@ extern enum system_states {
 	SYSTEM_HALT,
 	SYSTEM_POWER_OFF,
 	SYSTEM_RESTART,
+	SYSTEM_SUSPEND,
 } system_state;
 
 /* This cannot be an enum because some may be used in assembly source. */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 5454cc639a8d..9c85c7822383 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -287,6 +287,8 @@ static int create_image(int platform_mode)
 
 	local_irq_disable();
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (error) {
 		pr_err("Some system devices failed to power down, aborting hibernation\n");
@@ -317,6 +319,7 @@ static int create_image(int platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -445,6 +448,7 @@ static int resume_target_kernel(bool platform_mode)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 
 	error = syscore_suspend();
 	if (error)
@@ -478,6 +482,7 @@ static int resume_target_kernel(bool platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -563,6 +568,7 @@ int hibernation_platform_enter(void)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 	syscore_suspend();
 	if (pm_wakeup_pending()) {
 		error = -EAGAIN;
@@ -575,6 +581,7 @@ int hibernation_platform_enter(void)
 
  Power_up:
 	syscore_resume();
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 4c10be0f4843..5149c77506b3 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -428,6 +428,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 	arch_suspend_disable_irqs();
 	BUG_ON(!irqs_disabled());
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (!error) {
 		*wakeup = pm_wakeup_pending();
@@ -443,6 +445,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 		syscore_resume();
 	}
 
+	system_state = SYSTEM_RUNNING;
+
 	arch_suspend_enable_irqs();
 	BUG_ON(irqs_disabled());
 
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 49edc1c4f3e6..14de3727b18e 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -490,6 +490,7 @@ void tick_freeze(void)
 	if (tick_freeze_depth == num_online_cpus()) {
 		trace_suspend_resume(TPS("timekeeping_freeze"),
 				     smp_processor_id(), true);
+		system_state = SYSTEM_SUSPEND;
 		timekeeping_suspend();
 	} else {
 		tick_suspend_local();
@@ -513,6 +514,7 @@ void tick_unfreeze(void)
 
 	if (tick_freeze_depth == num_online_cpus()) {
 		timekeeping_resume();
+		system_state = SYSTEM_RUNNING;
 		trace_suspend_resume(TPS("timekeeping_freeze"),
 				     smp_processor_id(), false);
 	} else {
-- 
2.17.0

Re: [PATCH 1/4 v2] PM / suspend: Prevent might sleep splats

[Rafael J. Wysocki]

On Friday, May 25, 2018 5:54:41 PM CEST Sebastian Andrzej Siewior wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> timekeeping suspend/resume calls read_persistent_clock() which takes
> rtc_lock. That results in might sleep warnings because at that point
> we run with interrupts disabled.
> 
> We cannot convert rtc_lock to a raw spinlock as that would trigger
> other might sleep warnings.
> 
> As a workaround we disable the might sleep warnings by setting
> system_state to SYSTEM_SUSPEND before calling sysdev_suspend() and
> restoring it to SYSTEM_RUNNING afer sysdev_resume(). There is no lock
> contention because hibernate / suspend to RAM is single-CPU at this
> point.
> 
> In s2idle's case the system_state is set to SYSTEM_SUSPEND before
> timekeeping_suspend() which is invoked by the last CPU. In the resume
> case it set back to SYSTEM_RUNNING after timekeeping_resume() which is
> invoked by the first CPU in the resume case. The other CPUs will block
> on tick_freeze_lock.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> [bigeasy: cover s2idle in tick_freeze() / tick_unfreeze()]
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2: Moved the s2idle case into tick_freeze()/unfreeze() as suggested
>        by Rafael (tglx was okay with this).
> 
>  include/linux/kernel.h    | 1 +
>  kernel/power/hibernate.c  | 7 +++++++
>  kernel/power/suspend.c    | 4 ++++
>  kernel/time/tick-common.c | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 6a1eb0b0aad9..7aed92624531 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -542,6 +542,7 @@ extern enum system_states {
>  	SYSTEM_HALT,
>  	SYSTEM_POWER_OFF,
>  	SYSTEM_RESTART,
> +	SYSTEM_SUSPEND,
>  } system_state;
>  
>  /* This cannot be an enum because some may be used in assembly source. */
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 5454cc639a8d..9c85c7822383 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -287,6 +287,8 @@ static int create_image(int platform_mode)
>  
>  	local_irq_disable();
>  
> +	system_state = SYSTEM_SUSPEND;
> +
>  	error = syscore_suspend();
>  	if (error) {
>  		pr_err("Some system devices failed to power down, aborting hibernation\n");
> @@ -317,6 +319,7 @@ static int create_image(int platform_mode)
>  	syscore_resume();
>  
>   Enable_irqs:
> +	system_state = SYSTEM_RUNNING;
>  	local_irq_enable();
>  
>   Enable_cpus:
> @@ -445,6 +448,7 @@ static int resume_target_kernel(bool platform_mode)
>  		goto Enable_cpus;
>  
>  	local_irq_disable();
> +	system_state = SYSTEM_SUSPEND;
>  
>  	error = syscore_suspend();
>  	if (error)
> @@ -478,6 +482,7 @@ static int resume_target_kernel(bool platform_mode)
>  	syscore_resume();
>  
>   Enable_irqs:
> +	system_state = SYSTEM_RUNNING;
>  	local_irq_enable();
>  
>   Enable_cpus:
> @@ -563,6 +568,7 @@ int hibernation_platform_enter(void)
>  		goto Enable_cpus;
>  
>  	local_irq_disable();
> +	system_state = SYSTEM_SUSPEND;
>  	syscore_suspend();
>  	if (pm_wakeup_pending()) {
>  		error = -EAGAIN;
> @@ -575,6 +581,7 @@ int hibernation_platform_enter(void)
>  
>   Power_up:
>  	syscore_resume();
> +	system_state = SYSTEM_RUNNING;
>  	local_irq_enable();
>  
>   Enable_cpus:
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 4c10be0f4843..5149c77506b3 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -428,6 +428,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
>  	arch_suspend_disable_irqs();
>  	BUG_ON(!irqs_disabled());
>  
> +	system_state = SYSTEM_SUSPEND;
> +
>  	error = syscore_suspend();
>  	if (!error) {
>  		*wakeup = pm_wakeup_pending();
> @@ -443,6 +445,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
>  		syscore_resume();
>  	}
>  
> +	system_state = SYSTEM_RUNNING;
> +
>  	arch_suspend_enable_irqs();
>  	BUG_ON(irqs_disabled());
>  
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 49edc1c4f3e6..14de3727b18e 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -490,6 +490,7 @@ void tick_freeze(void)
>  	if (tick_freeze_depth == num_online_cpus()) {
>  		trace_suspend_resume(TPS("timekeeping_freeze"),
>  				     smp_processor_id(), true);
> +		system_state = SYSTEM_SUSPEND;
>  		timekeeping_suspend();
>  	} else {
>  		tick_suspend_local();
> @@ -513,6 +514,7 @@ void tick_unfreeze(void)
>  
>  	if (tick_freeze_depth == num_online_cpus()) {
>  		timekeeping_resume();
> +		system_state = SYSTEM_RUNNING;
>  		trace_suspend_resume(TPS("timekeeping_freeze"),
>  				     smp_processor_id(), false);
>  	} else {
> 

Applied along with the rest of the series, thanks!