Thread (18 messages) 18 messages, 5 authors, 2019-06-11

Re: [patch 2/3] timers: do not raise softirq unconditionally (spinlockless version)

From: Marcelo Tosatti <hidden>
Date: 2019-06-06 15:15:27
Also in: lkml

On Tue, Jun 04, 2019 at 02:29:31PM +0800, Peter Xu wrote:
On Mon, Apr 15, 2019 at 05:12:15PM -0300, Marcelo Tosatti wrote:
quoted
Check base->pending_map locklessly and skip raising timer softirq 
if empty.

What allows the lockless (and potentially racy against mod_timer) 
check is that mod_timer will raise another timer softirq after
modifying base->pending_map.

Signed-off-by: Marcelo Tosatti <redacted>

---
 kernel/time/timer.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Index: linux-rt-devel/kernel/time/timer.c
===================================================================
--- linux-rt-devel.orig/kernel/time/timer.c	2019-04-15 14:21:02.788704354 -0300
+++ linux-rt-devel/kernel/time/timer.c	2019-04-15 14:22:56.755047354 -0300
@@ -1776,6 +1776,24 @@
 		if (time_before(jiffies, base->clk))
 			return;
 	}
+
+#ifdef CONFIG_PREEMPT_RT_FULL
+/* On RT, irq work runs from softirq */
+	if (irq_work_needs_cpu())
+		goto raise;
+#endif
+	base = this_cpu_ptr(&timer_bases[BASE_STD]);
+	if (!housekeeping_cpu(base->cpu, HK_FLAG_TIMER)) {
+		if (!bitmap_empty(base->pending_map, WHEEL_SIZE))
+			goto raise;
+		base++;
Shall we check against CONFIG_NO_HZ_COMMON?  Otherwise the base could
point to something else rather tha the deferred base (NR_BASES==1 if
without nohz-common).

I see that run_local_timers() has similar pattern, actually I'm
thinking whether we can put things like "base++" to be inside some
"if"s of CONFIG_NO_HZ_COMMON to be clear.

Thanks,

-- 
Peter Xu
Hi Peter,

Yes, that is a good point and needs fixing. 

I'll wait for Anna's comments before posting -v2.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help