Re: [RFC PATCH 47/86] rcu: select PREEMPT_RCU if PREEMPT

[Paul E. McKenney]

On Mon, Nov 20, 2023 at 10:43:56PM -0500, Steven Rostedt wrote:
> On Mon, 20 Nov 2023 16:28:50 -0800
> "Paul E. McKenney" <paulmck@xxxxxxxxxx> wrote:
> 
> > On Tue, Nov 07, 2023 at 07:27:03PM -0500, Steven Rostedt wrote:
> > > On Tue,  7 Nov 2023 13:57:33 -0800
> > > Ankur Arora <ankur.a.arora@xxxxxxxxxx> wrote:
> > >   
> > > > With PREEMPTION being always-on, some configurations might prefer
> > > > the stronger forward-progress guarantees provided by PREEMPT_RCU=n
> > > > as compared to PREEMPT_RCU=y.
> > > > 
> > > > So, select PREEMPT_RCU=n for PREEMPT_VOLUNTARY and PREEMPT_NONE and
> > > > enabling PREEMPT_RCU=y for PREEMPT or PREEMPT_RT.
> > > > 
> > > > Note that the preemption model can be changed at runtime (modulo
> > > > configurations with ARCH_NO_PREEMPT), but the RCU configuration
> > > > is statically compiled.  
> > > 
> > > I wonder if we should make this a separate patch, and allow PREEMPT_RCU=n
> > > when PREEMPT=y?  
> > 
> > You mean independent of this series?  If so, I am not all that excited
> > about allowing a new option due to the effect on testing.  With this full
> > series, the number of test scenarios is preserved.
> > 
> > Actually, that is not exactly true, is it?  It would be if we instead had
> > something like this:
> > 
> > config PREEMPT_RCU
> > 	bool
> > 	default y if PREEMPT || PREEMPT_RT
> > 	depends on !PREEMPT_NONE && !PREEMPT_VOLUNTARY
> > 	select TREE_RCU
> > 
> > Any reason why this would be a problem?
> 
> Yes, because with this series, there isn't going to be PREEMPT_NONE,
> PREEMPT_VOLUNTARY and PREEMPT as a config option. I mean, you could define
> the preference you want at boot up. But it could change at run time.

I applied the series, and there was still a PREEMPT_NONE.  Some might
consider the name to be a bit misleading, perhaps, but it was still there.

Ah, I missed patch 30/86.  The idea is to make CONFIG_PREEMPT_DYNAMIC
unconditional?  Why?  

> > Or to put it another way, do you know of anyone who really wants
> > a preemptible kernel (CONFIG_PREEMPT=y, CONFIG_PREEMPT_NONE=n
> > and CONFIG_PREEMPT_VOLUNTARY=n) but also non-preemptible RCU
> > (CONFIG_PREEMPT_RCU=y)?  If so, why?  I am having some difficulty seeing
> > how this combination could be at all helpful.  And if it is not helpful,
> > we should not allow people to shoot themselves in the foot with it.
> 
> With the new preemption model, NONE, VOLUNTARY and PREEMPT are now going to
> determine when NEED_RESCHED is set as supposed to NEED_RESCHED_LAZY. As
> NEED_RESCHED_LAZY only schedules at kernel / user space transaction, and
> NEED_RESCHED will schedule when possible (non-preempt disable section).

So NONE really is still supposed to be there.  ;-)

>  Key: L - NEED_RESCHED_LAZY - schedule only at kernel/user boundary
>       N - NEED_RESCHED - schedule whenever possible (like PREEMPT does today)
> 
> 			SCHED_OTHER	REAL-TIME/DL
> 			  Schedule	  Schedule
> 
> NONE:			      L		     L
> 
> VOLUNTARY:		      L		     N
> 
> PREEMPT:		      N		     N
> 
> 
> So on NONE, NEED_RESCHED_LAZY is set only on scheduling SCHED_OTHER and RT.
> Which means, it will not schedule until it goes into user space (*).
> 
> On VOLUNTARY, NEED_RESCHED is set on RT/DL tasks, and LAZY on SCHED_OTHER.
> So that RT and DL get scheduled just like PREEMPT does today.
> 
> On PREEMPT, NEED_RESCHED is always set on all scheduling.
> 
> (*) - caveat - After the next tick, if NEED_RESCHED_LAZY is set, then
> NEED_RESCHED will be set and the kernel will schedule at the next available
> moment, this is true for all three models!

OK, so I see that this is now a SCHED_FEAT, and is initialized based
on CONFIG_PREEMPT_* in kernel/sched/feature.h.  Huh.  OK, we can still
control this at build time, which is fine.  I don't see how to set it
at boot time, only at build time or from debugfs.  I will let those who
want to set this at boot time complain, should they choose to do so.

> There may be more details to work out, but the above is basically the gist
> of the idea. Now, what do you want to do with RCU_PREEMPT? At run time, we
> can go from NONE to PREEMPT full! But there may be use cases that do not
> want the overhead of always having RCU_PREEMPT, and will want RCU to be a
> preempt_disable() section no matter what.

Understood, actually.  And as noted in other replies, I am a bit concerned
about added latencies from too aggressively removing cond_resched().

More testing required.

> Unless we can switch between RCU_PREEMPT and !RCU_PREEMPT at run time, the
> dependency on RCU_PREEMPT tied to PREEMPT doesn't make sense anymore.

I strongly recommend against runtime switching of RCU's preemptibility,
just in case you were wondering.  ;-)

My question is different.

Would anyone want PREEMPT (N N above) in combination with non-preemptible
RCU?  I cannot see why anyone would want this.

> > > This could allow us to test this without this having to be part of this
> > > series.  
> > 
> > OK, if you mean for testing purposes but not to go to mainline without
> > the rest of the series, I am good with that idea.
> > 
> > And thank you to Ankur for preserving non-preemptible RCU for those of us
> > using system that are adequately but not generously endowed with memory!
> 
> Exactly. It sounds like having non-preempt RCU is agnostic to the
> preemption model of the system, which is why I think we need to make them
> disjoint.

How about like this, where "Y" means allowed and "N" means not allowed:

			Non-Preemptible RCU	Preemptible RCU

NONE:				Y			Y

VOLUNTARY:			Y			Y

PREEMPT:			N			Y

PREEMPT_RT:			N			Y


We need preemptible RCU for NONE and VOLUNTARY, as you say,
to allow CONFIG_PREEMPT_DYNAMIC to continue to work.  (OK, OK,
CONFIG_PREEMPT_DYNAMIC is no longer, but appears to be unconditional.)
But again, I don't see why anyone would want (much less need)
non-preemptible RCU in the PREEMPT and PREEMPT_RT cases.  And if it is
neither wanted nor needed, there is no point in enabling it, much less
testing it.

Or am I missing a use case in there somewhere?

							Thanx, Paul