sched: fix up preempt lazy forward port

RFC PATCH In review Mike Galbraith 3 participants 18 messages Jan 18, 2016 – Feb 21, 2016

Subsystems (5)

Arm (32-bit) x86 arm64 Scheduler +1 more subsystems

lore

Share & tools

lore

Mike Galbraith

author

2016-01-18 09:08 UTC #0 lore raw

RFC PATCH 1/1 sched: fix up preempt lazy forward port

pre:
preempt lazy enabled
homer:/root # taskset -c 7 pipe-test 1 2.618038 usecs/loop -- avg 2.618038 763.9 KHz 2.620658 usecs/loop -- avg 2.618300 763.9 KHz 2.618775 usecs/loop -- avg 2.618347 763.8 KHz 2.618819 usecs/loop -- avg 2.618395 763.8 KHz 2.619269 usecs/loop -- avg 2.618482 763.8 KHz

tbench
Throughput 2612.9 MB/sec 8 procs

preempt lazy disabled
homer:/root # taskset -c 7 pipe-test 1 1.771921 usecs/loop -- avg 1.771921 1128.7 KHz 1.782686 usecs/loop -- avg 1.772998 1128.0 KHz 1.785444 usecs/loop -- avg 1.774242 1127.2 KHz 1.787388 usecs/loop -- avg 1.775557 1126.4 KHz 1.770772 usecs/loop -- avg 1.775078 1126.7 KHz

tbench
Throughput 2626.91 MB/sec 8 procs

post:
preempt lazy enabled
homer:/root # taskset -c 7 pipe-test 1 1.485592 usecs/loop -- avg 1.485592 1346.3 KHz 1.489640 usecs/loop -- avg 1.485997 1345.9 KHz 1.488325 usecs/loop -- avg 1.486230 1345.7 KHz 1.484632 usecs/loop -- avg 1.486070 1345.8 KHz 1.484889 usecs/loop -- avg 1.485952 1345.9 KHz

tbench
Throughput 3091.84 MB/sec 8 procs

preempt lazy disabled
homer:/root # taskset -c 7 pipe-test 1 1.579723 usecs/loop -- avg 1.579723 1266.0 KHz 1.562026 usecs/loop -- avg 1.577953 1267.5 KHz 1.546090 usecs/loop -- avg 1.574767 1270.0 KHz 1.543852 usecs/loop -- avg 1.571675 1272.5 KHz 1.546313 usecs/loop -- avg 1.569139 1274.6 KHz

tbench
Throughput 2649.65 MB/sec 8 procs

Signed-off-by Mike Galbraith <[email protected]>

---
 arch/x86/entry/common.c |    4 ++--
 kernel/sched/core.c     |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

arch/x86/entry/common.c +2 -2

