Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965900AbdGTWdD (ORCPT ); Thu, 20 Jul 2017 18:33:03 -0400 Received: from mail-qk0-f194.google.com ([209.85.220.194]:35464 "EHLO mail-qk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965270AbdGTWdB (ORCPT ); Thu, 20 Jul 2017 18:33:01 -0400 MIME-Version: 1.0 In-Reply-To: References: <1499586028-7402-1-git-send-email-wanpeng.li@hotmail.com> From: Paul McKenney Date: Thu, 20 Jul 2017 15:33:00 -0700 Message-ID: Subject: Re: [PATCH v3] sched/cputime: Fix using smp_processor_id() in preemptible To: Wanpeng Li Cc: "linux-kernel@vger.kernel.org" , Ingo Molnar , Peter Zijlstra , Wanpeng Li , Thomas Gleixner , Luiz Capitulino , Frederic Weisbecker , Rik van Riel Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3450 Lines: 98 On Wed, Jul 19, 2017 at 4:38 PM, Paul McKenney wrote: > On Thu, Jul 13, 2017 at 11:49 PM, Wanpeng Li wrote: >> >> Ping for the merge window. :) >> 2017-07-09 15:40 GMT+08:00 Wanpeng Li : >> > From: Wanpeng Li >> > >> > 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 Thanx, Paul >> > Reported-by: Xiaolong Ye >> > Cc: Thomas Gleixner >> > Cc: Luiz Capitulino >> > Cc: Frederic Weisbecker >> > Cc: Rik van Riel >> > Cc: Peter Zijlstra >> > Cc: Ingo Molnar >> > Signed-off-by: Wanpeng Li >> > --- >> > 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 >> > > >