Re: [PATCH 2/7] rcu: limit PREEMPT_RCU configurations

[Ankur Arora]

Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes:

> On 2024-10-11 08:59:14 [-0700], Paul E. McKenney wrote:
>> On Fri, Oct 11, 2024 at 04:43:41PM +0200, Sebastian Andrzej Siewior wrote:
>>
>> > Okay, this eliminates PREEMPT_DYNAMIC then.
>> > With PeterZ current series, PREEMPT_LAZY (without everything else
>> > enabled) behaves as PREEMPT without the "forced" wake up for the fair
>> > class. It is preemptible after all, with preempt_disable() actually
>> > doing something. This might speak for preemptible RCU.
>> > And assuming in this condition you that "low memory overhead RCU" which
>> > is not PREEMPT_RCU. This might require a config option.
>>
>> The PREEMPT_DYNAMIC case seems to work well as-is for the intended users,
>> so I don't see a need to change it.  In particular, we already learned
>> that we need to set PREEMPT_DYNAMIC=n.  Yes, had I caught this in time, I
>> would have argued against changing the default, but this was successfully
>> slid past me.
>>
>> As for PREEMPT_LAZY, you seem to be suggesting a more intrusive change
>> than just keeping non-preemptible RCU when the Kconfig options are
>> consistent with this being expected.  If this is the case, what are the
>> benefits of this more-intrusive change?
>
> As far as I understand you are only concerned about PREEMPT_LAZY and
> everything else (PREEMPT_LAZY + PREEMPT_DYNAMIC or PREEMPT_DYNAMIC
> without PREEMPT_LAZY) is fine.
> In the PREEMPT_LAZY + !PREEMPT_DYNAMIC the suggested change
>
> | config PREEMPT_RCU
> | 	bool
> | 	default y if (PREEMPT || PREEMPT_RT || PREEMPT_DYNAMIC)
> | 	select TREE_RCU
> | 	help
>
> would disable PREEMPT_RCU while the default model is PREEMPT. You argue

With PREEMPT_LAZY=y, PREEMPT_DYNAMIC=n, isn't the default model
PREEMPT_LAZY, which has PREEMPTION=y, but PREEMPT=n?

> that only people on small embedded would do such a thing and they would
> like to safe additional memory.
>
> I don't think this is always the case because the "preemptible" users
> would also get this and this is an unexpected change for them.

Can you clarify this? The intent with lazy is to be preemptible but
preempt less often. In that it is meant to be quite different from
CONFIG_PREEMPT.

Ankur

>> > > > If you would like to add some relief to memory constrained systems,
>> > > > wouldn't BASE_SMALL be something you could hook to? With EXPERT_RCU to
>> > > > allow to override it?
>> > >
>> > > Does BASE_SMALL affect anything but log buffer sizes?  Either way, we
>> > > would still need to avoid the larger memory footprint of preemptible
>> > > RCU that shows up due to RCU readers being preempted.
>> >
>> > It only reduces data structures where possible. So lower performance is
>> > probably due to things like futex hashmap (and others) are smaller.
>>
>> Which is still counterproductive for use cases other than small deep
>> embedded systems.
>
> Okay, so that option is gone.
>
>> > > Besides, we are not looking to give up performance vs BASE_SMALL's
>> > > "may reduce performance" help text.
>> > >
>> > > Yes, yes, it would simplify things to just get rid of non-preemptible RCU,
>> > > but that is simply not in the cards at the moment.
>> >
>> > Not sure what the time frame is here. If we go for LAZY and remove NONE
>> > and VOLUNTARY then making PREEMPT_RCU would make sense to lower the
>> > memory footprint (and not attaching to BASE_SMALL).
>> >
>> > Is this what you intend or did misunderstand something here?
>>
>> My requirement is that LAZY not remove/disable/whatever non-preemptible
>> RCU.  Those currently using non-preemptible RCU should continue to be able
>> to be able to use it, with or without LAZY.  So why is this requirement
>> a problem for you?  Or am I missing your point?
>
> Those who were using non-preemptible RCU, whish to use LAZY_PREEPMT +
> !PREEMPT_DYNAMIC should be able to disable PREEMPT_RCU only in this case.
> Would the following work?
>
> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
> index 8cf8a9a4d868c..2183c775e7808 100644
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -121,6 +121,7 @@ config PREEMPT_COUNT
>  config PREEMPTION
>         bool
>         select PREEMPT_COUNT
> +       select PREEMPT_RCU if PREEMPT_DYNAMIC
>
>  config PREEMPT_DYNAMIC
>  	bool "Preemption behaviour defined on boot"
> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> index 3e079de0f5b43..9e4bdbbca4ff9 100644
> --- a/kernel/rcu/Kconfig
> +++ b/kernel/rcu/Kconfig
> @@ -17,7 +17,7 @@ config TREE_RCU
>  	  smaller systems.
>
>  config PREEMPT_RCU
> -	bool
> +	bool "Preemptible RCU"
>  	default y if PREEMPTION
>  	select TREE_RCU
>  	help
> @@ -91,7 +91,7 @@ config NEED_TASKS_RCU
>
>  config TASKS_RCU
>  	bool
> -	default NEED_TASKS_RCU && (PREEMPTION || PREEMPT_AUTO)
> +	default NEED_TASKS_RCU && PREEMPTION
>  	select IRQ_WORK
>
>  config FORCE_TASKS_RUDE_RCU
>
> I added TASKS_RCU to the hunk since I am not sure if you wish to follow
> PREEMPTION (which is set by LAZY) or PREEMPT_RCU.
>
>> 							Thanx, Paul
>
> Sebastian


--
ankur