mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Milton Miller <miltonm@bga.com>
To: Thomas Gleixner <tglx@linutronix.de>, John Stultz <johnstul@us.ibm.com>
Cc: <linux-kernel@vger.kernel.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: [PATCH] nohz: fix race allowing use of stale jiffies when waking
Date: Thu, 12 Jan 2012 02:55:28 -0600	[thread overview]
Message-ID: <nohz-jiffies-update64-race@mdm.bga.com> (raw)

When waking up from nohz mode, all cpus call tick_do_update_jiffies64
regardless of tick_do_timer_cpu as it could be no cpu was assigned.

At the start of the function there is a quick lockless check to
determine if jiffies is current.  The check uses last_jiffies_update,
which is used to calculate when to perform the next increment.
Unfortunately it is updated when how many jiffies to advance the
clock is calculated, before the call to do_timer which actually
updates jiffies.  A second cpu waking up could use the (potentially
very) stale jiffies value during this window.

This patch changes the check to be against tick_next_period, which
is updated after the call to do_timer completes.  It compares the
result of subtraction to zero, but this is safe as ktime_sub returns
ktime_t which is s64, as signed type.

I found this race while trying to track down reports of network adapter
hangs on a large system.  I suspected premature false detection so
I added logging when the locked region determined a multiple jiffie
update would be required.  I noticed that it happened frequently when
tick_do_timer_cpu was NONE (-1), and realized the large update was
when all cpus were previously in nohz.  I then thought about what
would happen if multiple cpus woke up near close to each other in
time and decided the stale jiffies would be used.  (I later found at
least part of the hung adapter reports were due to faulty detection
logic that has since changed upstream.)

Signed-off-by: Milton Miller <miltonm@bga.com>
Cc: stable@vger.kernel.org
--- 
Patch was generated and tested against 2.6.36; I verified it applies
with offset -1 line to next-20120111.

Index: src/kernel/time/tick-sched.c
===================================================================
--- src.orig/kernel/time/tick-sched.c	2011-10-13 17:42:16.000000000 -0500
+++ src/kernel/time/tick-sched.c	2011-10-13 17:45:31.000000000 -0500
@@ -52,8 +52,8 @@ static void tick_do_update_jiffies64(kti
 	/*
 	 * Do a quick check without holding xtime_lock:
 	 */
-	delta = ktime_sub(now, last_jiffies_update);
-	if (delta.tv64 < tick_period.tv64)
+	delta = ktime_sub(now, tick_next_period);
+	if (delta.tv64 < 0)
 		return;
 
 	/* Reevalute with xtime_lock held */

             reply	other threads:[~2012-01-12  8:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-12  8:55 Milton Miller [this message]
2012-01-12  9:49 ` Eric Dumazet
2012-01-14  5:02   ` Milton Miller
2012-03-22  1:14     ` John Stultz

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=nohz-jiffies-update64-race@mdm.bga.com \
    --to=miltonm@bga.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®