@@ -220,14 +220,14 @@ long syscall_trace_enter(struct pt_regs
 
 #define EXIT_TO_USERMODE_LOOP_FLAGS				\
 	(_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE |	\
-	 _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
+	 _TIF_NEED_RESCHED_MASK | _TIF_USER_RETURN_NOTIFY)
 
 static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
 {
 	/*
 	 * In order to return to user mode, we need to have IRQs off with
 	 * none of _TIF_SIGPENDING, _TIF_NOTIFY_RESUME, _TIF_USER_RETURN_NOTIFY,
-	 * _TIF_UPROBE, or _TIF_NEED_RESCHED set.  Several of these flags
+	 * _TIF_UPROBE, or _TIF_NEED_RESCHED_MASK set.  Several of these flags
 	 * can be set at any time on preemptable kernels if we have IRQs on,
 	 * so we need to loop.  Disabling preemption wouldn't help: doing the
 	 * work to clear some of the flags can sleep.

kernel/sched/core.c +9 -0

@@ -3542,6 +3542,15 @@ asmlinkage __visible void __sched notrac
 	if (likely(!preemptible()))
 		return;
 
+#ifdef CONFIG_PREEMPT_LAZY
+	/*
+	 * Check for lazy preemption
+	 */
+	if (current_thread_info()->preempt_lazy_count &&
+	    !test_thread_flag(TIF_NEED_RESCHED))
+		return;
+#endif
+
 	preempt_schedule_common();
 }
 NOKPROBE_SYMBOL(preempt_schedule);

Sebastian Andrzej Siewior

2016-01-18 20:18 UTC #1 lore raw

arch/x86/entry/common.c long syscall_trace_enter(struct pt_regs · line 224

Quoted 9 lines · * Mike Galbraith | 2016-01-18 10:08:23 [+0100]:

arch/x86/entry/common.c +1 -1

@@ -220,14 +220,14 @@ long syscall_trace_enter(struct pt_regs
 
 #define EXIT_TO_USERMODE_LOOP_FLAGS				\
 	(_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE |	\
-	 _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
+	 _TIF_NEED_RESCHED_MASK | _TIF_USER_RETURN_NOTIFY)

If I read this right, the loop where this define is used _TIF_ALLWORK_MASK in v4.1 of which _TIF_NEED_RESCHED_MASK was part of. Adding this will reassmeble the old behaviour.
…

Quoted 5 lines

kernel/sched/core.c +0 -0

@@ -3542,6 +3542,15 @@ asmlinkage __visible void __sched notrac
 	if (likely(!preemptible()))
 		return;

kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3553

Quoted 10 lines

kernel/sched/core.c +9 -0

 
+#ifdef CONFIG_PREEMPT_LAZY
+	/*
+	 * Check for lazy preemption
+	 */
+	if (current_thread_info()->preempt_lazy_count &&
+	    !test_thread_flag(TIF_NEED_RESCHED))
+		return;
+#endif
+

And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is v4.1 … v4.4 or not.

kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3556

Quoted 3 lines

kernel/sched/core.c +0 -0

 	preempt_schedule_common();
 }
 NOKPROBE_SYMBOL(preempt_schedule);

Sebastian

Mike Galbraith

author

2016-01-19 02:30 UTC #2 lore raw

Quoted 6 lines · Sebastian Andrzej Siewior

Quoted 6 lines

kernel/sched/core.c +0 -0

@@ -3542,6 +3542,15 @@ asmlinkage __visible void __sched notrac
 	if (likely(!preemptible()))
 		return;

Quoted 13 lines

kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3553
Quoted 9 lines
kernel/sched/core.c +9 -0
+#ifdef CONFIG_PREEMPT_LAZY
+	/*
+	 * Check for lazy preemption
+	 */
+	if (current_thread_info()->preempt_lazy_count &&
+	    !test_thread_flag(TIF_NEED_RESCHED))
+		return;
+#endif
+
And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is v4.1 … v4.4 or not.

We should probably just add the lazy bits to preemptible().

-Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sebastian Andrzej Siewior

2016-01-21 12:54 UTC #3 lore raw

Quoted 5 lines · * Mike Galbraith | 2016-01-19 03:29:57 [+0100]:

Quoted 3 lines
And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is v4.1 … v4.4 or not.
We should probably just add the lazy bits to preemptible().

Subject: preempt-lazy: Add the lazy-preemption check to preempt_schedule()

Probably in the rebase onto v4.1 this check got moved into less commonly used preempt_schedule_notrace(). This patch ensures that both functions use it.

Reported-by Mike Galbraith <[email protected]>

Signed-off-by Sebastian Andrzej Siewior <[email protected]>

---
 kernel/sched/core.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

kernel/sched/core.c +28 -8

@@ -3461,6 +3461,30 @@ static void __sched notrace preempt_sche
 	} while (need_resched());
 }
 
+#ifdef CONFIG_PREEMPT_LAZY
+/*
+ * If TIF_NEED_RESCHED is then we allow to be scheduled away since this is
+ * set by a RT task. Oterwise we try to avoid beeing scheduled out as long as
+ * preempt_lazy_count counter >0.
+ */
+static int preemptible_lazy(void)
+{
+	if (test_thread_flag(TIF_NEED_RESCHED))
+		return 1;
+	if (current_thread_info()->preempt_lazy_count)
+		return 0;
+	return 1;
+}
+
+#else
+
+static int preemptible_lazy(void)
+{
+	return 1;
+}
+
+#endif
+
 #ifdef CONFIG_PREEMPT
 /*
  * this is the entry point to schedule() from in-kernel preemption

@@ -3475,6 +3499,8 @@ asmlinkage __visible void __sched notrac
 	 */
 	if (likely(!preemptible()))
 		return;
+	if (!preemptible_lazy())
+		return;
 
 	preempt_schedule_common();
 }

@@ -3501,15 +3527,9 @@ asmlinkage __visible void __sched notrac
 
 	if (likely(!preemptible()))
 		return;
-
-#ifdef CONFIG_PREEMPT_LAZY
-	/*
-	 * Check for lazy preemption
-	 */
-	if (current_thread_info()->preempt_lazy_count &&
-	    !test_thread_flag(TIF_NEED_RESCHED))
+	if (!preemptible_lazy())
 		return;
