Messages in this thread | | | Date | Wed, 3 Aug 2022 17:54:50 -0400 | From | Steven Rostedt <> | Subject | Re: [git pull] vfs.git pile 3 - dcache |
| |
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
| |