From patchwork Sat Mar 28 11:57:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 46451 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f72.google.com (mail-wg0-f72.google.com [74.125.82.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D3A802159F for ; Sat, 28 Mar 2015 11:57:43 +0000 (UTC) Received: by wgqa9 with SMTP id a9sf15466503wgq.2 for ; Sat, 28 Mar 2015 04:57:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:cc:subject:references:in-reply-to:content-type :content-transfer-encoding:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe; bh=iVSjXz2WDV0+sTYOGpteJ1JBzNAbr9rI3AAuqwkTOCw=; b=LH1tTQpsdpxjIDAqQ9Jt9HFXrhhy/5SEA+CRtLbXHXRtgBQZ2KVG1KJAq1p874Nt/t UJOxl+4ZZQ8BHulSg4zxvOx1JSDjEyIuahz+mtO7Mr9N5Q0hfblzzAMElbR9VZlagVFE ng0ms7rYJqfW49hM+Ujl7TUTw343msuYTlGASmsGsjKMrdc7Lw6GeFIDq6R0ehJT6CR4 aK5Fs+L5gPxlA3DDyUwMEtTrFQY2GQMD0WapIAU6L8Z+r5v0W8EOW9Md2k8xC2D9oYD3 j1DnLvWrKyS3GrK5ukp1366EZIK66w8zn5bFkuz2xKJn3NBuEvccwydnuLt9XVTONUgQ ixYQ== X-Gm-Message-State: ALoCoQlW3Dkqua1qhDggBlqDlWk5EO5USzCu5ft0wLqimCZ6vNv9ZRBVm8sihroctwVCsYbCWsCb X-Received: by 10.152.115.145 with SMTP id jo17mr307592lab.2.1427543863060; Sat, 28 Mar 2015 04:57:43 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.21.233 with SMTP id y9ls490894lae.9.gmail; Sat, 28 Mar 2015 04:57:42 -0700 (PDT) X-Received: by 10.152.198.8 with SMTP id iy8mr21504673lac.13.1427543862718; Sat, 28 Mar 2015 04:57:42 -0700 (PDT) Received: from mail-lb0-f170.google.com (mail-lb0-f170.google.com. [209.85.217.170]) by mx.google.com with ESMTPS id dc8si3236353lad.65.2015.03.28.04.57.42 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 28 Mar 2015 04:57:42 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.170 as permitted sender) client-ip=209.85.217.170; Received: by lbdc10 with SMTP id c10so25672809lbd.2 for ; Sat, 28 Mar 2015 04:57:42 -0700 (PDT) X-Received: by 10.152.5.225 with SMTP id v1mr20831183lav.76.1427543862158; Sat, 28 Mar 2015 04:57:42 -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.112.57.201 with SMTP id k9csp403696lbq; Sat, 28 Mar 2015 04:57:41 -0700 (PDT) X-Received: by 10.66.145.41 with SMTP id sr9mr43464225pab.18.1427543859968; Sat, 28 Mar 2015 04:57:39 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id bn4si6714652pbc.76.2015.03.28.04.57.38; Sat, 28 Mar 2015 04:57:39 -0700 (PDT) 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 S1753316AbbC1L5e (ORCPT + 27 others); Sat, 28 Mar 2015 07:57:34 -0400 Received: from mail-pd0-f180.google.com ([209.85.192.180]:36175 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752321AbbC1L5d (ORCPT ); Sat, 28 Mar 2015 07:57:33 -0400 Received: by pdcp1 with SMTP id p1so31727143pdc.3 for ; Sat, 28 Mar 2015 04:57:32 -0700 (PDT) X-Received: by 10.70.90.16 with SMTP id bs16mr27076570pdb.76.1427543852680; Sat, 28 Mar 2015 04:57:32 -0700 (PDT) Received: from [192.168.0.2] ([122.167.118.120]) by mx.google.com with ESMTPSA id o4sm4908136pdh.6.2015.03.28.04.57.27 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 28 Mar 2015 04:57:31 -0700 (PDT) Message-ID: <55169723.3070006@linaro.org> Date: Sat, 28 Mar 2015 17:27:23 +0530 From: viresh kumar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Peter Zijlstra CC: Andrew Morton , Johannes Weiner , Christoph Lameter , Linaro Kernel Mailman List , Linux Kernel Mailing List , vinmenon@codeaurora.org, shashim@codeaurora.org, Michal Hocko , Mel Gorman , dave@stgolabs.net, Konstantin Khlebnikov , Linux Memory Management List , Suresh Siddha , Thomas Gleixner Subject: Re: [RFC] vmstat: Avoid waking up idle-cpu to service shepherd work References: <359c926bc85cdf79650e39f2344c2083002545bb.1427347966.git.viresh.kumar@linaro.org> <20150326131822.fce6609efdd85b89ceb3f61c@linux-foundation.org> <20150327091613.GE27490@worktop.programming.kicks-ass.net> <20150327093023.GA32047@worktop.ger.corp.intel.com> <20150328095322.GH27490@worktop.programming.kicks-ass.net> In-Reply-To: <20150328095322.GH27490@worktop.programming.kicks-ass.net> 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.170 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: , On 28 March 2015 at 15:23, Peter Zijlstra wrote: > Well, for one your patch is indeed disgusting. Yeah, I agree :) > But yes I'm aware Thomas > wants to rewrite the timer thing. But Thomas is away for a little while > and if this really needs to happen then it does. Sometime back I was trying to use another bit from base pointer for marking a timer as PINNED: ------------x--------------------x---------------------- Right? Now there are few issues I see here (Sorry if they are all imaginary): - In case a timer re-arms itself from its handler and is migrated from CPU A to B, what happens if the re-armed timer fires before the first handler finishes ? i.e. timer->fn() hasn't finished running on CPU A and it has fired again on CPU B. Wouldn't this expose us to a lot of other problems? It wouldn't be serialized to itself anymore ? - Because the timer has migrated to another CPU, the locking in __run_timers() needs to be fixed. And that will make it complicated .. - __run_timer() doesn't lock bases of other CPUs, and it has to do it now.. - We probably need to take locks of both local CPU and the one to which timer migrated. - Its possible now that there can be more than one running timer for a base, which wasn't true earlier. Not sure if it will break something. Thanks for your continuous support to reply to my (sometimes stupid) queries. --- viresh [1] https://lists.01.org/pipermail/kbuild-all/2014-April/003982.html -- 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/include/linux/timer.h b/include/linux/timer.h index 8c5a197e1587..e7184f57449c 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -67,8 +67,9 @@ extern struct tvec_base boot_tvec_bases; */ #define TIMER_DEFERRABLE 0x1LU #define TIMER_IRQSAFE 0x2LU +#define TIMER_PINNED 0x4LU -#define TIMER_FLAG_MASK 0x3LU +#define TIMER_FLAG_MASK 0x7LU And Fenguang's build-bot showed the problem (only) on blackfin [1]. config: make ARCH=blackfin allyesconfig All error/warnings: kernel/timer.c: In function 'init_timers': >> kernel/timer.c:1683:2: error: call to '__compiletime_assert_1683' >> declared with attribute error: BUILD_BUG_ON failed: >> __alignof__(struct tvec_base) & TIMER_FLAG_MASK So probably we need to make 'base' aligned to 8 bytes ? So, what you are suggesting is something like this (untested): diff --git a/include/linux/timer.h b/include/linux/timer.h index 8c5a197e1587..68bf09d69352 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -67,8 +67,9 @@ extern struct tvec_base boot_tvec_bases; */ #define TIMER_DEFERRABLE 0x1LU #define TIMER_IRQSAFE 0x2LU +#define TIMER_RUNNING 0x4LU -#define TIMER_FLAG_MASK 0x3LU +#define TIMER_FLAG_MASK 0x7LU #define __TIMER_INITIALIZER(_function, _expires, _data, _flags) { \ .entry = { .prev = TIMER_ENTRY_STATIC }, \ diff --git a/kernel/time/timer.c b/kernel/time/timer.c index 2d3f5c504939..8f9efa64bd34 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -105,6 +105,21 @@ static inline unsigned int tbase_get_irqsafe(struct tvec_base *base) return ((unsigned int)(unsigned long)base & TIMER_IRQSAFE); } +static inline unsigned int tbase_get_running(struct tvec_base *base) +{ + return ((unsigned int)(unsigned long)base & TIMER_RUNNING); +} + +static inline unsigned int tbase_set_running(struct tvec_base *base) +{ + return ((unsigned int)(unsigned long)base | TIMER_RUNNING); +} + +static inline unsigned int tbase_clear_running(struct tvec_base *base) +{ + return ((unsigned int)(unsigned long)base & ~TIMER_RUNNING); +} + static inline struct tvec_base *tbase_get_base(struct tvec_base *base) { return ((struct tvec_base *)((unsigned long)base & ~TIMER_FLAG_MASK)); @@ -781,21 +796,12 @@ __mod_timer(struct timer_list *timer, unsigned long expires, new_base = per_cpu(tvec_bases, cpu); if (base != new_base) { - /* - * We are trying to schedule the timer on the local CPU. - * However we can't change timer's base while it is running, - * otherwise del_timer_sync() can't detect that the timer's - * handler yet has not finished. This also guarantees that - * the timer is serialized wrt itself. - */ - if (likely(base->running_timer != timer)) { - /* See the comment in lock_timer_base() */ - timer_set_base(timer, NULL); - spin_unlock(&base->lock); - base = new_base; - spin_lock(&base->lock); - timer_set_base(timer, base); - } + /* See the comment in lock_timer_base() */ + timer_set_base(timer, NULL); + spin_unlock(&base->lock); + base = new_base; + spin_lock(&base->lock); + timer_set_base(timer, base); } timer->expires = expires; @@ -1016,7 +1022,7 @@ int try_to_del_timer_sync(struct timer_list *timer) base = lock_timer_base(timer, &flags); - if (base->running_timer != timer) { + if (tbase_get_running(timer->base)) { timer_stats_timer_clear_start_info(timer); ret = detach_if_pending(timer, base, true); } @@ -1202,6 +1208,7 @@ static inline void __run_timers(struct tvec_base *base) timer_stats_account_timer(timer); base->running_timer = timer; + tbase_set_running(timer->base); detach_expired_timer(timer, base); if (irqsafe) { @@ -1216,6 +1223,7 @@ static inline void __run_timers(struct tvec_base *base) } } base->running_timer = NULL; + tbase_clear_running(timer->base); spin_unlock_irq(&base->lock); }