Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311384 > unrolled thread

[rfc patch v4.4-rt2] sched: fix up preempt lazy forward port

Back to article view | Back to linux.kernel

Contents

  [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-18 10:10 +0100
    Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-18 21:20 +0100
      Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-19 03:40 +0100
        Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-21 14:00 +0100
          Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-22 13:30 +0100
            Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-22 14:20 +0100
              Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-22 14:40 +0100
                Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-22 15:20 +0100
                  Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-22 16:10 +0100
                    Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-22 19:30 +0100
      Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-19 05:50 +0100
        Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Mike Galbraith <umgwanakikbuti@gmail.com> - 2016-01-19 06:20 +0100
      Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-22 13:00 +0100
        Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-22 14:00 +0100
          Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-01-22 21:50 +0100
            Re: [rfc patch v4.4-rt2] sched: fix up preempt lazy forward port Grygorii Strashko <grygorii.strashko@ti.com> - 2016-01-25 18:20 +0100

#1311384 — [rfc patch v4.4-rt2] 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 <umgwanakikbuti@gmail.com>
---
 arch/x86/entry/common.c |    4 ++--
 kernel/sched/core.c     |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -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.
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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);

[toc] | [next] | [standalone]

#1311790

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

>--- a/arch/x86/entry/common.c
>+++ b/arch/x86/entry/common.c
>@@ -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.
…

>--- a/kernel/sched/core.c
>+++ b/kernel/sched/core.c
>@@ -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
>+

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.

> 	preempt_schedule_common();
> }
> NOKPROBE_SYMBOL(preempt_schedule);

Sebastian

[toc] | [prev] | [next] | [standalone]

#1311901

On Mon, 2016-01-18 at 21:18 +0100, Sebastian Andrzej Siewior wrote:

> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -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
> > +
> 
> 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

[toc] | [prev] | [next] | [standalone]

#1314173

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

>> 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 <umgwanakikbuti@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/sched/core.c |   36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -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();
 		/*

[toc] | [prev] | [next] | [standalone]

#1314949

On 01/21/2016 02:54 PM, Sebastian Andrzej Siewior wrote:
> * Mike Galbraith | 2016-01-19 03:29:57 [+0100]:
> 
>>> 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 <umgwanakikbuti@gmail.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   kernel/sched/core.c |   36 ++++++++++++++++++++++++++++--------
>   1 file changed, 28 insertions(+), 8 deletions(-)
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -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();
>   		/*

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

 


-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1314971

On 01/22/2016 01:24 PM, Grygorii Strashko wrote:
> 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

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

Sebastian

[toc] | [prev] | [next] | [standalone]

#1314979

On 01/22/2016 03:19 PM, Sebastian Andrzej Siewior wrote:
> On 01/22/2016 01:24 PM, Grygorii Strashko wrote:
>> 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
>
> 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

-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1315016

On 01/22/2016 02:34 PM, Grygorii Strashko wrote:
>> 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

[toc] | [prev] | [next] | [standalone]

#1315062

On 01/22/2016 04:14 PM, Sebastian Andrzej Siewior wrote:
> On 01/22/2016 02:34 PM, Grygorii Strashko wrote:
>>> 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.
>

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

-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1315192

hi Sebastian,
On 01/22/2016 05:04 PM, Grygorii Strashko wrote:
> On 01/22/2016 04:14 PM, Sebastian Andrzej Siewior wrote:
>> On 01/22/2016 02:34 PM, Grygorii Strashko wrote:
>>>> 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.
>>
>
> 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.

-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1311941

On Mon, 2016-01-18 at 21:18 +0100, Sebastian Andrzej Siewior wrote:

> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -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
> > +
> 
> 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

[toc] | [prev] | [next] | [standalone]

#1311943

On Tue, 2016-01-19 at 05:41 +0100, Mike Galbraith wrote:
> On Mon, 2016-01-18 at 21:18 +0100, Sebastian Andrzej Siewior wrote:
> 
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -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
> > > +
> > 
> > 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.

(bah, -ESWILLMORECOFFEE)

[toc] | [prev] | [next] | [standalone]

#1314930

On 01/18/2016 10:18 PM, Sebastian Andrzej Siewior wrote:
> * Mike Galbraith | 2016-01-18 10:08:23 [+0100]:
>
>> --- a/arch/x86/entry/common.c
>> +++ b/arch/x86/entry/common.c
>> @@ -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.
> …

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?

-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1314964

On 01/22/2016 12:58 PM, Grygorii Strashko wrote:
> On 01/18/2016 10:18 PM, Sebastian Andrzej Siewior wrote:
>> * Mike Galbraith | 2016-01-18 10:08:23 [+0100]:
>>
>>> --- a/arch/x86/entry/common.c
>>> +++ b/arch/x86/entry/common.c
>>> @@ -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.
>> …
> 
> 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

[toc] | [prev] | [next] | [standalone]

#1315265

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

>> 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 :)

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 46cc07b5cae6..1f36a4eccc72 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -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 */
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 30a7228eaceb..c3bd6cbfce4b 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -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)
 
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index b229ef49a643..5f4e89fbc290 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -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 | \
-- 
2.7.0.rc3

[toc] | [prev] | [next] | [standalone]

#1317071

On 01/22/2016 10:40 PM, Sebastian Andrzej Siewior wrote:
> * Sebastian Andrzej Siewior | 2016-01-22 13:54:43 [+0100]:
>
>>> 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 :)
>
> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
> index 46cc07b5cae6..1f36a4eccc72 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -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 */
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 30a7228eaceb..c3bd6cbfce4b 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -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
		^^^^^^^^^^^^^^^^^

Should it be "fast_work_pending" in both cases?

>
>   	/* 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)
>
[...]

-- 
regards,
-grygorii

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web