linux-kernel - Re: [PATCH] pstore: Revert pmsg

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20230302152103.2618f1b7@gandalf.local.home>
Date:   Thu, 2 Mar 2023 15:21:03 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     John Stultz <jstultz@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, Wei Wang <wvw@...gle.com>,
        Midas Chien <midaschieh@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Anton Vorontsov <anton@...msg.org>,
        "Guilherme G. Piccoli" <gpiccoli@...lia.com>,
        Tony Luck <tony.luck@...el.com>, kernel-team@...roid.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH] pstore: Revert pmsg_lock back to a normal mutex

On Thu, 2 Mar 2023 11:39:12 -0800
John Stultz <jstultz@...gle.com> wrote:

> This sounds like something to try as well (though I'd still want to
> push this revert in the meantime to avoid regressions).
> 
> Do you have a more specific pointer to this adaptive spinning
> rt_mutexes for spinlocks? Looking at the current PREEMPT_RT patch I'm
> not seeing any changes to locking.

Actually, it's in mainline too.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/locking/rtmutex.c#n1557

The rtmutex_spin_on_owner() is the code.

But currently, only the highest priority waiter will spin. If you have two
waiters, the second one will sleep.

With today's computers, where it's not uncommon to have more than 16 CPUs,
it is very likely (and probably what happened in your case), that there's
more than one waiter in contention. I'm thinking all waiters should spin if
the owner and the top waiter are also running. But only the top waiter
should be allowed to grab the lock.

There's no harm in spinning, as the task can still be preempted, and
there's no issue of priority inversion, because the spinners will not be on
the same CPU as the owner and the top waiter, if they only spin if those
two tasks are also running on a CPU.

I could possibly add a patch, and see if that also works.

-- Steve