Message-ID: <ebfe3ca1-2f48-42ce-9d03-513a8c1bc530@paulmck-laptop>
Date:   Wed, 18 Oct 2023 13:42:21 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Ankur Arora <ankur.a.arora@...cle.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
        akpm@...ux-foundation.org, luto@...nel.org, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com, mingo@...hat.com,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        willy@...radead.org, mgorman@...e.de, rostedt@...dmis.org,
        jon.grimm@....com, bharata@....com, raghavendra.kt@....com,
        boris.ostrovsky@...cle.com, konrad.wilk@...cle.com,
        jgross@...e.com, andrew.cooper3@...rix.com,
        Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH v2 7/9] sched: define TIF_ALLOW_RESCHED

On Wed, Oct 18, 2023 at 01:15:28PM -0700, Ankur Arora wrote:
> 
> Paul E. McKenney <paulmck@...nel.org> writes:
> 
> > On Wed, Oct 18, 2023 at 03:16:12PM +0200, Thomas Gleixner wrote:
> >> Paul!
> >>
> >> On Tue, Oct 17 2023 at 18:03, Paul E. McKenney wrote:
> >> > Belatedly calling out some RCU issues.  Nothing fatal, just a
> >> > (surprisingly) few adjustments that will need to be made.  The key thing
> >> > to note is that from RCU's viewpoint, with this change, all kernels
> >> > are preemptible, though rcu_read_lock() readers remain
> >> > non-preemptible.
> >>
> >> Why? Either I'm confused or you or both of us :)
> >
> > Isn't rcu_read_lock() defined as preempt_disable() and rcu_read_unlock()
> > as preempt_enable() in this approach?  I certainly hope so, as RCU
> > priority boosting would be a most unwelcome addition to many datacenter
> > workloads.
> 
> No, in this approach, PREEMPT_AUTO selects PREEMPTION and thus
> PREEMPT_RCU so rcu_read_lock/unlock() would touch the
> rcu_read_lock_nesting.  Which is identical to what PREEMPT_DYNAMIC does.

Understood.  And we need some way to build a kernel such that RCU
read-side critical sections are non-preemptible.  This is a hard
requirement that is not going away anytime soon.

> >> With this approach the kernel is by definition fully preemptible, which
> >> means means rcu_read_lock() is preemptible too. That's pretty much the
> >> same situation as with PREEMPT_DYNAMIC.
> >
> > Please, just no!!!
> >
> > Please note that the current use of PREEMPT_DYNAMIC with preempt=none
> > avoids preempting RCU read-side critical sections.  This means that the
> > distro use of PREEMPT_DYNAMIC has most definitely *not* tested preemption
> > of RCU readers in environments expecting no preemption.
> 
> Ah. So, though PREEMPT_DYNAMIC with preempt=none runs with PREEMPT_RCU,
> preempt=none stubs out the actual preemption via __preempt_schedule.
> 
> Okay, I see what you are saying.

More to the point, currently, you can build with CONFIG_PREEMPT_DYNAMIC=n
and CONFIG_PREEMPT_NONE=y and have non-preemptible RCU read-side critical
sections.

> (Side issue: but this means that even for PREEMPT_DYNAMIC preempt=none,
> _cond_resched() doesn't call rcu_all_qs().)

I have no idea if anyone runs with CONFIG_PREEMPT_DYNAMIC=y and
preempt=none.  We don't do so.  ;-)

> >> For throughput sake this fully preemptible kernel provides a mechanism
> >> to delay preemption for SCHED_OTHER tasks, i.e. instead of setting
> >> NEED_RESCHED the scheduler sets NEED_RESCHED_LAZY.
> >>
> >> That means the preemption points in preempt_enable() and return from
> >> interrupt to kernel will not see NEED_RESCHED and the tasks can run to
> >> completion either to the point where they call schedule() or when they
> >> return to user space. That's pretty much what PREEMPT_NONE does today.
> >>
> >> The difference to NONE/VOLUNTARY is that the explicit cond_resched()
> >> points are not longer required because the scheduler can preempt the
> >> long running task by setting NEED_RESCHED instead.
> >>
> >> That preemption might be suboptimal in some cases compared to
> >> cond_resched(), but from my initial experimentation that's not really an
> >> issue.
> >
> > I am not (repeat NOT) arguing for keeping cond_resched().  I am instead
> > arguing that the less-preemptible variants of the kernel should continue
> > to avoid preempting RCU read-side critical sections.
> 
> [ snip ]
> 
> >> In the end there is no CONFIG_PREEMPT_XXX anymore. The only knob
> >> remaining would be CONFIG_PREEMPT_RT, which should be renamed to
> >> CONFIG_RT or such as it does not really change the preemption
> >> model itself. RT just reduces the preemption disabled sections with the
> >> lock conversions, forced interrupt threading and some more.
> >
> > Again, please, no.
> >
> > There are situations where we still need rcu_read_lock() and
> > rcu_read_unlock() to be preempt_disable() and preempt_enable(),
> > repectively.  Those can be cases selected only by Kconfig option, not
> > available in kernels compiled with CONFIG_PREEMPT_DYNAMIC=y.
> 
> As far as non-preemptible RCU read-side critical sections are concerned,
> are the current
> - PREEMPT_DYNAMIC=y, PREEMPT_RCU, preempt=none config
>   (rcu_read_lock/unlock() do not manipulate preempt_count, but do
>    stub out preempt_schedule())
> - and PREEMPT_NONE=y, TREE_RCU config (rcu_read_lock/unlock() manipulate
>    preempt_count)?
> 
> roughly similar or no?

No.

There is still considerable exposure to preemptible-RCU code paths,
for example, when current->rcu_read_unlock_special.b.blocked is set.

> >> > I am sure that I am missing something, but I have not yet seen any
> >> > show-stoppers.  Just some needed adjustments.
> >>
> >> Right. If it works out as I think it can work out the main adjustments
> >> are to remove a large amount of #ifdef maze and related gunk :)
> >
> > Just please don't remove the #ifdef gunk that is still needed!
> 
> Always the hard part :).

Hey, we wouldn't want to insult your intelligence by letting you work
on too easy of a problem!  ;-)

						Thanx, Paul

Powered by blists - more mailing lists