-#endif
+
 	do {
 		preempt_disable_notrace();
 		/*

Grygorii Strashko

2016-01-22 12:24 UTC #4 lore raw

Quoted 69 lines · Sebastian Andrzej Siewior

Quoted 5 lines · * Mike Galbraith | 2016-01-19 03:29:57 [+0100]:
Quoted 3 lines
And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is v4.1 … v4.4 or not.
We should probably just add the lazy bits to preemptible().
Subject: preempt-lazy: Add the lazy-preemption check to preempt_schedule()
Probably in the rebase onto v4.1 this check got moved into less commonly used preempt_schedule_notrace(). This patch ensures that both functions use it.
Reported-by Mike Galbraith <[email protected]>
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
---
  kernel/sched/core.c |   36 ++++++++++++++++++++++++++++--------
  1 file changed, 28 insertions(+), 8 deletions(-)
kernel/sched/core.c +26 -4
@@ -3461,6 +3461,30 @@ static void __sched notrace preempt_sche
  	} while (need_resched());
  }
 
+#ifdef CONFIG_PREEMPT_LAZY
+/*
+ * If TIF_NEED_RESCHED is then we allow to be scheduled away since this is
+ * set by a RT task. Oterwise we try to avoid beeing scheduled out as long as
+ * preempt_lazy_count counter >0.
+ */
+static int preemptible_lazy(void)
+{
+	if (test_thread_flag(TIF_NEED_RESCHED))
+		return 1;
+	if (current_thread_info()->preempt_lazy_count)
+		return 0;
+	return 1;
+}
+
+#else
+
+static int preemptible_lazy(void)
+{
+	return 1;
+}
+
+#endif
+
  #ifdef CONFIG_PREEMPT
  /*
   * this is the entry point to schedule() from in-kernel preemption
@@ -3475,6 +3499,8 @@ asmlinkage __visible void __sched notrac
  	 */
  	if (likely(!preemptible()))
  		return;
+	if (!preemptible_lazy())
+		return;
 
  	preempt_schedule_common();
  }
@@ -3501,15 +3527,9 @@ asmlinkage __visible void __sched notrac
 
  	if (likely(!preemptible()))
  		return;
-
-#ifdef CONFIG_PREEMPT_LAZY
-	/*
-	 * Check for lazy preemption

kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3535

Quoted 10 lines

kernel/sched/core.c +2 -4

-	 */
-	if (current_thread_info()->preempt_lazy_count &&
-	    !test_thread_flag(TIF_NEED_RESCHED))
+	if (!preemptible_lazy())
  		return;
