* [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible
@ 2017-07-09 7:40 Wanpeng Li
2017-07-14 6:49 ` Wanpeng Li
2017-07-14 9:10 ` [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context tip-bot for Wanpeng Li
0 siblings, 2 replies; 5+ messages in thread
From: Wanpeng Li @ 2017-07-09 7:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Peter Zijlstra, Wanpeng Li, Thomas Gleixner,
Luiz Capitulino, Frederic Weisbecker, Rik van Riel
From: Wanpeng Li <wanpeng.li@hotmail.com>
BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181
caller is debug_smp_processor_id+0x17/0x19
CPU: 0 PID: 181 Comm: 99-trinity Not tainted 4.12.0-01059-g2a42eb9 #1
Call Trace:
dump_stack+0x82/0xb8
check_preemption_disabled+0xd1/0xe3
debug_smp_processor_id+0x17/0x19
vtime_delta+0xd/0x2c
task_cputime+0x89/0xdb
thread_group_cputime+0x11b/0x1ed
thread_group_cputime_adjusted+0x1f/0x47
wait_consider_task+0x2a9/0xaf9
? lock_acquire+0x97/0xa4
do_wait+0xdf/0x1f4
SYSC_wait4+0x8e/0xb5
? list_add+0x34/0x34
SyS_wait4+0x9/0xb
do_syscall_64+0x70/0x82
entry_SYSCALL64_slow_path+0x25/0x25
As Frederic pointed out:
| Although those sched_clock_cpu() things seem to only matter when the
| sched_clock() is unstable. And that stability is a condition for nohz_full
| to work anyway. So probably sched_clock() alone would be enough.
This patch fixes it by replacing sched_clock_cpu() by sched_clock() to
avoid to call smp_processor_id() in preemptible context.
Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
kernel/sched/cputime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 6e3ea4a..14d2dbf 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -683,7 +683,7 @@ static u64 vtime_delta(struct vtime *vtime)
{
unsigned long long clock;
- clock = sched_clock_cpu(smp_processor_id());
+ clock = sched_clock();
if (clock < vtime->starttime)
return 0;
@@ -814,7 +814,7 @@ void arch_vtime_task_switch(struct task_struct *prev)
write_seqcount_begin(&vtime->seqcount);
vtime->state = VTIME_SYS;
- vtime->starttime = sched_clock_cpu(smp_processor_id());
+ vtime->starttime = sched_clock();
write_seqcount_end(&vtime->seqcount);
}
@@ -826,7 +826,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
local_irq_save(flags);
write_seqcount_begin(&vtime->seqcount);
vtime->state = VTIME_SYS;
- vtime->starttime = sched_clock_cpu(cpu);
+ vtime->starttime = sched_clock();
write_seqcount_end(&vtime->seqcount);
local_irq_restore(flags);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible
2017-07-09 7:40 [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li
@ 2017-07-14 6:49 ` Wanpeng Li
[not found] ` <CAJzB8QHu3Jfy6+H_qxJd83696qsLBnJp0ke55iaJ-JMqZ0TAGQ@mail.gmail.com>
2017-07-14 9:10 ` [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context tip-bot for Wanpeng Li
1 sibling, 1 reply; 5+ messages in thread
From: Wanpeng Li @ 2017-07-14 6:49 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Peter Zijlstra, Wanpeng Li, Thomas Gleixner,
Luiz Capitulino, Frederic Weisbecker, Rik van Riel
Ping for the merge window. :)
2017-07-09 15:40 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181
> caller is debug_smp_processor_id+0x17/0x19
> CPU: 0 PID: 181 Comm: 99-trinity Not tainted 4.12.0-01059-g2a42eb9 #1
> Call Trace:
> dump_stack+0x82/0xb8
> check_preemption_disabled+0xd1/0xe3
> debug_smp_processor_id+0x17/0x19
> vtime_delta+0xd/0x2c
> task_cputime+0x89/0xdb
> thread_group_cputime+0x11b/0x1ed
> thread_group_cputime_adjusted+0x1f/0x47
> wait_consider_task+0x2a9/0xaf9
> ? lock_acquire+0x97/0xa4
> do_wait+0xdf/0x1f4
> SYSC_wait4+0x8e/0xb5
> ? list_add+0x34/0x34
> SyS_wait4+0x9/0xb
> do_syscall_64+0x70/0x82
> entry_SYSCALL64_slow_path+0x25/0x25
>
> As Frederic pointed out:
>
> | Although those sched_clock_cpu() things seem to only matter when the
> | sched_clock() is unstable. And that stability is a condition for nohz_full
> | to work anyway. So probably sched_clock() alone would be enough.
>
> This patch fixes it by replacing sched_clock_cpu() by sched_clock() to
> avoid to call smp_processor_id() in preemptible context.
>
> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> kernel/sched/cputime.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 6e3ea4a..14d2dbf 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -683,7 +683,7 @@ static u64 vtime_delta(struct vtime *vtime)
> {
> unsigned long long clock;
>
> - clock = sched_clock_cpu(smp_processor_id());
> + clock = sched_clock();
> if (clock < vtime->starttime)
> return 0;
>
> @@ -814,7 +814,7 @@ void arch_vtime_task_switch(struct task_struct *prev)
>
> write_seqcount_begin(&vtime->seqcount);
> vtime->state = VTIME_SYS;
> - vtime->starttime = sched_clock_cpu(smp_processor_id());
> + vtime->starttime = sched_clock();
> write_seqcount_end(&vtime->seqcount);
> }
>
> @@ -826,7 +826,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
> local_irq_save(flags);
> write_seqcount_begin(&vtime->seqcount);
> vtime->state = VTIME_SYS;
> - vtime->starttime = sched_clock_cpu(cpu);
> + vtime->starttime = sched_clock();
> write_seqcount_end(&vtime->seqcount);
> local_irq_restore(flags);
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context
2017-07-09 7:40 [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li
2017-07-14 6:49 ` Wanpeng Li
@ 2017-07-14 9:10 ` tip-bot for Wanpeng Li
2017-07-17 2:31 ` Frederic Weisbecker
1 sibling, 1 reply; 5+ messages in thread
From: tip-bot for Wanpeng Li @ 2017-07-14 9:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: wanpeng.li, torvalds, riel, peterz, tglx, hpa, lcapitulino,
linux-kernel, xiaolong.ye, fweisbec, mingo
Commit-ID: 0e4097c3354e2f5a5ad8affd9dc7f7f7d00bb6b9
Gitweb: http://git.kernel.org/tip/0e4097c3354e2f5a5ad8affd9dc7f7f7d00bb6b9
Author: Wanpeng Li <wanpeng.li@hotmail.com>
AuthorDate: Sun, 9 Jul 2017 00:40:28 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 14 Jul 2017 10:27:15 +0200
sched/cputime: Don't use smp_processor_id() in preemptible context
Recent kernels trigger this warning:
BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181
caller is debug_smp_processor_id+0x17/0x19
CPU: 0 PID: 181 Comm: 99-trinity Not tainted 4.12.0-01059-g2a42eb9 #1
Call Trace:
dump_stack+0x82/0xb8
check_preemption_disabled()
debug_smp_processor_id()
vtime_delta()
task_cputime()
thread_group_cputime()
thread_group_cputime_adjusted()
wait_consider_task()
do_wait()
SYSC_wait4()
do_syscall_64()
entry_SYSCALL64_slow_path()
As Frederic pointed out:
| Although those sched_clock_cpu() things seem to only matter when the
| sched_clock() is unstable. And that stability is a condition for nohz_full
| to work anyway. So probably sched_clock() alone would be enough.
This patch fixes it by replacing sched_clock_cpu() with sched_clock() to
avoid calling smp_processor_id() in a preemptible context.
Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1499586028-7402-1-git-send-email-wanpeng.li@hotmail.com
[ Prettified the changelog. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/cputime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 6e3ea4a..14d2dbf 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -683,7 +683,7 @@ static u64 vtime_delta(struct vtime *vtime)
{
unsigned long long clock;
- clock = sched_clock_cpu(smp_processor_id());
+ clock = sched_clock();
if (clock < vtime->starttime)
return 0;
@@ -814,7 +814,7 @@ void arch_vtime_task_switch(struct task_struct *prev)
write_seqcount_begin(&vtime->seqcount);
vtime->state = VTIME_SYS;
- vtime->starttime = sched_clock_cpu(smp_processor_id());
+ vtime->starttime = sched_clock();
write_seqcount_end(&vtime->seqcount);
}
@@ -826,7 +826,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
local_irq_save(flags);
write_seqcount_begin(&vtime->seqcount);
vtime->state = VTIME_SYS;
- vtime->starttime = sched_clock_cpu(cpu);
+ vtime->starttime = sched_clock();
write_seqcount_end(&vtime->seqcount);
local_irq_restore(flags);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context
2017-07-14 9:10 ` [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context tip-bot for Wanpeng Li
@ 2017-07-17 2:31 ` Frederic Weisbecker
0 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2017-07-17 2:31 UTC (permalink / raw)
To: riel, peterz, wanpeng.li, torvalds, xiaolong.ye, mingo, tglx,
hpa, lcapitulino, linux-kernel
Cc: linux-tip-commits
On Fri, Jul 14, 2017 at 02:10:14AM -0700, tip-bot for Wanpeng Li wrote:
> Commit-ID: 0e4097c3354e2f5a5ad8affd9dc7f7f7d00bb6b9
> Gitweb: http://git.kernel.org/tip/0e4097c3354e2f5a5ad8affd9dc7f7f7d00bb6b9
> Author: Wanpeng Li <wanpeng.li@hotmail.com>
> AuthorDate: Sun, 9 Jul 2017 00:40:28 -0700
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitDate: Fri, 14 Jul 2017 10:27:15 +0200
>
> sched/cputime: Don't use smp_processor_id() in preemptible context
>
> Recent kernels trigger this warning:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181
> caller is debug_smp_processor_id+0x17/0x19
> CPU: 0 PID: 181 Comm: 99-trinity Not tainted 4.12.0-01059-g2a42eb9 #1
> Call Trace:
> dump_stack+0x82/0xb8
> check_preemption_disabled()
> debug_smp_processor_id()
> vtime_delta()
> task_cputime()
> thread_group_cputime()
> thread_group_cputime_adjusted()
> wait_consider_task()
> do_wait()
> SYSC_wait4()
> do_syscall_64()
> entry_SYSCALL64_slow_path()
>
> As Frederic pointed out:
>
> | Although those sched_clock_cpu() things seem to only matter when the
> | sched_clock() is unstable. And that stability is a condition for nohz_full
> | to work anyway. So probably sched_clock() alone would be enough.
>
> This patch fixes it by replacing sched_clock_cpu() with sched_clock() to
> avoid calling smp_processor_id() in a preemptible context.
>
> Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/1499586028-7402-1-git-send-email-wanpeng.li@hotmail.com
> [ Prettified the changelog. ]
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
Thanks! The patch looks good!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible
[not found] ` <CAJzB8QHu3Jfy6+H_qxJd83696qsLBnJp0ke55iaJ-JMqZ0TAGQ@mail.gmail.com>
@ 2017-07-20 22:33 ` Paul McKenney
0 siblings, 0 replies; 5+ messages in thread
From: Paul McKenney @ 2017-07-20 22:33 UTC (permalink / raw)
To: Wanpeng Li
Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Wanpeng Li,
Thomas Gleixner, Luiz Capitulino, Frederic Weisbecker,
Rik van Riel
On Wed, Jul 19, 2017 at 4:38 PM, Paul McKenney <paulmckrcu@gmail.com> wrote:
> On Thu, Jul 13, 2017 at 11:49 PM, Wanpeng Li <kernellwp@gmail.com> wrote:
>>
>> Ping for the merge window. :)
>> 2017-07-09 15:40 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>> > From: Wanpeng Li <wanpeng.li@hotmail.com>
>> >
>> > BUG: using smp_processor_id() in preemptible [00000000] code:
>> > 99-trinity/181
>> > caller is debug_smp_processor_id+0x17/0x19
>> > CPU: 0 PID: 181 Comm: 99-trinity Not tainted 4.12.0-01059-g2a42eb9 #1
>> > Call Trace:
>> > dump_stack+0x82/0xb8
>> > check_preemption_disabled+0xd1/0xe3
>> > debug_smp_processor_id+0x17/0x19
>> > vtime_delta+0xd/0x2c
>> > task_cputime+0x89/0xdb
>> > thread_group_cputime+0x11b/0x1ed
>> > thread_group_cputime_adjusted+0x1f/0x47
>> > wait_consider_task+0x2a9/0xaf9
>> > ? lock_acquire+0x97/0xa4
>> > do_wait+0xdf/0x1f4
>> > SYSC_wait4+0x8e/0xb5
>> > ? list_add+0x34/0x34
>> > SyS_wait4+0x9/0xb
>> > do_syscall_64+0x70/0x82
>> > entry_SYSCALL64_slow_path+0x25/0x25
>> >
>> > As Frederic pointed out:
>> >
>> > | Although those sched_clock_cpu() things seem to only matter when the
>> > | sched_clock() is unstable. And that stability is a condition for
>> > nohz_full
>> > | to work anyway. So probably sched_clock() alone would be enough.
>> >
>> > This patch fixes it by replacing sched_clock_cpu() by sched_clock() to
>> > avoid to call smp_processor_id() in preemptible context.
>
>
> I am hitting this with rcutorture, so have kicked off an overnight
> run with this patch.
Which passed, so:
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Thanx, Paul
>> > Reported-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: Luiz Capitulino <lcapitulino@redhat.com>
>> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> > Cc: Rik van Riel <riel@redhat.com>
>> > Cc: Peter Zijlstra <peterz@infradead.org>
>> > Cc: Ingo Molnar <mingo@kernel.org>
>> > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> > ---
>> > kernel/sched/cputime.c | 6 +++---
>> > 1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
>> > index 6e3ea4a..14d2dbf 100644
>> > --- a/kernel/sched/cputime.c
>> > +++ b/kernel/sched/cputime.c
>> > @@ -683,7 +683,7 @@ static u64 vtime_delta(struct vtime *vtime)
>> > {
>> > unsigned long long clock;
>> >
>> > - clock = sched_clock_cpu(smp_processor_id());
>> > + clock = sched_clock();
>> > if (clock < vtime->starttime)
>> > return 0;
>> >
>> > @@ -814,7 +814,7 @@ void arch_vtime_task_switch(struct task_struct
>> > *prev)
>> >
>> > write_seqcount_begin(&vtime->seqcount);
>> > vtime->state = VTIME_SYS;
>> > - vtime->starttime = sched_clock_cpu(smp_processor_id());
>> > + vtime->starttime = sched_clock();
>> > write_seqcount_end(&vtime->seqcount);
>> > }
>> >
>> > @@ -826,7 +826,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
>> > local_irq_save(flags);
>> > write_seqcount_begin(&vtime->seqcount);
>> > vtime->state = VTIME_SYS;
>> > - vtime->starttime = sched_clock_cpu(cpu);
>> > + vtime->starttime = sched_clock();
>> > write_seqcount_end(&vtime->seqcount);
>> > local_irq_restore(flags);
>> > }
>> > --
>> > 2.7.4
>> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-20 22:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-09 7:40 [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li
2017-07-14 6:49 ` Wanpeng Li
[not found] ` <CAJzB8QHu3Jfy6+H_qxJd83696qsLBnJp0ke55iaJ-JMqZ0TAGQ@mail.gmail.com>
2017-07-20 22:33 ` Paul McKenney
2017-07-14 9:10 ` [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context tip-bot for Wanpeng Li
2017-07-17 2:31 ` Frederic Weisbecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).