From patchwork Wed Apr 9 04:34:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 28058 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-yh0-f69.google.com (mail-yh0-f69.google.com [209.85.213.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 5146120447 for ; Wed, 9 Apr 2014 04:34:40 +0000 (UTC) Received: by mail-yh0-f69.google.com with SMTP id b6sf5887711yha.4 for ; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=6YFoGK3xaKQLDCQPQ7ioN0eLA4eZMvWpaEL5htQjvZ4=; b=SNhstEI3ijShJITQmfJwQMUlHblWOJoY58v5YE8zlIyUnN4DQVzwcLl3w2pMMP2pCJ 76MjaMAm0e/Lrye/l6MWoIMrf2kYx5R+mgmaeSHR2SYcJOHQKrItB+OYKQPAJAJS5I1H yr9Q24oFDP1YWh8Spem6vLSMIF64nXq4UwWL0AY0udgplL6Kdnsd7MXFUunV9vDwVp6N GU7Cn5nEO6JS24+FdL3ts3J3QOR3QHkjVAo9/OW0CDDf5howHYPkrSM/VR5NSmbK7qjD 3+y5629cNZ4mBXHGkVV1vMW5Ks85yGNcyZ+F0Zz4uUdNv0uwgkWZXIF4wPaBv1D2EDOX 48DQ== X-Gm-Message-State: ALoCoQnZPjpV8oshEkT4Kqyn0GuuYoSuZOnmRqvaN2NXvIaTDGhITy4rf+2w2eFsShXkB0yjairs X-Received: by 10.236.109.169 with SMTP id s29mr3265143yhg.43.1397018080530; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.30.67 with SMTP id c61ls483759qgc.33.gmail; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) X-Received: by 10.52.240.207 with SMTP id wc15mr5756799vdc.14.1397018080377; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) Received: from mail-ve0-f169.google.com (mail-ve0-f169.google.com [209.85.128.169]) by mx.google.com with ESMTPS id v8si784746vcs.176.2014.04.08.21.34.40 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 08 Apr 2014 21:34:40 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.169 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.169; Received: by mail-ve0-f169.google.com with SMTP id pa12so1635159veb.14 for ; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) X-Received: by 10.58.31.136 with SMTP id a8mr7003027vei.20.1397018080272; Tue, 08 Apr 2014 21:34:40 -0700 (PDT) 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.220.12.8 with SMTP id v8csp299078vcv; Tue, 8 Apr 2014 21:34:39 -0700 (PDT) X-Received: by 10.66.233.169 with SMTP id tx9mr9130319pac.7.1397018078945; Tue, 08 Apr 2014 21:34:38 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id tk5si2054603pbc.338.2014.04.08.21.34.38; Tue, 08 Apr 2014 21:34:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757820AbaDIEeZ (ORCPT + 27 others); Wed, 9 Apr 2014 00:34:25 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:63419 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750779AbaDIEeW (ORCPT ); Wed, 9 Apr 2014 00:34:22 -0400 Received: by mail-wg0-f51.google.com with SMTP id k14so1914434wgh.10 for ; Tue, 08 Apr 2014 21:34:21 -0700 (PDT) X-Received: by 10.194.21.193 with SMTP id x1mr7128270wje.33.1397018061522; Tue, 08 Apr 2014 21:34:21 -0700 (PDT) Received: from localhost ([213.122.173.131]) by mx.google.com with ESMTPSA id t6sm7362629wix.4.2014.04.08.21.34.17 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 08 Apr 2014 21:34:20 -0700 (PDT) From: Viresh Kumar To: tglx@linutronix.de, john.stultz@linaro.org Cc: linaro-kernel@lists.linaro.org, linaro-networking@linaro.org, Arvind.Chauhan@arm.com, linux-kernel@vger.kernel.org, fengguang.wu@intel.com, jet.chen@intel.com, Viresh Kumar Subject: [PATCH V2] clocksource: register cpu notifier to remove timer from dying CPU Date: Wed, 9 Apr 2014 10:04:15 +0530 Message-Id: X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e 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=neutral (google.com: 209.85.128.169 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) 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: , clocksource core is using add_timer_on() to run clocksource_watchdog() on all CPUs one by one. But when a core is brought down, clocksource core doesn't remove this timer from the dying CPU. And in this case timer core gives this (Gives this only with unmerged code, anyway in the current code as well timer core is migrating a pinned timer to other CPUs, which is also wrong: http://www.gossamer-threads.com/lists/linux/kernel/1898117) migrate_timer_list: can't migrate pinned timer: ffffffff81f06a60, timer->function: ffffffff810d7010,deactivating it Modules linked in: CPU: 0 PID: 1932 Comm: 01-cpu-hotplug Not tainted 3.14.0-rc1-00088-gab3c4fd #4 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 0000000000000009 ffff88001d407c38 ffffffff817237bd ffff88001d407c80 ffff88001d407c70 ffffffff8106a1dd 0000000000000010 ffffffff81f06a60 ffff88001e04d040 ffffffff81e3d4c0 ffff88001e04d030 ffff88001d407cd0 Call Trace: [] dump_stack+0x4d/0x66 [] warn_slowpath_common+0x7d/0xa0 [] warn_slowpath_fmt+0x4c/0x50 [] ? __internal_add_timer+0x113/0x130 [] ? clocksource_watchdog_kthread+0x40/0x40 [] migrate_timer_list+0xdb/0xf0 [] timer_cpu_notify+0xfc/0x1f0 [] notifier_call_chain+0x4c/0x70 [] __raw_notifier_call_chain+0xe/0x10 [] cpu_notify+0x23/0x50 [] cpu_notify_nofail+0xe/0x20 [] _cpu_down+0x1ad/0x2e0 [] cpu_down+0x34/0x50 [] cpu_subsys_offline+0x14/0x20 [] device_offline+0x95/0xc0 [] online_store+0x40/0x90 [] dev_attr_store+0x18/0x30 [] sysfs_kf_write+0x3d/0x50 This patch tries to fix this by registering cpu notifiers from clocksource core, only when we start clocksource-watchdog. And if on the CPU_DEAD notification it is found that dying CPU was the CPU on which this timer is queued on, then it is removed from that CPU and queued to next CPU. Reported-and-tested-by: Jet Chen Reported-by: Fengguang Wu Signed-off-by: Viresh Kumar --- V1->V2: - Moved 'static int timer_cpu' within #ifdef CONFIG_CLOCKSOURCE_WATCHDOG/endif - replaced spin_lock with spin_lock_irqsave in clocksource_cpu_notify() as a bug is reported by Jet Chen with that. - Tested again by Jet Chen (Thanks again :)) kernel/time/clocksource.c | 65 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index ba3e502..d288f1f 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -23,10 +23,12 @@ * o Allow clocksource drivers to be unregistered */ +#include #include #include #include #include +#include #include /* for spin_unlock_irq() using preempt_count() m68k */ #include #include @@ -180,6 +182,9 @@ static char override_name[CS_NAME_LEN]; static int finished_booting; #ifdef CONFIG_CLOCKSOURCE_WATCHDOG +/* Tracks current CPU to queue watchdog timer on */ +static int timer_cpu; + static void clocksource_watchdog_work(struct work_struct *work); static void clocksource_select(void); @@ -246,12 +251,25 @@ void clocksource_mark_unstable(struct clocksource *cs) spin_unlock_irqrestore(&watchdog_lock, flags); } +static void queue_timer_on_next_cpu(void) +{ + /* + * Cycle through CPUs to check if the CPUs stay synchronized to each + * other. + */ + timer_cpu = cpumask_next(timer_cpu, cpu_online_mask); + if (timer_cpu >= nr_cpu_ids) + timer_cpu = cpumask_first(cpu_online_mask); + watchdog_timer.expires = jiffies + WATCHDOG_INTERVAL; + add_timer_on(&watchdog_timer, timer_cpu); +} + static void clocksource_watchdog(unsigned long data) { struct clocksource *cs; cycle_t csnow, wdnow; int64_t wd_nsec, cs_nsec; - int next_cpu, reset_pending; + int reset_pending; spin_lock(&watchdog_lock); if (!watchdog_running) @@ -336,27 +354,51 @@ static void clocksource_watchdog(unsigned long data) if (reset_pending) atomic_dec(&watchdog_reset_pending); - /* - * Cycle through CPUs to check if the CPUs stay synchronized - * to each other. - */ - next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask); - if (next_cpu >= nr_cpu_ids) - next_cpu = cpumask_first(cpu_online_mask); - watchdog_timer.expires += WATCHDOG_INTERVAL; - add_timer_on(&watchdog_timer, next_cpu); + queue_timer_on_next_cpu(); out: spin_unlock(&watchdog_lock); } +static int clocksource_cpu_notify(struct notifier_block *self, + unsigned long action, void *hcpu) +{ + long cpu = (long)hcpu; + unsigned long flags; + + spin_lock_irqsave(&watchdog_lock, flags); + if (!watchdog_running) + goto notify_out; + + switch (action) { + case CPU_DEAD: + case CPU_DEAD_FROZEN: + if (cpu != timer_cpu) + break; + del_timer(&watchdog_timer); + queue_timer_on_next_cpu(); + break; + } + +notify_out: + spin_unlock_irqrestore(&watchdog_lock, flags); + return NOTIFY_OK; +} + +static struct notifier_block clocksource_nb = { + .notifier_call = clocksource_cpu_notify, + .priority = 1, +}; + static inline void clocksource_start_watchdog(void) { if (watchdog_running || !watchdog || list_empty(&watchdog_list)) return; + timer_cpu = cpumask_first(cpu_online_mask); + register_cpu_notifier(&clocksource_nb); init_timer(&watchdog_timer); watchdog_timer.function = clocksource_watchdog; watchdog_timer.expires = jiffies + WATCHDOG_INTERVAL; - add_timer_on(&watchdog_timer, cpumask_first(cpu_online_mask)); + add_timer_on(&watchdog_timer, timer_cpu); watchdog_running = 1; } @@ -365,6 +407,7 @@ static inline void clocksource_stop_watchdog(void) if (!watchdog_running || (watchdog && !list_empty(&watchdog_list))) return; del_timer(&watchdog_timer); + unregister_cpu_notifier(&clocksource_nb); watchdog_running = 0; }