PREEMPT_LAZY can be enabled stand-alone or alongside PREEMPT_DYNAMIC
which allows for dynamic switching of preemption models.
The choice of PREEMPT_RCU or not, however, is fixed at compile time.
Given that PREEMPT_RCU makes some trade-offs to optimize for latency
as opposed to throughput, configurations with limited preemption
might prefer the stronger forward-progress guarantees of PREEMPT_RCU=n.
Accordingly, explicitly limit PREEMPT_RCU=y to the latency oriented
preemption models: PREEMPT, PREEMPT_RT, and the runtime configurable
model PREEMPT_DYNAMIC.
This means the throughput oriented models, PREEMPT_NONE,
PREEMPT_VOLUNTARY and PREEMPT_LAZY will run with PREEMPT_RCU=n.
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
kernel/rcu/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
index 5a7ff5e1cdcb..9d52f87fac27 100644
--- a/kernel/rcu/Kconfig
+++ b/kernel/rcu/Kconfig
@@ -18,7 +18,7 @@ config TREE_RCU
config PREEMPT_RCU
bool
- default y if PREEMPTION
+ default y if (PREEMPT || PREEMPT_RT || PREEMPT_DYNAMIC)
select TREE_RCU
help
This option selects the RCU implementation that is
--
2.43.5Le Wed, Nov 06, 2024 at 12:17:55PM -0800, Ankur Arora a écrit :
> PREEMPT_LAZY can be enabled stand-alone or alongside PREEMPT_DYNAMIC
> which allows for dynamic switching of preemption models.
>
> The choice of PREEMPT_RCU or not, however, is fixed at compile time.
>
> Given that PREEMPT_RCU makes some trade-offs to optimize for latency
> as opposed to throughput, configurations with limited preemption
> might prefer the stronger forward-progress guarantees of PREEMPT_RCU=n.
>
> Accordingly, explicitly limit PREEMPT_RCU=y to the latency oriented
> preemption models: PREEMPT, PREEMPT_RT, and the runtime configurable
> model PREEMPT_DYNAMIC.
>
> This means the throughput oriented models, PREEMPT_NONE,
> PREEMPT_VOLUNTARY and PREEMPT_LAZY will run with PREEMPT_RCU=n.
>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> ---
> kernel/rcu/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> index 5a7ff5e1cdcb..9d52f87fac27 100644
> --- a/kernel/rcu/Kconfig
> +++ b/kernel/rcu/Kconfig
> @@ -18,7 +18,7 @@ config TREE_RCU
>
> config PREEMPT_RCU
> bool
> - default y if PREEMPTION
> + default y if (PREEMPT || PREEMPT_RT || PREEMPT_DYNAMIC)
> select TREE_RCU
> help
> This option selects the RCU implementation that is
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
But looking at !CONFIG_PREEMPT_RCU code on tree_plugin.h, I see
some issues now that the code can be preemptible. Well I think
it has always been preemptible but PREEMPTION && !PREEMPT_RCU
has seldom been exerciced (or was it even possible?).
For example rcu_read_unlock_strict() can be called with preemption
enabled so we need the following otherwise the rdp is unstable, the
norm value becomes racy (though automagically fixed in rcu_report_qs_rdp())
and rcu_report_qs_rdp() might warn.
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 58d84c59f3dd..368f00267d4e 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -95,9 +95,9 @@ static inline void __rcu_read_lock(void)
static inline void __rcu_read_unlock(void)
{
- preempt_enable();
if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD))
rcu_read_unlock_strict();
+ preempt_enable();
}
static inline int rcu_preempt_depth(void)
Let me audit further if we missed something else...
Thanks.
> --
> 2.43.5
>
Frederic Weisbecker <frederic@kernel.org> writes:
> Le Wed, Nov 06, 2024 at 12:17:55PM -0800, Ankur Arora a écrit :
>> PREEMPT_LAZY can be enabled stand-alone or alongside PREEMPT_DYNAMIC
>> which allows for dynamic switching of preemption models.
>>
>> The choice of PREEMPT_RCU or not, however, is fixed at compile time.
>>
>> Given that PREEMPT_RCU makes some trade-offs to optimize for latency
>> as opposed to throughput, configurations with limited preemption
>> might prefer the stronger forward-progress guarantees of PREEMPT_RCU=n.
>>
>> Accordingly, explicitly limit PREEMPT_RCU=y to the latency oriented
>> preemption models: PREEMPT, PREEMPT_RT, and the runtime configurable
>> model PREEMPT_DYNAMIC.
>>
>> This means the throughput oriented models, PREEMPT_NONE,
>> PREEMPT_VOLUNTARY and PREEMPT_LAZY will run with PREEMPT_RCU=n.
>>
>> Cc: Paul E. McKenney <paulmck@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>> ---
>> kernel/rcu/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
>> index 5a7ff5e1cdcb..9d52f87fac27 100644
>> --- a/kernel/rcu/Kconfig
>> +++ b/kernel/rcu/Kconfig
>> @@ -18,7 +18,7 @@ config TREE_RCU
>>
>> config PREEMPT_RCU
>> bool
>> - default y if PREEMPTION
>> + default y if (PREEMPT || PREEMPT_RT || PREEMPT_DYNAMIC)
>> select TREE_RCU
>> help
>> This option selects the RCU implementation that is
>
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
Thanks!
> But looking at !CONFIG_PREEMPT_RCU code on tree_plugin.h, I see
> some issues now that the code can be preemptible. Well I think
> it has always been preemptible but PREEMPTION && !PREEMPT_RCU
> has seldom been exerciced (or was it even possible?).
>
> For example rcu_read_unlock_strict() can be called with preemption
> enabled so we need the following otherwise the rdp is unstable, the
> norm value becomes racy
I think I see your point about rdp being racy, but given that
rcu_read_unlock_strict() would always be called with a non-zero
preemption count (with CONFIG_PREEMPTION), wouldn't the preempt_count()
check defeat any calls to rcu_read_unlock_strict()?
void rcu_read_unlock_strict(void)
{
struct rcu_data *rdp;
if (irqs_disabled() || preempt_count() || !rcu_state.gp_kthread)
return;
Or am I missing something?
Ankur
> (though automagically fixed in rcu_report_qs_rdp())
> and rcu_report_qs_rdp() might warn.
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 58d84c59f3dd..368f00267d4e 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -95,9 +95,9 @@ static inline void __rcu_read_lock(void)
>
> static inline void __rcu_read_unlock(void)
> {
> - preempt_enable();
> if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD))
> rcu_read_unlock_strict();
> + preempt_enable();
> }
>
> static inline int rcu_preempt_depth(void)
>
>
> Let me audit further if we missed something else...
>
> Thanks.
>
>> --
>> 2.43.5
>>
On 2024-11-13 16:23:03 [-0800], Ankur Arora wrote:
> > But looking at !CONFIG_PREEMPT_RCU code on tree_plugin.h, I see
> > some issues now that the code can be preemptible. Well I think
> > it has always been preemptible but PREEMPTION && !PREEMPT_RCU
> > has seldom been exerciced (or was it even possible?).
> >
> > For example rcu_read_unlock_strict() can be called with preemption
> > enabled so we need the following otherwise the rdp is unstable, the
> > norm value becomes racy
>
> I think I see your point about rdp being racy, but given that
> rcu_read_unlock_strict() would always be called with a non-zero
> preemption count (with CONFIG_PREEMPTION), wouldn't the preempt_count()
> check defeat any calls to rcu_read_unlock_strict()?
>
> void rcu_read_unlock_strict(void)
> {
> struct rcu_data *rdp;
>
> if (irqs_disabled() || preempt_count() || !rcu_state.gp_kthread)
> return;
>
> Or am I missing something?
This is indeed broken. By moving preempt_disable() as Frederic suggested
then rcu_read_unlock_strict() becomes a NOP. Keeping this as-is results
in spats due to this_cpu_ptr() in preemptible regions. Looking further
we have "rdp->cpu != smp_processor_id()" as the next candidate.
That preempt_disable() should go to rcu_read_unlock_strict() after the
check.
> Ankur
Sebastian
Le Thu, Nov 14, 2024 at 09:25:34AM +0100, Sebastian Andrzej Siewior a écrit :
> On 2024-11-13 16:23:03 [-0800], Ankur Arora wrote:
> > > But looking at !CONFIG_PREEMPT_RCU code on tree_plugin.h, I see
> > > some issues now that the code can be preemptible. Well I think
> > > it has always been preemptible but PREEMPTION && !PREEMPT_RCU
> > > has seldom been exerciced (or was it even possible?).
> > >
> > > For example rcu_read_unlock_strict() can be called with preemption
> > > enabled so we need the following otherwise the rdp is unstable, the
> > > norm value becomes racy
> >
> > I think I see your point about rdp being racy, but given that
> > rcu_read_unlock_strict() would always be called with a non-zero
> > preemption count (with CONFIG_PREEMPTION), wouldn't the preempt_count()
> > check defeat any calls to rcu_read_unlock_strict()?
> >
> > void rcu_read_unlock_strict(void)
> > {
> > struct rcu_data *rdp;
> >
> > if (irqs_disabled() || preempt_count() || !rcu_state.gp_kthread)
> > return;
> >
> > Or am I missing something?
Haha, right!
>
> This is indeed broken. By moving preempt_disable() as Frederic suggested
> then rcu_read_unlock_strict() becomes a NOP. Keeping this as-is results
> in spats due to this_cpu_ptr() in preemptible regions. Looking further
> we have "rdp->cpu != smp_processor_id()" as the next candidate.
>
> That preempt_disable() should go to rcu_read_unlock_strict() after the
> check.
Yeah that looks better ;-)
>
> > Ankur
>
> Sebastian
© 2016 - 2024 Red Hat, Inc.