[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202212141453.C79E68F@keescook>
Date: Wed, 14 Dec 2022 14:53:27 -0800
From: Kees Cook <keescook@...omium.org>
To: John Stultz <jstultz@...gle.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Wei Wang <wvw@...gle.com>,
Midas Chien <midaschieh@...gle.com>,
Connor O'Brien <connoro@...gle.com>,
Anton Vorontsov <anton@...msg.org>,
Colin Cross <ccross@...roid.com>,
Tony Luck <tony.luck@...el.com>, kernel-team@...roid.com
Subject: Re: [RFC PATCH] pstore: Switch pmsg_lock to an rt_mutex to avoid
priority inversion
On Mon, Dec 05, 2022 at 08:32:53PM +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.
>
> 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.
>
> Feedback would be appreciate!
This looks fine to me. Is there an appropriate "Fixes:" tag that could
be used?
-Kees
>
> Cc: Wei Wang <wvw@...gle.com>
> Cc: Midas Chien<midaschieh@...gle.com>
> Cc: Connor O'Brien <connoro@...gle.com>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Anton Vorontsov <anton@...msg.org>
> Cc: Colin Cross <ccross@...roid.com>
> Cc: Tony Luck <tony.luck@...el.com>
> Cc: kernel-team@...roid.com
> Reported-by: Wei Wang <wvw@...gle.com>
> Signed-off-by: John Stultz <jstultz@...gle.com>
> ---
> fs/pstore/pmsg.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> 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;
> }
>
> --
> 2.39.0.rc0.267.gcb52ba06e7-goog
>
--
Kees Cook
Powered by blists - more mailing lists