-#endif
+
  	do {
  		preempt_disable_notrace();
  		/*

I can see below backtrace during boot if i backport this change in K4.1-rt TI am437x-idk (UP) board.

[    0.204022] Switched to clocksource arm_global_timer
[    0.211982] ------------[ cut here ]------------
[    0.212021] WARNING: CPU: 0 PID: 3 at kernel/softirq.c:894 __tasklet_action+0x150/0x17c()
[    0.212027] Modules linked in:
[    0.212041] CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.1.15-rt11-00324-g52f8148-dirty #117
[    0.212046] Hardware name: Generic AM43 (Flattened Device Tree)
[    0.212052] Backtrace: 
[    0.212080] [<c0012b50>] (dump_backtrace) from [<c0012d70>] (show_stack+0x18/0x1c)
[    0.212085]  r7:c0730a84 r6:0000037e r5:00000009 r4:00000000
[    0.212112] [<c0012d58>] (show_stack) from [<c05e6940>] (dump_stack+0x20/0x28)
[    0.212124] [<c05e6920>] (dump_stack) from [<c003713c>] (warn_slowpath_common+0x7c/0xb4)
[    0.212134] [<c00370c0>] (warn_slowpath_common) from [<c0037218>] (warn_slowpath_null+0x24/0x2c)
[    0.212138]  r8:000f4240 r7:00000000 r6:c0841238 r5:00000018 r4:00000006
[    0.212156] [<c00371f4>] (warn_slowpath_null) from [<c003a0a4>] (__tasklet_action+0x150/0x17c)
[    0.212165] [<c0039f54>] (__tasklet_action) from [<c003a138>] (tasklet_action+0x30/0x38)
[    0.212169]  r9:04208140 r8:c0862300 r7:000000c0 r6:c082d774 r5:00000018 r4:00000006
[    0.212187] [<c003a108>] (tasklet_action) from [<c0039c2c>] (do_current_softirqs+0x190/0x258)
[    0.212197] [<c0039a9c>] (do_current_softirqs) from [<c003a170>] (run_ksoftirqd+0x30/0x58)
[    0.212201]  r10:00000000 r9:00000000 r8:00000002 r7:c082d8f0 r6:00000001 r5:ee81f740
[    0.212213]  r4:ee874000
[    0.212227] [<c003a140>] (run_ksoftirqd) from [<c00534e0>] (smpboot_thread_fn+0x15c/0x278)
[    0.212242] [<c0053384>] (smpboot_thread_fn) from [<c00500fc>] (kthread+0xcc/0xe4)
[    0.212246]  r9:00000000 r8:00000000 r7:c0053384 r6:ee81f740 r5:ee81f780 r4:00000000
[    0.212265] [<c0050030>] (kthread) from [<c000f988>] (ret_from_fork+0x14/0x2c)
[    0.212269]  r7:00000000 r6:00000000 r5:c0050030 r4:ee81f780
[    0.212279] ---[ end trace 0000000000000001 ]---
[    0.212284] softirq: ================== c037d4c4 <---- t->func
^^
# grep c037d4c4 System.map
c037d4c4 t kbd_bh

Signature · 6 lines

regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sebastian Andrzej Siewior

2016-01-22 13:19 UTC #5 lore raw

Quoted 25 lines · Grygorii Strashko

I can see below backtrace during boot if i backport this change in K4.1-rt TI am437x-idk (UP) board.

[    0.204022] Switched to clocksource arm_global_timer
[    0.211982] ------------[ cut here ]------------
[    0.212021] WARNING: CPU: 0 PID: 3 at kernel/softirq.c:894 __tasklet_action+0x150/0x17c()
[    0.212027] Modules linked in:
[    0.212041] CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.1.15-rt11-00324-g52f8148-dirty #117
[    0.212046] Hardware name: Generic AM43 (Flattened Device Tree)
[    0.212052] Backtrace: 
[    0.212080] [<c0012b50>] (dump_backtrace) from [<c0012d70>] (show_stack+0x18/0x1c)
[    0.212085]  r7:c0730a84 r6:0000037e r5:00000009 r4:00000000
[    0.212112] [<c0012d58>] (show_stack) from [<c05e6940>] (dump_stack+0x20/0x28)
[    0.212124] [<c05e6920>] (dump_stack) from [<c003713c>] (warn_slowpath_common+0x7c/0xb4)
[    0.212134] [<c00370c0>] (warn_slowpath_common) from [<c0037218>] (warn_slowpath_null+0x24/0x2c)
[    0.212138]  r8:000f4240 r7:00000000 r6:c0841238 r5:00000018 r4:00000006
[    0.212156] [<c00371f4>] (warn_slowpath_null) from [<c003a0a4>] (__tasklet_action+0x150/0x17c)
[    0.212165] [<c0039f54>] (__tasklet_action) from [<c003a138>] (tasklet_action+0x30/0x38)
[    0.212169]  r9:04208140 r8:c0862300 r7:000000c0 r6:c082d774 r5:00000018 r4:00000006
[    0.212187] [<c003a108>] (tasklet_action) from [<c0039c2c>] (do_current_softirqs+0x190/0x258)
[    0.212197] [<c0039a9c>] (do_current_softirqs) from [<c003a170>] (run_ksoftirqd+0x30/0x58)
[    0.212201]  r10:00000000 r9:00000000 r8:00000002 r7:c082d8f0 r6:00000001 r5:ee81f740
[    0.212213]  r4:ee874000
[    0.212227] [<c003a140>] (run_ksoftirqd) from [<c00534e0>] (smpboot_thread_fn+0x15c/0x278)
[    0.212242] [<c0053384>] (smpboot_thread_fn) from [<c00500fc>] (kthread+0xcc/0xe4)

Quoted 8 lines

[    0.212246]  r9:00000000 r8:00000000 r7:c0053384 r6:ee81f740 r5:ee81f780 r4:00000000
[    0.212265] [<c0050030>] (kthread) from [<c000f988>] (ret_from_fork+0x14/0x2c)
[    0.212269]  r7:00000000 r6:00000000 r5:c0050030 r4:ee81f780
[    0.212279] ---[ end trace 0000000000000001 ]---
[    0.212284] softirq: ================== c037d4c4 <---- t->func
^^
# grep c037d4c4 System.map
c037d4c4 t kbd_bh

Do you still see it with the missing lazy bits? I don't see where this could come from.

Sebastian

Grygorii Strashko

2016-01-22 13:34 UTC #6 lore raw

Quoted 35 lines · Sebastian Andrzej Siewior

Quoted 25 lines · Grygorii Strashko

I can see below backtrace during boot if i backport this change in K4.1-rt TI am437x-idk (UP) board.

[    0.204022] Switched to clocksource arm_global_timer
[    0.211982] ------------[ cut here ]------------
[    0.212021] WARNING: CPU: 0 PID: 3 at kernel/softirq.c:894 __tasklet_action+0x150/0x17c()
[    0.212027] Modules linked in:
[    0.212041] CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.1.15-rt11-00324-g52f8148-dirty #117
[    0.212046] Hardware name: Generic AM43 (Flattened Device Tree)
[    0.212052] Backtrace:
[    0.212080] [<c0012b50>] (dump_backtrace) from [<c0012d70>] (show_stack+0x18/0x1c)
[    0.212085]  r7:c0730a84 r6:0000037e r5:00000009 r4:00000000
[    0.212112] [<c0012d58>] (show_stack) from [<c05e6940>] (dump_stack+0x20/0x28)
[    0.212124] [<c05e6920>] (dump_stack) from [<c003713c>] (warn_slowpath_common+0x7c/0xb4)
[    0.212134] [<c00370c0>] (warn_slowpath_common) from [<c0037218>] (warn_slowpath_null+0x24/0x2c)
[    0.212138]  r8:000f4240 r7:00000000 r6:c0841238 r5:00000018 r4:00000006
[    0.212156] [<c00371f4>] (warn_slowpath_null) from [<c003a0a4>] (__tasklet_action+0x150/0x17c)
[    0.212165] [<c0039f54>] (__tasklet_action) from [<c003a138>] (tasklet_action+0x30/0x38)
[    0.212169]  r9:04208140 r8:c0862300 r7:000000c0 r6:c082d774 r5:00000018 r4:00000006
[    0.212187] [<c003a108>] (tasklet_action) from [<c0039c2c>] (do_current_softirqs+0x190/0x258)
[    0.212197] [<c0039a9c>] (do_current_softirqs) from [<c003a170>] (run_ksoftirqd+0x30/0x58)
[    0.212201]  r10:00000000 r9:00000000 r8:00000002 r7:c082d8f0 r6:00000001 r5:ee81f740
[    0.212213]  r4:ee874000
[    0.212227] [<c003a140>] (run_ksoftirqd) from [<c00534e0>] (smpboot_thread_fn+0x15c/0x278)
[    0.212242] [<c0053384>] (smpboot_thread_fn) from [<c00500fc>] (kthread+0xcc/0xe4)

Quoted 8 lines

[    0.212246]  r9:00000000 r8:00000000 r7:c0053384 r6:ee81f740 r5:ee81f780 r4:00000000
[    0.212265] [<c0050030>] (kthread) from [<c000f988>] (ret_from_fork+0x14/0x2c)
[    0.212269]  r7:00000000 r6:00000000 r5:c0050030 r4:ee81f780
[    0.212279] ---[ end trace 0000000000000001 ]---
[    0.212284] softirq: ================== c037d4c4 <---- t->func
^^
# grep c037d4c4 System.map
c037d4c4 t kbd_bh

Quoted 3 lines

Do you still see it with the missing lazy bits? I don't see where this could come from.

yep. I've tried it with and without changing _TIF_WORK_MASK Seems It's triggered by Virtual terminal keyboard_tasklet

Signature · 2 lines

regards,
-grygorii

Sebastian Andrzej Siewior

2016-01-22 14:14 UTC #7 lore raw

Quoted 6 lines · Grygorii Strashko

Quoted 3 lines
Do you still see it with the missing lazy bits? I don't see where this could come from.
yep. I've tried it with and without changing _TIF_WORK_MASK Seems It's triggered by Virtual terminal keyboard_tasklet

Just bootet my current v4.4 to cross check what you see and I have this:

[    3.808273] clocksource: Switched to clocksource arch_sys_counter
[    3.851193] keyboard: kbd_init(1609)
[    3.861903] keyboard: kbd_bh(1201)
[    3.868322] keyboard: kbd_init(1612)
[    3.868581] NET: Registered protocol family 2

So. No backtrace here. According to the full dmesg I should have all CPUs up by now. kbd_bh is comming from ksoftirqd as well. No idea why you see this.
There is a irq off region which sets and removes RUN bit, I have no idea why you have this backtrace.

Sebastian

Grygorii Strashko

2016-01-22 15:04 UTC #8 lore raw

Quoted 16 lines · Sebastian Andrzej Siewior

Quoted 6 lines · Grygorii Strashko
Quoted 3 lines
Do you still see it with the missing lazy bits? I don't see where this could come from.
yep. I've tried it with and without changing _TIF_WORK_MASK Seems It's triggered by Virtual terminal keyboard_tasklet
Just bootet my current v4.4 to cross check what you see and I have this:
[    3.808273] clocksource: Switched to clocksource arch_sys_counter
[    3.851193] keyboard: kbd_init(1609)
[    3.861903] keyboard: kbd_bh(1201)
[    3.868322] keyboard: kbd_init(1612)
[    3.868581] NET: Registered protocol family 2

Quoted 6 lines

So. No backtrace here. According to the full dmesg I should have all CPUs up by now. kbd_bh is comming from ksoftirqd as well. No idea why you see this.
There is a irq off region which sets and removes RUN bit, I have no idea why you have this backtrace.

As of now, i'm too :(
- not sure if this could be the case, but my kernel is !SMP # CONFIG_SMP is not set
# CONFIG_CPUSETS is not set

Signature · 2 lines

regards,
-grygorii

Grygorii Strashko

2016-01-22 18:21 UTC #9 lore raw

hi Sebastian,

Quoted 24 lines · Grygorii Strashko

Quoted 16 lines · Sebastian Andrzej Siewior
Quoted 6 lines · Grygorii Strashko
Quoted 3 lines
Do you still see it with the missing lazy bits? I don't see where this could come from.
yep. I've tried it with and without changing _TIF_WORK_MASK Seems It's triggered by Virtual terminal keyboard_tasklet
Just bootet my current v4.4 to cross check what you see and I have this:
[    3.808273] clocksource: Switched to clocksource arch_sys_counter
[    3.851193] keyboard: kbd_init(1609)
[    3.861903] keyboard: kbd_bh(1201)
[    3.868322] keyboard: kbd_init(1612)
[    3.868581] NET: Registered protocol family 2
Quoted 6 lines
So. No backtrace here. According to the full dmesg I should have all CPUs up by now. kbd_bh is comming from ksoftirqd as well. No idea why you see this.
There is a irq off region which sets and removes RUN bit, I have no idea why you have this backtrace.

Quoted 5 lines

As of now, i'm too :(
- not sure if this could be the case, but my kernel is !SMP # CONFIG_SMP is not set
# CONFIG_CPUSETS is not set

Sorry, this is false alarm - I did mistake in my patch.

Sorry for your time.

Signature · 2 lines

regards,
-grygorii

Mike Galbraith

author

2016-02-21 15:11 UTC #10 lore raw

PATCH 1/1 sched,rt: __always_inline preemptible_lazy()

In reply to Sebastian Andrzej Siewior

homer: # nm kernel/sched/core.o|grep preemptible_lazy 00000000000000b5 t preemptible_lazy

echo wakeup_rt > current_tracer ==> Welcome to infinity.

Signed-off-bx: Mike Galbraith <[email protected]>

---
 kernel/sched/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/sched/core.c +1 -1

@@ -3469,7 +3469,7 @@ static void __sched notrace preempt_sche
  * set by a RT task. Oterwise we try to avoid beeing scheduled out as long as
  * preempt_lazy_count counter >0.
  */
-static int preemptible_lazy(void)
+static __always_inline int preemptible_lazy(void)
 {
 	if (test_thread_flag(TIF_NEED_RESCHED))
 		return 1;

Mike Galbraith

author

2016-01-19 04:42 UTC #11 lore raw

In reply to Sebastian Andrzej Siewior

Quoted 6 lines · Sebastian Andrzej Siewior

Quoted 6 lines

kernel/sched/core.c +0 -0

@@ -3542,6 +3542,15 @@ asmlinkage __visible void __sched notrac
 	if (likely(!preemptible()))
 		return;

Quoted 13 lines

kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3553
Quoted 9 lines
kernel/sched/core.c +9 -0
+#ifdef CONFIG_PREEMPT_LAZY
+	/*
+	 * Check for lazy preemption
+	 */
+	if (current_thread_info()->preempt_lazy_count &&
+	    !test_thread_flag(TIF_NEED_RESCHED))
+		return;
+#endif
+
And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is v4.1 … v4.4 or not.

That check wants a !rt_task(current) qualifier anyway.

-Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Mike Galbraith

author

2016-01-19 05:14 UTC #12 lore raw

Quoted 23 lines · Mike Galbraith

Quoted 6 lines · Sebastian Andrzej Siewior
Quoted 6 lines
kernel/sched/core.c +0 -0
@@ -3542,6 +3542,15 @@ asmlinkage __visible void __sched notrac
 	if (likely(!preemptible()))
 		return;
 
Quoted 14 lines
kernel/sched/core.c asmlinkage __visible void __sched notrac · line 3553
Quoted 9 lines
kernel/sched/core.c +9 -0
+#ifdef CONFIG_PREEMPT_LAZY
+	/*
+	 * Check for lazy preemption
+	 */
+	if (current_thread_info()->preempt_lazy_count &&
+	    !test_thread_flag(TIF_NEED_RESCHED))
+		return;
+#endif
+
And this is a new piece. So you forbid that tasks leave the CPU if lazy_count > 0. Let me look closed why this is happening and if this is
v4.1 … v4.4 or not.

Quoted 1 line

That check wants a !rt_task(current) qualifier anyway.

(bah, -ESWILLMORECOFFEE)
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Grygorii Strashko

2016-01-22 11:58 UTC #13 lore raw

In reply to Sebastian Andrzej Siewior

Quoted 6 lines · Sebastian Andrzej Siewior

Quoted 4 lines · * Mike Galbraith | 2016-01-18 10:08:23 [+0100]:
arch/x86/entry/common.c +0 -0
@@ -220,14 +220,14 @@ long syscall_trace_enter(struct pt_regs
 

Quoted 9 lines

arch/x86/entry/common.c long syscall_trace_enter(struct pt_regs · line 223
Quoted 4 lines
arch/x86/entry/common.c +1 -1
 #define EXIT_TO_USERMODE_LOOP_FLAGS				\
 	(_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE |	\
-	 _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
+	 _TIF_NEED_RESCHED_MASK | _TIF_USER_RETURN_NOTIFY)
If I read this right, the loop where this define is used _TIF_ALLWORK_MASK in v4.1 of which _TIF_NEED_RESCHED_MASK was part of. Adding this will reassmeble the old behaviour.
…

Just a question (sorry if dumb). ARM has _TIF_WORK_MASK defined:
#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME | _TIF_UPROBE)

which is used to calculate loop exit condition in
do_work_pending() (arch/arm/kernel/signal.c).

Should _TIF_WORK_MASK also contain _TIF_NEED_RESCHED_LAZY?

Signature · 6 lines

regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sebastian Andrzej Siewior

2016-01-22 12:54 UTC #14 lore raw

Quoted 17 lines · Grygorii Strashko

Quoted 6 lines · Sebastian Andrzej Siewior
Quoted 4 lines · * Mike Galbraith | 2016-01-18 10:08:23 [+0100]:
arch/x86/entry/common.c +0 -0
@@ -220,14 +220,14 @@ long syscall_trace_enter(struct pt_regs
 
Quoted 9 lines
arch/x86/entry/common.c long syscall_trace_enter(struct pt_regs · line 223
Quoted 4 lines
arch/x86/entry/common.c +1 -1
 #define EXIT_TO_USERMODE_LOOP_FLAGS                \
    (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_UPROBE |    \
-     _TIF_NEED_RESCHED | _TIF_USER_RETURN_NOTIFY)
+     _TIF_NEED_RESCHED_MASK | _TIF_USER_RETURN_NOTIFY)
If I read this right, the loop where this define is used _TIF_ALLWORK_MASK in v4.1 of which _TIF_NEED_RESCHED_MASK was part of. Adding this will reassmeble the old behaviour.
…

Quoted 8 lines

Just a question (sorry if dumb). ARM has _TIF_WORK_MASK defined:
#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME | _TIF_UPROBE)
which is used to calculate loop exit condition in
do_work_pending() (arch/arm/kernel/signal.c).
Should _TIF_WORK_MASK also contain _TIF_NEED_RESCHED_LAZY?

Yes, and arm64 lacks the same bits.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sebastian Andrzej Siewior

2016-01-22 20:40 UTC #15 lore raw

Quoted 3 lines · * Sebastian Andrzej Siewior | 2016-01-22 13:54:43 [+0100]:

Quoted 1 line
Should _TIF_WORK_MASK also contain _TIF_NEED_RESCHED_LAZY?
Yes, and arm64 lacks the same bits.

That would be this. If a compiler is reading here and knows how to improve the following, please let me know :)

arch/arm/include/asm/thread_info.h +4 -3

index 46cc07b5cae6..1f36a4eccc72 100644
@@ -143,8 +143,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
 #define TIF_SYSCALL_TRACE	4	/* syscall trace active */
 #define TIF_SYSCALL_AUDIT	5	/* syscall auditing active */
 #define TIF_SYSCALL_TRACEPOINT	6	/* syscall tracepoint instrumentation */
-#define TIF_SECCOMP		7	/* seccomp syscall filtering active */
-#define TIF_NEED_RESCHED_LAZY	8
+#define TIF_SECCOMP		8	/* seccomp syscall filtering active */
+#define TIF_NEED_RESCHED_LAZY	7
 
 #define TIF_NOHZ		12	/* in adaptive nohz mode */
 #define TIF_USING_IWMMXT	17

@@ -170,7 +170,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
  * Change these and you break ASM code in entry-common.S
  */
 #define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
-				 _TIF_NOTIFY_RESUME | _TIF_UPROBE)
+				 _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
+				 _TIF_NEED_RESCHED_LAZY)
 
 #endif /* __KERNEL__ */
 #endif /* __ASM_ARM_THREAD_INFO_H */

arch/arm/kernel/entry-common.S +7 -2

index 30a7228eaceb..c3bd6cbfce4b 100644
@@ -36,7 +36,9 @@
  UNWIND(.cantunwind	)
 	disable_irq_notrace			@ disable interrupts
 	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
-	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+	tst	r1, #((_TIF_SYSCALL_WORK | _TIF_WORK_MASK) & ~_TIF_SECCOMP)
+	bne	fast_work_pending
+	tst	r1, #_TIF_SECCOMP
 	bne	fast_work_pending
 
 	/* perform architecture specific actions before user return */

@@ -62,8 +64,11 @@ ENDPROC(ret_fast_syscall)
 	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
 	disable_irq_notrace			@ disable interrupts
 	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
-	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+	tst	r1, #((_TIF_SYSCALL_WORK | _TIF_WORK_MASK) & ~_TIF_SECCOMP)
+	bne 	do_slower_path
+	tst	r1, #_TIF_SECCOMP
 	beq	no_work_pending
+do_slower_path:
  UNWIND(.fnend		)
 ENDPROC(ret_fast_syscall)

arch/arm64/include/asm/thread_info.h +2 -1

index b229ef49a643..5f4e89fbc290 100644
@@ -129,7 +129,8 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_32BIT		(1 << TIF_32BIT)
 
 #define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
-				 _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE)
+				 _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \
+				 _TIF_NEED_RESCHED_LAZY)
 
 #define _TIF_SYSCALL_WORK	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \

