From patchwork Tue Dec 16 04:53:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 42309 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f72.google.com (mail-la0-f72.google.com [209.85.215.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 9575E26C6C for ; Tue, 16 Dec 2014 04:53:23 +0000 (UTC) Received: by mail-la0-f72.google.com with SMTP id gq15sf8146369lab.11 for ; Mon, 15 Dec 2014 20:53:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe; bh=HpQWDfm6Lldo0yFPBex96vqKAG48TqkLfonKVtLZPKI=; b=QmT7TnfzDOlkFQay2B3E6DCFWtPGVkParRCpmCGZbjDMQ/hgyaQaAF2aW4HpygCd9x KI135COVg+P63lRtzVsSWwV7Ix302XNvwzR2YELpj34d63Wfqlp6asR7EKoFRnjQMWC+ PpdrBLbddMm4+wqynC++XdhiJSq46u4TzX/oon6+R3uy1O/C+N9CyA8MI/TM8pgt+02N Y0+rMkH3QZo3nNfFb8HUiSs5kql+XY5bydxFM6SeeXt7bwLql3DS/xQf/7Vby6kRwlEm aXCQBTnVIOjAkInopSQXg4wtEHlvRu7+qO11ie1d/a6zQ6zd9B1KfeR8didZfUNZl9wA ejNQ== X-Gm-Message-State: ALoCoQnhsBtEECQ7uHj+rbcZivatYjHW0t6lAan9ctuaIUqiA+YXthHPJC7arHQX9NIH2kw8++Ux X-Received: by 10.152.42.196 with SMTP id q4mr12441lal.6.1418705602543; Mon, 15 Dec 2014 20:53:22 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.153.7.71 with SMTP id da7ls837687lad.31.gmail; Mon, 15 Dec 2014 20:53:22 -0800 (PST) X-Received: by 10.153.5.33 with SMTP id cj1mr34261795lad.65.1418705601985; Mon, 15 Dec 2014 20:53:21 -0800 (PST) Received: from mail-lb0-f171.google.com (mail-lb0-f171.google.com. [209.85.217.171]) by mx.google.com with ESMTPS id jq17si12324267lab.78.2014.12.15.20.53.21 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 15 Dec 2014 20:53:21 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.171 as permitted sender) client-ip=209.85.217.171; Received: by mail-lb0-f171.google.com with SMTP id w7so6087875lbi.16 for ; Mon, 15 Dec 2014 20:53:21 -0800 (PST) X-Received: by 10.152.87.100 with SMTP id w4mr33779921laz.71.1418705601507; Mon, 15 Dec 2014 20:53:21 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.142.69 with SMTP id ru5csp924129lbb; Mon, 15 Dec 2014 20:53:20 -0800 (PST) X-Received: by 10.69.8.99 with SMTP id dj3mr24649388pbd.16.1418705599591; Mon, 15 Dec 2014 20:53:19 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id fc9si8742109pdb.211.2014.12.15.20.53.18 for ; Mon, 15 Dec 2014 20:53:19 -0800 (PST) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751513AbaLPExQ (ORCPT + 27 others); Mon, 15 Dec 2014 23:53:16 -0500 Received: from mail-ob0-f175.google.com ([209.85.214.175]:37918 "EHLO mail-ob0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750915AbaLPExO (ORCPT ); Mon, 15 Dec 2014 23:53:14 -0500 Received: by mail-ob0-f175.google.com with SMTP id wp4so21377720obc.6 for ; Mon, 15 Dec 2014 20:53:14 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.202.208.199 with SMTP id h190mr1951828oig.29.1418705594103; Mon, 15 Dec 2014 20:53:14 -0800 (PST) Received: by 10.182.139.37 with HTTP; Mon, 15 Dec 2014 20:53:14 -0800 (PST) In-Reply-To: <20141215234443.GA23297@lerouge> References: <20141211194204.GA19083@wfg-t540p.sh.intel.com> <548E8D01.9050707@linux.vnet.ibm.com> <20141215234443.GA23297@lerouge> Date: Tue, 16 Dec 2014 10:23:14 +0530 Message-ID: Subject: Re: [nohz] 2a16fc93d2c: kernel lockup on idle injection From: Viresh Kumar To: Frederic Weisbecker Cc: Preeti U Murthy , Thomas Gleixner , Fengguang Wu , Frederic Weisbecker , "Pan, Jacob jun" , LKML , LKP Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.171 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , + Peter from Jacob's mail .. On 16 December 2014 at 05:14, Frederic Weisbecker wrote: > So to summarize: I see it enqueues a timer then it loops on that timer expiration. > On that loop we stop the CPU and we expect the timer to fire and wake the thread up. > But if the delayed tick fires too early, before the timer actually > expires, then we go to sleep again because we haven't yet reached the delay, > but since tick_nohz_irq_exit() is only called on idle tasks and not for threads > like powerclamp, the tick isn't rescheduled to handle the remaining timer slice > so we sleep forever. Perfect !! > Hence if we really want to stop the tick when we mimic idle from powerclamp driver, > we must call tick_nohz_irq_exit() on irq exit to do it correctly. > > It happened to work by accident before the commit because we were rescheduling the > tick from itself without tick_nohz_irq_exit() to cancel anything. And that restored > the periodic behaviour necessary to complete the delay. > > So the above change is rather a hack than a solution. > > We have several choices: > > 1) Revert the commit. But this has to be a temporary solution really. Powerclamp has > to be fixed and handle tick_nohz_irq_exit(). > > 2) Remove powerclamp tick stop until somebody fixes it to handle nohz properly. > > 2) Fix it directly. But I believe there is a release that is going to miss the fix > and suffer the regression. Does the regression matter for anybody? Is powerclamp > meant for anything else than testing (I have no idea what it's used for)? > > So to fix powerclamp to handle nohz correctly, tick_nohz_irq_exit() must be called > for both idle tasks and powerclamp kthreads. Checking ts->inidle can be a good way to match > both. That means we might need to use a reduced part of idle_cpu() to avoid redundant checks. > tick_irq_enter() must be called as well for powerclamp, in case it's the only CPU running, it > has to fixup the timekeeping alone. Yeah, you can call my fix a Hacky one. I agree.. But I don't know if calling tick_nohz_irq_exit() from these threads wouldn't be hacky as well. And ofcourse my knowledge wouldn't be adequate here to judge that :) It looked a bit odd to me. Why should we call irq-exit from the threads working with idle? And that's not what we do even for the real-idle loop as well .. Also from Jacob's referenced patch, we would be moving to a consolidated idle-loop for both real idle and threads like powerclamp, and this part may be tricky then.. Untested, but what about something like this? I am not sure of the purpose of the idle-checks in the irq-paths. All I understood from git logs is, these are to check if we came out of dynticks mode and need to start/stop ticks again.. Then why not handle these idle mimicking threads as well? We just need to check ts->idle_active which will take care of both the cases, real idle and these threads.. Sorry, if this will break things further :( --- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ diff --git a/kernel/softirq.c b/kernel/softirq.c index 5918d227730f..5e4bfc367735 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -321,7 +321,7 @@ asmlinkage __visible void do_softirq(void) void irq_enter(void) { rcu_irq_enter(); - if (is_idle_task(current) && !in_interrupt()) { + if (tick_idle_active() && !in_interrupt()) { /* * Prevent raise_softirq from needlessly waking up ksoftirqd * here, as softirq will be serviced on return from interrupt. @@ -363,7 +363,7 @@ static inline void tick_irq_exit(void) int cpu = smp_processor_id(); /* Make sure that timer wheel updates are propagated */ - if ((idle_cpu(cpu) && !need_resched()) || tick_nohz_full_cpu(cpu)) { + if ((tick_idle_active() && !need_resched()) || tick_nohz_full_cpu(cpu)) { if (!in_interrupt()) tick_nohz_irq_exit(); } diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 07c2bad0afce..e52b76037c0a 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -46,6 +46,13 @@ struct tick_sched *tick_get_tick_sched(int cpu) return &per_cpu(tick_cpu_sched, cpu); } +bool tick_idle_active(void) +{ + struct tick_sched *ts = &__get_cpu_var(tick_cpu_sched); + + return ts->idle_active; +} + /* * Must be called with interrupts disabled ! */