On Fri, Nov 24, 2023 at 01:22:19PM -0800, Linus Torvalds wrote:
> On Thu, 23 Nov 2023 at 22:04, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > ->d_lock on parent does not stabilize ->d_inode of child.
> > We don't do much with that inode in there, but we need
> > at least to avoid struct inode getting freed under us...
>
> Gaah. We've gone back and forth on this. Being non-preemptible is
> already equivalent to rcu read locking.
>
> >From Documentation/RCU/rcu_dereference.rst:
>
> With the new consolidated
> RCU flavors, an RCU read-side critical section is entered
> using rcu_read_lock(), anything that disables bottom halves,
> anything that disables interrupts, or anything that disables
> preemption.
>
> so I actually think the coda code is already mostly fine, because that
> parent spin_lock may not stabilize d_child per se, but it *does* imply
> a RCU read lock.
>
> So I think you should drop the rcu_read_lock/rcu_read_unlock from that patch.
>
> But that
>
> struct inode *inode = d_inode_rcu(de);
>
> conversion is required to get a stable inode pointer.
>
> So half of this patch is unnecessary.
>
> Adding Paul to the cc just to verify that the docs are up-to-date and
> that we're still good here.
>
> Because we've gone back-and-forth on the "spinlocks are an implied RCU
> read-side critical section" a couple of times.
Yes, spinlocks are implied RCU read-side critical sections. Even in -rt,
where non-raw spinlocks are preemptible, courtesy of this:
static __always_inline void __rt_spin_lock(spinlock_t *lock)
{
rtlock_might_resched();
rtlock_lock(&lock->lock);
rcu_read_lock();
migrate_disable();
}
So given -rt's preemptible spinlocks still being RCU readers, I need to
explicitly call this out in the documentation.
How about as shown below for a start?
Thanx, Paul
------------------------------------------------------------------------
diff --git a/Documentation/RCU/rcu_dereference.rst b/Documentation/RCU/rcu_dereference.rst
index 659d5913784d..2524dcdadde2 100644
--- a/Documentation/RCU/rcu_dereference.rst
+++ b/Documentation/RCU/rcu_dereference.rst
@@ -408,7 +408,10 @@ member of the rcu_dereference() to use in various situations:
RCU flavors, an RCU read-side critical section is entered
using rcu_read_lock(), anything that disables bottom halves,
anything that disables interrupts, or anything that disables
- preemption.
+ preemption. Please note that spinlock critical sections
+ are also implied RCU read-side critical sections, even when
+ they are preemptible, as they are in kernels built with
+ CONFIG_PREEMPT_RT=y.
2. If the access might be within an RCU read-side critical section
on the one hand, or protected by (say) my_lock on the other,
