From: Wanpeng Li <[email protected]>
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 <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Luiz Capitulino <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
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
On Fri, Jul 07, 2017 at 02:08:25AM -0700, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> 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.
2017-07-07 20:01 GMT+08:00 Frederic Weisbecker <[email protected]>:
> On Fri, Jul 07, 2017 at 02:08:25AM -0700, Wanpeng Li wrote:
>> From: Wanpeng Li <[email protected]>
>>
>> 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