From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758673AbbIDMLt (ORCPT ); Fri, 4 Sep 2015 08:11:49 -0400 Received: from mx2.suse.de ([195.135.220.15]:33434 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758489AbbIDMLr (ORCPT ); Fri, 4 Sep 2015 08:11:47 -0400 From: Petr Mladek To: "Paul E. McKenney" , Josh Triplett Cc: Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Jiri Kosina , linux-kernel@vger.kernel.org, Petr Mladek Subject: [PATCH 2/2] rcu: Fix up timeouts for forcing the quiescent state Date: Fri, 4 Sep 2015 14:11:30 +0200 Message-Id: <1441368690-32345-3-git-send-email-pmladek@suse.com> X-Mailer: git-send-email 1.8.5.6 In-Reply-To: <1441368690-32345-1-git-send-email-pmladek@suse.com> References: <1441368690-32345-1-git-send-email-pmladek@suse.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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