Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1683700 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2017-07-09 09:50 +0200 |
| Last post | 2017-07-17 04:40 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li <kernellwp@gmail.com> - 2017-07-09 09:50 +0200
Re: [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li <kernellwp@gmail.com> - 2017-07-14 09:00 +0200
[tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context tip-bot for Wanpeng Li <tipbot@zytor.com> - 2017-07-14 11:20 +0200
Re: [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context Frederic Weisbecker <fweisbec@gmail.com> - 2017-07-17 04:40 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-07-09 09:50 +0200 |
| Subject | [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible |
| Message-ID | <u1f8B-4SC-1@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-07-14 09:00 +0200 |
| Message-ID | <u32JY-88H-7@gated-at.bofh.it> |
| In reply to | #1683700 |
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
>
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Wanpeng Li <tipbot@zytor.com> |
|---|---|
| Date | 2017-07-14 11:20 +0200 |
| Subject | [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context |
| Message-ID | <u34Vt-1k8-11@gated-at.bofh.it> |
| In reply to | #1683700 |
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);
}
[toc] | [prev] | [next] | [standalone]
| From | Frederic Weisbecker <fweisbec@gmail.com> |
|---|---|
| Date | 2017-07-17 04:40 +0200 |
| Subject | Re: [tip:sched/urgent] sched/cputime: Don't use smp_processor_id() in preemptible context |
| Message-ID | <u446Z-7v6-7@gated-at.bofh.it> |
| In reply to | #1687175 |
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!
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web