Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1683050 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2017-07-07 11:10 +0200 |
| Last post | 2017-07-07 14:20 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li <kernellwp@gmail.com> - 2017-07-07 11:10 +0200
Re: [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible Frederic Weisbecker <fweisbec@gmail.com> - 2017-07-07 14:10 +0200
Re: [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li <kernellwp@gmail.com> - 2017-07-07 14:20 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-07-07 11:10 +0200 |
| Subject | [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible |
| Message-ID | <u0xqX-1Re-21@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
This patch fixes it by replacing sched_clock_cpu() in vtime_delta() by
local_clock() for effectively raw_smp_processor_id().
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>
---
v1 -> v2:
* replace sched_clock_cpu() by local_clock()
kernel/sched/cputime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 6e3ea4a..d86b94e 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 = local_clock();
if (clock < vtime->starttime)
return 0;
--
2.7.4
[toc] | [next] | [standalone]
| From | Frederic Weisbecker <fweisbec@gmail.com> |
|---|---|
| Date | 2017-07-07 14:10 +0200 |
| Subject | Re: [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible |
| Message-ID | <u0Af8-3Q5-21@gated-at.bofh.it> |
| In reply to | #1683050 |
On Fri, Jul 07, 2017 at 02:08:25AM -0700, Wanpeng Li wrote: > 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 > > This patch fixes it by replacing sched_clock_cpu() in vtime_delta() by > local_clock() for effectively raw_smp_processor_id(). That's also broken because task_cputime() can be called from a different CPU than where the target task is running on, even though there shouldn't be practical effect as the clock must be stable but still the code would be confusing. No I think you can still use sched_clock(), just make sure you also use it on arch_vtime_task_switch() and vtime_init_idle(). Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-07-07 14:20 +0200 |
| Message-ID | <u0AoN-3TA-1@gated-at.bofh.it> |
| In reply to | #1683152 |
2017-07-07 20:01 GMT+08:00 Frederic Weisbecker <fweisbec@gmail.com>: > On Fri, Jul 07, 2017 at 02:08:25AM -0700, Wanpeng Li wrote: >> 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 >> >> This patch fixes it by replacing sched_clock_cpu() in vtime_delta() by >> local_clock() for effectively raw_smp_processor_id(). > > That's also broken because task_cputime() can be called from a different CPU than > where the target task is running on, even though there shouldn't be practical effect Agreed. > as the clock must be stable but still the code would be confusing. > > No I think you can still use sched_clock(), just make sure you also use it on > arch_vtime_task_switch() and vtime_init_idle(). Is it acceptable to you, Peterz? :) Regards, Wanpeng Li
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web