On Thu, Apr 25, 2024, Chen Yu wrote:
> Hi Sean,
>
> On 2024-03-12 at 12:39:11 -0700, Sean Christopherson wrote:
> > Use preempt_model_preemptible() to detect a preemptible kernel when
> > deciding whether or not to reschedule in order to drop a contended
> > spinlock or rwlock. Because PREEMPT_DYNAMIC selects PREEMPTION, kernels
>
> It took me a while to wonder why PREEMPT_DYNAMIC selects PREEMPTION
> in Kconfig, then I assume that you mean the static config is CONFIG_PREEMPTION,
> but the live preemption model is "none" or "voluntary", which makes the
> static check of CONFIG_PREEMPTION in spin_needbreak() and rwlock_needbreak()
> invalid?
Yep, exactly.
> > diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
> > index 3fcd20de6ca8..63dd8cf3c3c2 100644
> > --- a/include/linux/spinlock.h
> > +++ b/include/linux/spinlock.h
> > @@ -462,11 +462,10 @@ static __always_inline int spin_is_contended(spinlock_t *lock)
> > */
> > static inline int spin_needbreak(spinlock_t *lock)
> > {
> > -#ifdef CONFIG_PREEMPTION
> > + if (!preempt_model_preemptible())
>
> The old version checks against static CONFIG_PREEMPTION, now we check
> the live CONFIG_PREEMPTION and static CONFIG_PREEMPT_RT, just wonder
> if the rt check is needed here?
It's required, as CONFIG_PREEMPT_RT=y doesn't imply CONFIG_PREEMPT, and
CONFIG_PREEMPT_RT=y is mutually exclusive with CONFIG_PREEMPT_DYNAMIC. I.e. a
CONFIG_PREEMPT_RT=y kernel will look yield:
CONFIG_PREEMPT_RT=y
CONFIG_PREEMPT_DYNAMIC=n
CONFIG_PREEMPT=n
which in turn generates:
static inline bool preempt_model_full(void)
{
return IS_ENABLED(CONFIG_PREEMPT);
}
and so just checking preempt_model_full() would incorrectly return false for
CONFIG_PREEMPT_RT=y.
