On Wed, Aug 03, 2022 at 11:57:27AM -0700, Linus Torvalds wrote:
>
> I really dislike this pattern:
>
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> preempt_disable();
> ...
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> preempt_enable();
>
> and while the new comment explains *why* it exists, it's still very ugly indeed.
>
> We have it in a couple of other places, and we also end up having
> another variation on the theme that is about "migrate_{dis,en}able()",
> except it is written as
>
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> migrate_disable();
> else
> preempt_disable();
>
> because on non-PREEMPT_RT obviously preempt_disable() is the better
> and simpler thing.
>
> Can we please just introduce helper functions?
>
> At least that
>
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> preempt_disable();
> ...
>
> pattern could be much more naturally expressed as
>
> preempt_disable_under_spinlock();
> ...
>
The original patch years ago use to have:
preempt_disable_rt()
preempt_enable_rt()
That did exactly that, but an effort was made to get rid of it. But your more
descriptive "preempt_enable/disable_under_spinlock()" may make more sense.
-- Steve
