* [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible
@ 2017-07-07 9:08 Wanpeng Li
2017-07-07 12:01 ` Frederic Weisbecker
0 siblings, 1 reply; 3+ messages in thread
From: Wanpeng Li @ 2017-07-07 9:08 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
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible
2017-07-07 9:08 [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li
@ 2017-07-07 12:01 ` Frederic Weisbecker
2017-07-07 12:17 ` Wanpeng Li
0 siblings, 1 reply; 3+ messages in thread
From: Frederic Weisbecker @ 2017-07-07 12:01 UTC (permalink / raw)
To: Wanpeng Li
Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Wanpeng Li,
Thomas Gleixner, Luiz Capitulino, Rik van Riel
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible
2017-07-07 12:01 ` Frederic Weisbecker
@ 2017-07-07 12:17 ` Wanpeng Li
0 siblings, 0 replies; 3+ messages in thread
From: Wanpeng Li @ 2017-07-07 12:17 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, Ingo Molnar, Peter Zijlstra, Wanpeng Li,
Thomas Gleixner, Luiz Capitulino, Rik van Riel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-07-07 12:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 9:08 [PATCH v2] sched/cputime: Fix using smp_processor_id() in preemptible Wanpeng Li
2017-07-07 12:01 ` Frederic Weisbecker
2017-07-07 12:17 ` Wanpeng Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.