[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1441368690-32345-3-git-send-email-pmladek@suse.com>
Date: Fri, 4 Sep 2015 14:11:30 +0200
From: Petr Mladek <pmladek@...e.com>
To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Josh Triplett <josh@...htriplett.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Lai Jiangshan <jiangshanlai@...il.com>,
Jiri Kosina <jkosina@...e.com>, linux-kernel@...r.kernel.org,
Petr Mladek <pmladek@...e.com>
Subject: [PATCH 2/2] rcu: Fix up timeouts for forcing the quiescent state
The deadline to force the quiescent state (jiffies_force_qs) is currently
updated only when the previous timeout passed. But the timeout used for
wait_event() is always the entire original timeout. This is strange.
First, we might miss the deadline if we wait after a spurious wake up
or after sleeping in cond_resched() because we wait too long.
Second, we might do another forcing too early if the previous forcing
was done earlier because of RCU_GP_FLAG_FQS and we later get a spurious
wake up. IMHO, we should reset the deadline in this case.
This patch updates the deadline "jiffies_force_qs" right after forcing
the quiescent state by rcu_gp_fqs().
Also it updates the remaining timeout according to the current jiffies and
the requested deadline.
It moves the cond_resched_rcu_qs() to a single place. It changes the order
of the check for the pending signal. But there never should be a pending
signal. If there was we would have bigger problems because wait_event()
would never sleep again until someone flushed the signal.
I have found these problems when trying to understand the code. I do not
have any reproducer. I think that it is hardly visible because
the spurious wakeup is rather theoretical.
Signed-off-by: Petr Mladek <pmladek@...e.com>
---
kernel/rcu/tree.c | 77 ++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 24 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 54af8d5f9f7b..aaeeabcba545 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2035,13 +2035,45 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
}
/*
+ * Normalize, update, and return the first timeout.
+ */
+static unsigned long normalize_jiffies_till_first_fqs(void)
+{
+ unsigned long j = jiffies_till_first_fqs;
+
+ if (unlikely(j > HZ)) {
+ j = HZ;
+ jiffies_till_first_fqs = HZ;
+ }
+
+ return j;
+}
+
+/*
+ * Normalize, update, and return the first timeout.
+ */
+static unsigned long normalize_jiffies_till_next_fqs(void)
+{
+ unsigned long j = jiffies_till_next_fqs;
+
+ if (unlikely(j > HZ)) {
+ j = HZ;
+ jiffies_till_next_fqs = HZ;
+ } else if (unlikely(j < 1)) {
+ j = 1;
+ jiffies_till_next_fqs = 1;
+ }
+
+ return j;
+}
+
+/*
* Body of kthread that handles grace periods.
*/
static int __noreturn rcu_gp_kthread(void *arg)
{
int gf;
- unsigned long j;
- int ret;
+ unsigned long timeout, j;
struct rcu_state *rsp = arg;
struct rcu_node *rnp = rcu_get_root(rsp);
@@ -2071,22 +2103,18 @@ static int __noreturn rcu_gp_kthread(void *arg)
/* Handle quiescent-state forcing. */
rsp->fqs_state = RCU_SAVE_DYNTICK;
- j = jiffies_till_first_fqs;
- if (j > HZ) {
- j = HZ;
- jiffies_till_first_fqs = HZ;
- }
- ret = 0;
+ timeout = normalize_jiffies_till_first_fqs();
+ rsp->jiffies_force_qs = jiffies + timeout;
for (;;) {
- if (!ret)
- rsp->jiffies_force_qs = jiffies + j;
trace_rcu_grace_period(rsp->name,
READ_ONCE(rsp->gpnum),
TPS("fqswait"));
rsp->gp_state = RCU_GP_WAIT_FQS;
- ret = wait_event_interruptible_timeout(rsp->gp_wq,
- rcu_gp_fqs_check_wake(rsp, &gf), j);
+ wait_event_interruptible_timeout(rsp->gp_wq,
+ rcu_gp_fqs_check_wake(rsp, &gf),
+ timeout);
rsp->gp_state = RCU_GP_DOING_FQS;
+try_again:
/* Locking provides needed memory barriers. */
/* If grace period done, leave loop. */
if (!READ_ONCE(rnp->qsmask) &&
@@ -2099,28 +2127,29 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("fqsstart"));
rcu_gp_fqs(rsp);
+ timeout = normalize_jiffies_till_next_fqs();
+ rsp->jiffies_force_qs = jiffies + timeout;
trace_rcu_grace_period(rsp->name,
READ_ONCE(rsp->gpnum),
TPS("fqsend"));
- cond_resched_rcu_qs();
- WRITE_ONCE(rsp->gp_activity, jiffies);
} else {
/* Deal with stray signal. */
- cond_resched_rcu_qs();
- WRITE_ONCE(rsp->gp_activity, jiffies);
WARN_ON(signal_pending(current));
trace_rcu_grace_period(rsp->name,
READ_ONCE(rsp->gpnum),
TPS("fqswaitsig"));
}
- j = jiffies_till_next_fqs;
- if (j > HZ) {
- j = HZ;
- jiffies_till_next_fqs = HZ;
- } else if (j < 1) {
- j = 1;
- jiffies_till_next_fqs = 1;
- }
+ cond_resched_rcu_qs();
+ WRITE_ONCE(rsp->gp_activity, jiffies);
+ /*
+ * Count the remaining timeout when it was a spurious
+ * wakeup. Well, it is useful also when we have slept
+ * in the cond_resched().
+ */
+ j = jiffies;
+ if (ULONG_CMP_GE(j, rsp->jiffies_force_qs))
+ goto try_again;
+ timeout = rsp->jiffies_force_qs - j;
}
/* Handle grace-period end. */
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists