[v2] pstore: Switch pmsg_lock to an rt_mutex to avoid priority inversion

Message ID 20221214231834.3711880-1-jstultz@google.com
State New
Headers
Series [v2] pstore: Switch pmsg_lock to an rt_mutex to avoid priority inversion |

Commit Message

John Stultz Dec. 14, 2022, 11:18 p.m. UTC
  Wei Wang reported seeing priority inversion caused latencies
caused by contention on pmsg_lock, and suggested it be switched
to a rt_mutex.

I was initially hesitant this would help, as the tasks in that
trace all seemed to be SCHED_NORMAL, so the benefit would be
limited to only nice boosting.

However, another similar issue was raised where the priority
inversion was seen did involve a blocked RT task so it is clear
this would be helpful in that case.

Cc: Wei Wang <wvw@google.com>
Cc: Midas Chien<midaschieh@google.com>
Cc: Connor O'Brien <connoro@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: kernel-team@android.com
Fixes: 9d5438f462ab ("pstore: Add pmsg - user-space accessible pstore object")
Reported-by: Wei Wang <wvw@google.com>
Signed-off-by: John Stultz <jstultz@google.com>
---
v2: Added Fixes tag
---
 fs/pstore/pmsg.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Comments

Kees Cook Dec. 15, 2022, 3:42 a.m. UTC | #1
On Wed, 14 Dec 2022 23:18:34 +0000, John Stultz wrote:
> Wei Wang reported seeing priority inversion caused latencies
> caused by contention on pmsg_lock, and suggested it be switched
> to a rt_mutex.
> 
> I was initially hesitant this would help, as the tasks in that
> trace all seemed to be SCHED_NORMAL, so the benefit would be
> limited to only nice boosting.
> 
> [...]

Applied to for-next/pstore, thanks!

[1/1] pstore: Switch pmsg_lock to an rt_mutex to avoid priority inversion
      https://git.kernel.org/kees/c/76d62f24db07
  

Patch

diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c
index d8542ec2f38c..18cf94b597e0 100644
--- a/fs/pstore/pmsg.c
+++ b/fs/pstore/pmsg.c
@@ -7,9 +7,10 @@ 
 #include <linux/device.h>
 #include <linux/fs.h>
 #include <linux/uaccess.h>
+#include <linux/rtmutex.h>
 #include "internal.h"
 
-static DEFINE_MUTEX(pmsg_lock);
+static DEFINE_RT_MUTEX(pmsg_lock);
 
 static ssize_t write_pmsg(struct file *file, const char __user *buf,
 			  size_t count, loff_t *ppos)
@@ -28,9 +29,9 @@  static ssize_t write_pmsg(struct file *file, const char __user *buf,
 	if (!access_ok(buf, count))
 		return -EFAULT;
 
-	mutex_lock(&pmsg_lock);
+	rt_mutex_lock(&pmsg_lock);
 	ret = psinfo->write_user(&record, buf);
-	mutex_unlock(&pmsg_lock);
+	rt_mutex_unlock(&pmsg_lock);
 	return ret ? ret : count;
 }