From patchwork Wed Nov 20 09:49:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 21651 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oa0-f70.google.com (mail-oa0-f70.google.com [209.85.219.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 53CDE23FDC for ; Wed, 20 Nov 2013 09:49:35 +0000 (UTC) Received: by mail-oa0-f70.google.com with SMTP id g12sf23777413oah.5 for ; Wed, 20 Nov 2013 01:49:34 -0800 (PST) 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:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=Pio/Fl5/9uISM8Fy/W9vzDP9QsmPR1mKj1pV9YD69jE=; b=d4FPaTk8ti3QhO5orJKZUKilMvsze+KFKuZgq27l8KtiSWlOwQPmIETQJrMv4GmG8/ uuDa3OsFEoySDLx27DeDAtpeRv1drrp5BnhEbt/7LNWMML0w9Z6TFwTpc0pgfymmmtYX tdpo7VLmFNWiryCfGm+xJlZLxztkbyDg37oa085jrZKW+6GChMGpRd1x95YoDdnbI276 y3XeKCcjh9CohDdPWuPeFZOAO+p0yVNrUOF6mocyY0uQs5dTAJyXlOuwmsWgMerOyz+T rB/c435KBbr/JIfJpIzilDocZtvaW3MtxfH1VxNtCBqKrN7k0dXNcWOjWe8klE9ohSiA DWWQ== X-Gm-Message-State: ALoCoQkpca5BxsHnaiEfBuL/nMvQOJ2gx9/bRtN6s02d/OR8noqBBYqZ4WaB5sngo3gawc2Uv8MS X-Received: by 10.182.245.197 with SMTP id xq5mr11647805obc.27.1384940974892; Wed, 20 Nov 2013 01:49:34 -0800 (PST) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.4.138 with SMTP id k10ls4031qek.13.gmail; Wed, 20 Nov 2013 01:49:34 -0800 (PST) X-Received: by 10.52.245.9 with SMTP id xk9mr9134043vdc.8.1384940974784; Wed, 20 Nov 2013 01:49:34 -0800 (PST) Received: from mail-vc0-f175.google.com (mail-vc0-f175.google.com [209.85.220.175]) by mx.google.com with ESMTPS id dp3si9252274vcb.96.2013.11.20.01.49.34 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 20 Nov 2013 01:49:34 -0800 (PST) Received-SPF: neutral (google.com: 209.85.220.175 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.175; Received: by mail-vc0-f175.google.com with SMTP id ld13so3223049vcb.20 for ; Wed, 20 Nov 2013 01:49:34 -0800 (PST) X-Received: by 10.52.98.99 with SMTP id eh3mr4587973vdb.29.1384940974687; Wed, 20 Nov 2013 01:49:34 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp341570vcz; Wed, 20 Nov 2013 01:49:34 -0800 (PST) X-Received: by 10.224.130.72 with SMTP id r8mr50468781qas.32.1384940974132; Wed, 20 Nov 2013 01:49:34 -0800 (PST) Received: from mail-qc0-f178.google.com (mail-qc0-f178.google.com [209.85.216.178]) by mx.google.com with ESMTPS id j3si15930354qab.91.2013.11.20.01.49.33 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 20 Nov 2013 01:49:34 -0800 (PST) Received-SPF: neutral (google.com: 209.85.216.178 is neither permitted nor denied by best guess record for domain of viresh.kumar@linaro.org) client-ip=209.85.216.178; Received: by mail-qc0-f178.google.com with SMTP id i7so5695664qcq.9 for ; Wed, 20 Nov 2013 01:49:33 -0800 (PST) X-Received: by 10.229.65.201 with SMTP id k9mr50408768qci.11.1384940973827; Wed, 20 Nov 2013 01:49:33 -0800 (PST) Received: from localhost (git.linaro.org. [54.235.93.228]) by mx.google.com with ESMTPSA id b9sm62653084qas.7.2013.11.20.01.49.32 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 20 Nov 2013 01:49:33 -0800 (PST) From: Viresh Kumar To: tglx@linutronix.de Cc: linaro-kernel@lists.linaro.org, patches@linaro.org, bigeasy@linutronix.de, rostedt@goodmis.org, tj@kernel.org, mingo@redhat.com, peterz@infradead.org, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, john.stultz@linaro.org, paulmck@linux.vnet.ibm.com, Viresh Kumar Subject: [RFC] Timer: Migrate running timers Date: Wed, 20 Nov 2013 15:19:18 +0530 Message-Id: <3a0ef8e5a838fcf1a3cdaecc029cc97dea5edf65.1384940804.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e 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.220.175 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 Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi Thomas, This was earlier discussed here (Well, Not much :)): https://lkml.org/lkml/2012/11/6/160 I am floating the idea again to get more attention on this patch. This is just an idea for now, haven't seen much testing.. Migration of timers from idle cores to non-idle ones for power saving is very well working and really saves a lot of power for us. What's currently not working is the migration of running timers Or timers which re-arms themselves. There are complications with migrating timers which schedules themselves again from their handler. del_timer_sync() can't detect that the timer's handler yet has not finished. __mod_timer migrates the timer with following code: ... spin_lock(&old_base->lock); ... timer_set_base(timer, NULL); spin_unlock(&old_base->lock); ->A spin_lock(&new_base->lock); ->B timer_set_base(timer, new_base); ... After the unlock at time A, old_base->running_timer may get updated to the next timer in queue. After lock at time B, lock_timer_base() will return new_base where another timer might be running timer at that point of time. Whereas, del_timer_sync() depends on below code to check if a timer's handler is currently running or not. if (base->running_timer != timer) Because there is window where timer's handler would be running and at the same time it wasn't marked as running timer for any of the bases (well, it matters only for its current base, i.e. new_base). del_timer_sync() wouldn't know this and will go on and delete the timer, whose handler is currently running. The new approach tries to mark such timers with wait_for_migration_to_complete flag (can be named better and some memory can be saved as PeterZ suggested), which will be used in del_timer_sync() to see if the timer is currently migrating and so isn't marked as running_timer of its base. Benefits: Well, obviously saving power for a core which is being interrupted again and again in its idle loop by this timer event. Which will also prevent the core to go in deeper idle states if possible. Please share your feedback on this approach. Signed-off-by: Viresh Kumar --- include/linux/timer.h | 1 + kernel/timer.c | 29 +++++++++++++---------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/include/linux/timer.h b/include/linux/timer.h index 8c5a197..ad00ebe 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -20,6 +20,7 @@ struct timer_list { void (*function)(unsigned long); unsigned long data; + int wait_for_migration_to_complete; int slack; diff --git a/kernel/timer.c b/kernel/timer.c index 6582b82..30a93e6 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -748,21 +748,15 @@ __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); - } + if (base->running_timer == timer) + timer->wait_for_migration_to_complete = 1; + + /* 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; @@ -992,7 +986,8 @@ int try_to_del_timer_sync(struct timer_list *timer) base = lock_timer_base(timer, &flags); - if (base->running_timer != timer) { + if ((base->running_timer != timer) && + !timer->wait_for_migration_to_complete) { timer_stats_timer_clear_start_info(timer); ret = detach_if_pending(timer, base, true); } @@ -1185,6 +1180,8 @@ static inline void __run_timers(struct tvec_base *base) call_timer_fn(timer, fn, data); spin_lock_irq(&base->lock); } + if (timer->wait_for_migration_to_complete) + timer->wait_for_migration_to_complete = 0; } } base->running_timer = NULL;