All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lingutla Chandrasekhar <clingutla@codeaurora.org>
To: tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, neeraju@codeaurora.org,
	linux-arm-msm@vger.kernel.org,
	Lingutla Chandrasekhar <clingutla@codeaurora.org>
Subject: [PATCH v1] timer: Forward timer base before migrating timers
Date: Wed, 17 Jan 2018 18:14:42 +0530	[thread overview]
Message-ID: <20180117124442.24904-1-clingutla@codeaurora.org> (raw)

In case when timers are migrated to a CPU, after it exits
idle, but before timer base is forwarded, either from
run_timer_softirq()/mod_timer()/add_timer_on(), it's
possible that migrated timers are queued, based on older
clock value. This can cause delays in handling those timers.

For example, consider below sequence of events:

- CPU0 timer1 expires = 59969 and base->clk = 59131. So,
  timer is queued at level 2, with next expiry for this timer
  = 60032 (due to granularity addition).
- CPU1 enters idle @60007, with next timer expiry @60020.
- CPU1 exits idle.
- CPU0 is hotplugged at 60009, and timers are migrated to
  CPU1, with new base->clk = 60007. timer1 is queued,
  based on 60007 at level 0, for immediate handling (in
  next timer softirq handling).
- CPU1's base->clk is forwarded to 60009, so, in next sched
  timer interrupt, timer1 is not handled.

The issue happens as timer wheel collects expired timers
starting from the current clk's index onwards, but migrated
timers, if enqueued, based on older clk value can result
in their index being less than clk's current index.
This can only happen if new base->clk is ahead of
timer->expires, resulting in timer being queued at
new base->clk's current index.

Signed-off-by: Lingutla Chandrasekhar <clingutla@codeaurora.org>
Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 89a9e1b4264a..f66c7ad55d7a 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1886,6 +1886,12 @@ int timers_dead_cpu(unsigned int cpu)
 		raw_spin_lock_irq(&new_base->lock);
 		raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
 
+		/*
+		 * Before migrating timers, update new base clk to avoid
+		 * queueing timers based on older clock value.
+		 */
+		forward_timer_base(new_base);
+
 		BUG_ON(old_base->running_timer);
 
 		for (i = 0; i < WHEEL_SIZE; i++)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
 a Linux Foundation Collaborative Project.

             reply	other threads:[~2018-01-17 12:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-17 12:44 Lingutla Chandrasekhar [this message]
2018-01-18 10:45 ` [PATCH v1] timer: Forward timer base before migrating timers Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180117124442.24904-1-clingutla@codeaurora.org \
    --to=clingutla@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neeraju@codeaurora.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.