linux-kernel - [PATCH v2] PM: runtime: avoid priority inversion on PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211117183709.1832925-1-john@metanate.com>
Date:   Wed, 17 Nov 2021 18:37:09 +0000
From:   John Keeping <john@...anate.com>
To:     linux-pm@...r.kernel.org
Cc:     John Keeping <john@...anate.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Subject: [PATCH v2] PM: runtime: avoid priority inversion on PREEMPT_RT

With PREEMPT_RT the cpu_relax() loops in rpm_suspend and rpm_resume can
cause unbounded latency if they preempt an asynchronous suspend.  The
main scenario where this can happen is when a realtime thread resumes a
device while it is asynchronously suspending on a worker thread.

I'm not convinced this can actually happen in the rpm_suspend case, or
at least it's a lot less likely for a synchronous suspend to run at the
same time as an asynchronous suspend, but both functions are updated
here for symmetry.

For devices setting power.irq_safe, it is possible that RPM functions
will be called with a spinlock held (for example in
pl330_issue_pending()).  This means a normal call to schedule() can't be
used, but to avoid the priority inversion it is necessary to wait and
schedule.  schedule_rtlock() is only available when CONFIG_PREEMPT_RT is
defined, so even though the logic is correct without any preprocessor
guards around schedule_rtlock(), they are necessary for compilation.

Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Peter Zijlstra (Intel) <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...nel.org>
Signed-off-by: John Keeping <john@...anate.com>
---
Changes since v1:
- Use schedule_rtlock() instead of schedule() for PREEMPT_RT & irq_safe
- Rewritten commit description

 drivers/base/power/runtime.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index f3de7bfc7f5b..fdf461bfae8c 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -596,7 +596,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
 			goto out;
 		}
 
-		if (dev->power.irq_safe) {
+		if (dev->power.irq_safe && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			spin_unlock(&dev->power.lock);
 
 			cpu_relax();
@@ -614,7 +614,12 @@ static int rpm_suspend(struct device *dev, int rpmflags)
 
 			spin_unlock_irq(&dev->power.lock);
 
-			schedule();
+#ifdef CONFIG_PREEMPT_RT
+			if (dev->power.irq_safe)
+				schedule_rtlock();
+			else
+#endif
+				schedule();
 
 			spin_lock_irq(&dev->power.lock);
 		}
@@ -779,7 +784,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
 			goto out;
 		}
 
-		if (dev->power.irq_safe) {
+		if (dev->power.irq_safe && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
 			spin_unlock(&dev->power.lock);
 
 			cpu_relax();
@@ -798,7 +803,12 @@ static int rpm_resume(struct device *dev, int rpmflags)
 
 			spin_unlock_irq(&dev->power.lock);
 
-			schedule();
+#ifdef CONFIG_PREEMPT_RT
+			if (dev->power.irq_safe)
+				schedule_rtlock();
+			else
+#endif
+				schedule();
 
 			spin_lock_irq(&dev->power.lock);
 		}
-- 
2.34.0