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