Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 3 Aug 2022 21:32:55 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [git pull] vfs.git pile 3 - dcache
Message-ID: <20220803213255.3ab719e3@gandalf.local.home>
In-Reply-To: <YusV8cr382PeBNLM@casper.infradead.org>
References: <YurA3aSb4GRr4wlW@ZenIV>
        <CAHk-=wizUgMbZKnOjvyeZT5E+WZM0sV+zS5Qxt84wp=BsRk3eQ@mail.gmail.com>
        <YuruqoGHJONpdZcK@home.goodmis.org>
        <CAHk-=whJvgykcTnR+BMJNwd+me5wvg+CxjSBeiPYTR1B2g5NpQ@mail.gmail.com>
        <20220803185936.228dc690@gandalf.local.home>
        <YusDmF39ykDmfSkF@casper.infradead.org>
        <CAHk-=wh6VSqsnANHkQpw=yD-Hkt90Y1LX=ad9+r+SusfriUOfA@mail.gmail.com>
        <YusV8cr382PeBNLM@casper.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 4 Aug 2022 01:42:25 +0100
Matthew Wilcox <willy@infradead.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