From: Petr Mladek <pmladek@suse.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Josh Triplett <josh@joshtriplett.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Jiri Kosina <jkosina@suse.com>,
linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.com>
Subject: [PATCH 2/2] rcu: Fix up timeouts for forcing the quiescent state
Date: Fri, 4 Sep 2015 14:11:30 +0200 [thread overview]
Message-ID: <1441368690-32345-3-git-send-email-pmladek@suse.com> (raw)
In-Reply-To: <1441368690-32345-1-git-send-email-pmladek@suse.com>
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@suse.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
next prev parent reply other threads:[~2015-09-04 12:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-04 12:11 [PATCH 0/2] rcu: two small fixes for RCU kthreads Petr Mladek
2015-09-04 12:11 ` [PATCH 1/2] rcu: Show the real fqs_state Petr Mladek
2015-09-04 23:24 ` Paul E. McKenney
2015-09-07 14:58 ` Petr Mladek
2015-09-08 19:59 ` Paul E. McKenney
2015-09-09 12:39 ` Petr Mladek
2015-09-09 19:12 ` Paul E. McKenney
2015-09-04 12:11 ` Petr Mladek [this message]
2015-09-04 23:49 ` [PATCH 2/2] rcu: Fix up timeouts for forcing the quiescent state Paul E. McKenney
2015-09-07 15:21 ` Petr Mladek
2015-09-08 20:02 ` Paul E. McKenney
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=1441368690-32345-3-git-send-email-pmladek@suse.com \
--to=pmladek@suse.com \
--cc=jiangshanlai@gmail.com \
--cc=jkosina@suse.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=rostedt@goodmis.org \
/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
Powered by JetHome