linux-kernel - Re: [git pull] vfs.git pile 3

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: <20220803213255.3ab719e3@gandalf.local.home>
Date:   Wed, 3 Aug 2022 21:32:55 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] vfs.git pile 3 - dcache

On Thu, 4 Aug 2022 01:42:25 +0100
Matthew Wilcox <willy@...radead.org> wrote:

> > So let's make it verbose and clear and unambiguous. It's not like I
> > expect to see a _lot_ of those. Knock wood.  
> 
> Should we have it take a spinlock_t pointer?  We could have lockdep
> check it is actually held.

We don't care if the lock is held or not. The point of the matter is that
spinlocks in RT do not disable preemption. The code that the
preempt_disable_under_spinlock() is inside, can not be preempted. If it is,
bad things can happen.

Currently this code assumes that spinlocks disable preemption, so there's
no need to disable preemption here. But in RT, just holding the spinlock is
not enough to disable preemption, hence we need to explicitly call it here.

As Linus's name suggests, the "preempt_enable_under_spinlock" is to make
sure preemption is disabled regardless if it's under a normal spinlock that
disables preemption, or a RT spinlock that does not.

I wonder if raw_preempt_disable() would be another name to use? We have
raw_spin_lock() to denote that it's a real spinlock even under PREEMPT_RT.
We could say that "raw_preempt_disable()" makes sure the location really
has preemption disabled regardless of PREEMPT_RT.

-- Steve