Signature · 1 line

2.7.0.rc3

Grygorii Strashko

2016-01-25 17:14 UTC #16 lore raw

Quoted 39 lines · Sebastian Andrzej Siewior

Quoted 3 lines · * Sebastian Andrzej Siewior | 2016-01-22 13:54:43 [+0100]:

Quoted 1 line
Should _TIF_WORK_MASK also contain _TIF_NEED_RESCHED_LAZY?
Yes, and arm64 lacks the same bits.

That would be this. If a compiler is reading here and knows how to improve the following, please let me know :)

arch/arm/include/asm/thread_info.h +4 -3

index 46cc07b5cae6..1f36a4eccc72 100644
@@ -143,8 +143,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
  #define TIF_SYSCALL_TRACE	4	/* syscall trace active */
  #define TIF_SYSCALL_AUDIT	5	/* syscall auditing active */
  #define TIF_SYSCALL_TRACEPOINT	6	/* syscall tracepoint instrumentation */
-#define TIF_SECCOMP		7	/* seccomp syscall filtering active */
-#define TIF_NEED_RESCHED_LAZY	8
+#define TIF_SECCOMP		8	/* seccomp syscall filtering active */
+#define TIF_NEED_RESCHED_LAZY	7
 
  #define TIF_NOHZ		12	/* in adaptive nohz mode */
  #define TIF_USING_IWMMXT	17

