All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Wood <swood@redhat.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Paul E . McKenney" <paulmck@linux.ibm.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Clark Williams <williams@redhat.com>,
	Scott Wood <swood@redhat.com>
Subject: [PATCH RT v2 1/3] rcu: Acquire RCU lock when disabling BHs
Date: Wed, 21 Aug 2019 18:19:04 -0500	[thread overview]
Message-ID: <20190821231906.4224-2-swood@redhat.com> (raw)
In-Reply-To: <20190821231906.4224-1-swood@redhat.com>

A plain local_bh_disable() is documented as creating an RCU critical
section, and (at least) rcutorture expects this to be the case.  However,
in_softirq() doesn't block a grace period on PREEMPT_RT, since RCU checks
preempt_count() directly.  Even if RCU were changed to check
in_softirq(), that wouldn't allow blocked BH disablers to be boosted.

Fix this by calling rcu_read_lock() from local_bh_disable(), and update
rcu_read_lock_bh_held() accordingly.

Signed-off-by: Scott Wood <swood@redhat.com>
---
Another question is whether non-raw spinlocks are intended to create an
RCU read-side critical section due to implicit preempt disable.  If they
are, then we'd need to add rcu_read_lock() there as well since RT doesn't
disable preemption (and rcutorture should explicitly test with a
spinlock).  If not, the documentation should make that clear.

 include/linux/rcupdate.h |  4 ++++
 kernel/rcu/update.c      |  4 ++++
 kernel/softirq.c         | 12 +++++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 388ace315f32..d6e357378732 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -615,10 +615,12 @@ static inline void rcu_read_unlock(void)
 static inline void rcu_read_lock_bh(void)
 {
 	local_bh_disable();
+#ifndef CONFIG_PREEMPT_RT_FULL
 	__acquire(RCU_BH);
 	rcu_lock_acquire(&rcu_bh_lock_map);
 	RCU_LOCKDEP_WARN(!rcu_is_watching(),
 			 "rcu_read_lock_bh() used illegally while idle");
+#endif
 }
 
 /*
@@ -628,10 +630,12 @@ static inline void rcu_read_lock_bh(void)
  */
 static inline void rcu_read_unlock_bh(void)
 {
+#ifndef CONFIG_PREEMPT_RT_FULL
 	RCU_LOCKDEP_WARN(!rcu_is_watching(),
 			 "rcu_read_unlock_bh() used illegally while idle");
 	rcu_lock_release(&rcu_bh_lock_map);
 	__release(RCU_BH);
+#endif
 	local_bh_enable();
 }
 
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 016c66a98292..a9cdf3d562bc 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -296,7 +296,11 @@ int rcu_read_lock_bh_held(void)
 		return 0;
 	if (!rcu_lockdep_current_cpu_online())
 		return 0;
+#ifdef CONFIG_PREEMPT_RT_FULL
+	return lock_is_held(&rcu_lock_map) || irqs_disabled();
+#else
 	return in_softirq() || irqs_disabled();
+#endif
 }
 EXPORT_SYMBOL_GPL(rcu_read_lock_bh_held);
 
diff --git a/kernel/softirq.c b/kernel/softirq.c
index d16d080a74f7..6080c9328df1 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -115,8 +115,10 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 	long soft_cnt;
 
 	WARN_ON_ONCE(in_irq());
-	if (!in_atomic())
+	if (!in_atomic()) {
 		local_lock(bh_lock);
+		rcu_read_lock();
+	}
 	soft_cnt = this_cpu_inc_return(softirq_counter);
 	WARN_ON_ONCE(soft_cnt == 0);
 	current->softirq_count += SOFTIRQ_DISABLE_OFFSET;
@@ -151,8 +153,10 @@ void _local_bh_enable(void)
 #endif
 
 	current->softirq_count -= SOFTIRQ_DISABLE_OFFSET;
-	if (!in_atomic())
+	if (!in_atomic()) {
+		rcu_read_unlock();
 		local_unlock(bh_lock);
+	}
 }
 
 void _local_bh_enable_rt(void)
@@ -185,8 +189,10 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 	WARN_ON_ONCE(count < 0);
 	local_irq_enable();
 
-	if (!in_atomic())
+	if (!in_atomic()) {
+		rcu_read_unlock();
 		local_unlock(bh_lock);
+	}
 
 	current->softirq_count -= SOFTIRQ_DISABLE_OFFSET;
 	preempt_check_resched();
-- 
1.8.3.1


  reply	other threads:[~2019-08-21 23:19 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21 23:19 [PATCH RT v2 0/3] RCU fixes Scott Wood
2019-08-21 23:19 ` Scott Wood [this message]
2019-08-21 23:33   ` [PATCH RT v2 1/3] rcu: Acquire RCU lock when disabling BHs Paul E. McKenney
2019-08-22 13:39     ` Joel Fernandes
2019-08-22 15:27       ` Paul E. McKenney
2019-08-23  1:50         ` Joel Fernandes
2019-08-23  2:11           ` Paul E. McKenney
2019-08-23  3:23       ` Scott Wood
2019-08-23 12:30         ` Paul E. McKenney
2019-08-23 16:17         ` Sebastian Andrzej Siewior
2019-08-23 19:46           ` Scott Wood
2019-08-26 15:59             ` Sebastian Andrzej Siewior
2019-08-26 23:21               ` Scott Wood
2019-08-23  2:36     ` Scott Wood
2019-08-23  2:54       ` Paul E. McKenney
2019-08-21 23:19 ` [PATCH RT v2 2/3] sched: migrate_enable: Use sleeping_lock to indicate involuntary sleep Scott Wood
2019-08-21 23:35   ` Paul E. McKenney
2019-08-23  1:21     ` Scott Wood
2019-08-23 16:20   ` Sebastian Andrzej Siewior
2019-08-23 19:28     ` Scott Wood
2019-08-24  3:10       ` Joel Fernandes
2019-08-26 15:25         ` Sebastian Andrzej Siewior
2019-08-26 16:29           ` Paul E. McKenney
2019-08-26 17:49             ` Scott Wood
2019-08-26 18:12               ` Paul E. McKenney
2019-08-27  9:23             ` Sebastian Andrzej Siewior
2019-08-27 13:08               ` Joel Fernandes
2019-08-27 15:58                 ` Paul E. McKenney
2019-08-27 16:06                   ` Joel Fernandes
2019-08-27 15:53               ` Paul E. McKenney
2019-08-28  9:27                 ` Sebastian Andrzej Siewior
2019-08-28 12:54                   ` Paul E. McKenney
2019-08-28 13:14                     ` Sebastian Andrzej Siewior
2019-08-28 13:59                       ` Joel Fernandes
2019-08-28 15:51                         ` Paul E. McKenney
2019-08-28 15:50                       ` Paul E. McKenney
2019-08-21 23:19 ` [PATCH RT v2 3/3] rcu: Disable use_softirq on PREEMPT_RT Scott Wood
2019-08-21 23:40   ` Paul E. McKenney
2019-08-23 16:32     ` Sebastian Andrzej Siewior
2019-08-22 13:59   ` Joel Fernandes
2019-08-22 15:29     ` Paul E. McKenney
2019-08-22 19:31     ` Scott Wood
2019-08-23  0:52       ` Joel Fernandes

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=20190821231906.4224-2-swood@redhat.com \
    --to=swood@redhat.com \
    --cc=bigeasy@linutronix.de \
    --cc=joel@joelfernandes.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=paulmck@linux.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /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.