linux-kernel - Re: [PATCH v2 0/6] RCU changes for PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241114100127.9xLSy4yq@linutronix.de>
Date: Thu, 14 Nov 2024 11:01:27 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Ankur Arora <ankur.a.arora@...cle.com>
Cc: linux-kernel@...r.kernel.org, peterz@...radead.org, tglx@...utronix.de,
	paulmck@...nel.org, mingo@...nel.org, juri.lelli@...hat.com,
	vincent.guittot@...aro.org, dietmar.eggemann@....com,
	rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
	vschneid@...hat.com, frederic@...nel.org, efault@....de,
	sshegde@...ux.ibm.com, boris.ostrovsky@...cle.com
Subject: Re: [PATCH v2 0/6] RCU changes for PREEMPT_LAZY

On 2024-11-06 12:17:52 [-0800], Ankur Arora wrote:
> This series adds RCU and some leftover scheduler bits for lazy
> preemption.

This is not critical for the current implementation. The way I
understand is that you make a change in 3/6 and then all other patches
in this series are required to deal with this.

For bisect reasons it would make sense to have 3/6 last in the series
and to the "fixes" first before the code is enabled. I mean if you apply
3/6 first then you get build failures without 1/6. But with 3/6 before
5/6 you should get runtime errors, right?

> The main problem addressed in the RCU related patches is that before
> PREEMPT_LAZY, PREEMPTION=y implied PREEMPT_RCU=y. With PREEMPT_LAZY,
> that's no longer true. 

No, you want to make PREEMPTION=y + PREEMPT_RCU=n + PREEMPT_LAZY=y
possible. This is different. Your wording makes it sound like there _is_
an actual problem.

> That's because PREEMPT_RCU makes some trade-offs to optimize for
> latency as opposed to throughput, and configurations with limited
> preemption might prefer the stronger forward-progress guarantees of
> PREEMPT_RCU=n.
> 
> Accordingly, with standalone PREEMPT_LAZY (much like PREEMPT_NONE,
> PREEMPT_VOLUNTARY) we want to use PREEMPT_RCU=n. And, when used in
> conjunction with PREEMPT_DYNAMIC, we continue to use PREEMPT_RCU=y.
> 
> Patches 1 and 2 are cleanup patches:
>   "rcu: fix header guard for rcu_all_qs()"
>   "rcu: rename PREEMPT_AUTO to PREEMPT_LAZY"
> 
> Patch 3, "rcu: limit PREEMPT_RCU configurations", explicitly limits
> PREEMPT_RCU=y to the PREEMPT_DYNAMIC or the latency oriented models.
> 
> Patches 4 and 5,
>   "rcu: handle quiescent states for PREEMPT_RCU=n, PREEMPT_COUNT=y"
>   "osnoise: handle quiescent states for PREEMPT_RCU=n, PREEMPTION=y"
> 
> handle quiescent states for the (PREEMPT_LAZY=y, PREEMPT_RCU=n)
> configuration.

I was briefly thinking about 

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5646,8 +5646,11 @@ void sched_tick(void)
 	hw_pressure = arch_scale_hw_pressure(cpu_of(rq));
 	update_hw_load_avg(rq_clock_task(rq), rq, hw_pressure);
 
-	if (dynamic_preempt_lazy() && tif_test_bit(TIF_NEED_RESCHED_LAZY))
+	if (dynamic_preempt_lazy() && tif_test_bit(TIF_NEED_RESCHED_LAZY)) {
 		resched_curr(rq);
+		if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
+			rcu_all_qs();
+	}
 
 	donor->sched_class->task_tick(rq, donor, 0);
 	if (sched_feat(LATENCY_WARN))

which should make #4+ #5 obsolete. But I think it is nicer to have the
change in #4 since it extends the check to cover all cases. And then
we would do it twice just for osnoise.

Sebastian