@@ -170,7 +170,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *,
   * Change these and you break ASM code in entry-common.S
   */
  #define _TIF_WORK_MASK		(_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
-				 _TIF_NOTIFY_RESUME | _TIF_UPROBE)
+				 _TIF_NOTIFY_RESUME | _TIF_UPROBE | \
+				 _TIF_NEED_RESCHED_LAZY)
 
  #endif /* __KERNEL__ */
  #endif /* __ASM_ARM_THREAD_INFO_H */

arch/arm/kernel/entry-common.S +0 -0

index 30a7228eaceb..c3bd6cbfce4b 100644

arch/arm/kernel/entry-common.S line 40

Quoted 6 lines

arch/arm/kernel/entry-common.S +2 -1

@@ -36,7 +36,9 @@
   UNWIND(.cantunwind	)
  	disable_irq_notrace			@ disable interrupts
  	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
-	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+	tst	r1, #((_TIF_SYSCALL_WORK | _TIF_WORK_MASK) & ~_TIF_SECCOMP)
+	bne	fast_work_pending

		^^^^^^^^^^^^^^^^

arch/arm/kernel/entry-common.S line 42

Quoted 2 lines

arch/arm/kernel/entry-common.S +1 -0
+	tst	r1, #_TIF_SECCOMP
  	bne	fast_work_pending

		^^^^^^^^^^^^^^^^^

