From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:35906 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752309AbeGBPPW (ORCPT ); Mon, 2 Jul 2018 11:15:22 -0400 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w62FEC88116180 for ; Mon, 2 Jul 2018 11:15:21 -0400 Received: from e17.ny.us.ibm.com (e17.ny.us.ibm.com [129.33.205.207]) by mx0b-001b2d01.pphosted.com with ESMTP id 2jym8fp6yk-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 02 Jul 2018 11:15:21 -0400 Received: from localhost by e17.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 2 Jul 2018 11:15:20 -0400 Date: Mon, 2 Jul 2018 08:17:28 -0700 From: "Paul E. McKenney" To: Frederic Weisbecker Cc: LKML , Peter Zijlstra , Thomas Gleixner , Ingo Molnar , stable@vger.kernel.org, Anna-Maria Gleixner , Jacek Tomaka Subject: Re: [PATCH] sched/nohz: Skip remote tick on idle task entirely Reply-To: paulmck@linux.vnet.ibm.com References: <1530203381-31234-1-git-send-email-frederic@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1530203381-31234-1-git-send-email-frederic@kernel.org> Message-Id: <20180702151728.GO3593@linux.vnet.ibm.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Jun 28, 2018 at 06:29:41PM +0200, Frederic Weisbecker wrote: > Some people have reported that the warning in sched_tick_remote() > occasionally triggers, especially in favour of some RCU-Torture > pressure: > > WARNING: CPU: 11 PID: 906 at kernel/sched/core.c:3138 sched_tick_remote+0xb6/0xc0 > Modules linked in: > CPU: 11 PID: 906 Comm: kworker/u32:3 Not tainted 4.18.0-rc2+ #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 > Workqueue: events_unbound sched_tick_remote > RIP: 0010:sched_tick_remote+0xb6/0xc0 > Code: e8 0f 06 b8 00 c6 03 00 fb eb 9d 8b 43 04 85 c0 75 8d 48 8b 83 e0 0a 00 00 48 85 c0 75 81 eb 88 48 89 df e8 bc fe ff ff eb aa <0f> 0b eb > +c5 66 0f 1f 44 00 00 bf 17 00 00 00 e8 b6 2e fe ff 0f b6 > Call Trace: > process_one_work+0x1df/0x3b0 > worker_thread+0x44/0x3d0 > kthread+0xf3/0x130 > ? set_worker_desc+0xb0/0xb0 > ? kthread_create_worker_on_cpu+0x70/0x70 > ret_from_fork+0x35/0x40 > > This happens when the remote tick applies on an idle task. Usually the > idle_cpu() check avoids that, but it is performed before we lock the > runqueue and it is therefore racy. It was intended to be that way in > order to prevent from useless runqueue locks since idle task tick > callback is a no-op. > > Now if the racy check slips out of our hands and we end up remotely > ticking an idle task, the empty task_tick_idle() is harmless. Still > it won't pass the WARN_ON_ONCE() test that ensures rq_clock_task() is > not too far from curr->se.exec_start because update_curr_idle() doesn't > update the exec_start value like other scheduler policies. Hence the > reported false positive. > > So let's have another check, while the rq is locked, to make sure we > don't remote tick on an idle task. The lockless idle_cpu() still applies > to avoid unecessary rq lock contention. > > Reported-by: Jacek Tomaka > Reported-by: Paul E. McKenney > Reported-by: Anna-Maria Gleixner > Cc: Thomas Gleixner > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: stable@vger.kernel.org > Signed-off-by: Frederic Weisbecker Tested-by: Paul E. McKenney > --- > kernel/sched/core.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 78d8fac..da8f121 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3127,16 +3127,18 @@ static void sched_tick_remote(struct work_struct *work) > u64 delta; > > rq_lock_irq(rq, &rf); > - update_rq_clock(rq); > curr = rq->curr; > - delta = rq_clock_task(rq) - curr->se.exec_start; > + if (!is_idle_task(curr)) { > + update_rq_clock(rq); > + delta = rq_clock_task(rq) - curr->se.exec_start; > > - /* > - * Make sure the next tick runs within a reasonable > - * amount of time. > - */ > - WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3); > - curr->sched_class->task_tick(rq, curr, 0); > + /* > + * Make sure the next tick runs within a reasonable > + * amount of time. > + */ > + WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3); > + curr->sched_class->task_tick(rq, curr, 0); > + } > rq_unlock_irq(rq, &rf); > } > > -- > 2.7.4 >