Should it be "fast_work_pending" in both cases?

Quoted 15 lines

arch/arm/kernel/entry-common.S +4 -1

  	/* perform architecture specific actions before user return */

@@ -62,8 +64,11 @@ ENDPROC(ret_fast_syscall)
  	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
  	disable_irq_notrace			@ disable interrupts
  	ldr	r1, [tsk, #TI_FLAGS]		@ re-check for syscall tracing
-	tst	r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+	tst	r1, #((_TIF_SYSCALL_WORK | _TIF_WORK_MASK) & ~_TIF_SECCOMP)
+	bne 	do_slower_path
+	tst	r1, #_TIF_SECCOMP
  	beq	no_work_pending
+do_slower_path:
   UNWIND(.fnend		)
  ENDPROC(ret_fast_syscall)

[...]

Signature · 2 lines

regards,
-grygorii

Sebastian Andrzej Siewior

2016-02-01 13:04 UTC #17 lore raw

Quoted 14 lines · Grygorii Strashko

arch/arm/kernel/entry-common.S line 40

Quoted 12 lines

arch/arm/kernel/entry-common.S +2 -1

index 30a7228eaceb..c3bd6cbfce4b 100644
@@ -36,7 +36,9 @@
   UNWIND(.cantunwind    )
      disable_irq_notrace            @ disable interrupts
      ldr    r1, [tsk, #TI_FLAGS]        @ re-check for syscall tracing
-    tst    r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
+    tst    r1, #((_TIF_SYSCALL_WORK | _TIF_WORK_MASK) & ~_TIF_SECCOMP)
+    bne    fast_work_pending

        ^^^^^^^^^^^^^^^^

Quoted 5 lines

arch/arm/kernel/entry-common.S line 42
Quoted 2 lines
arch/arm/kernel/entry-common.S +1 -0
+    tst    r1, #_TIF_SECCOMP
      bne    fast_work_pending
        ^^^^^^^^^^^^^^^^^
Should it be "fast_work_pending" in both cases?

Yes, I think so and it is, isn't it?
The original is

tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
bne fast_work_pending

and with the LAZY bits in it, the ASM opcode no longer works because the constant is too large (or has to many bits set). So I removed the _TIF_SECCOMP bit out of the mask and made two compare & jumps here.

Sebastian