Messages in this thread Patches in this message | | | Date | Mon, 19 Sep 2011 13:49:33 +0800 | Subject | Re: [PATCH tip/core/rcu 41/55] rcu: Permit rt_mutex_unlock() with irqs disabled | From | Yong Zhang <> |
| |
On Mon, Sep 19, 2011 at 12:14 PM, Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote: > On Sun, Sep 18, 2011 at 12:09:23PM +0800, Yong Zhang wrote: >> > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h >> > index d3127e8..f6c63ea 100644 >> > --- a/kernel/rcutree_plugin.h >> > +++ b/kernel/rcutree_plugin.h >> > @@ -1149,6 +1149,8 @@ static void rcu_initiate_boost_trace(struct rcu_node *rnp) >> > >> > #endif /* #else #ifdef CONFIG_RCU_TRACE */ >> > >> > +static struct lock_class_key rcu_boost_class; >> > + >> > /* >> > * Carry out RCU priority boosting on the task indicated by ->exp_tasks >> > * or ->boost_tasks, advancing the pointer to the next task in the >> > @@ -1211,10 +1213,14 @@ static int rcu_boost(struct rcu_node *rnp) >> > */ >> > t = container_of(tb, struct task_struct, rcu_node_entry); >> > rt_mutex_init_proxy_locked(&mtx, t); >> > + /* Avoid lockdep false positives. This rt_mutex is its own thing. */ >> > + lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, >> > + "rcu_boost_mutex"); >> > t->rcu_boost_mutex = &mtx; >> >> raw_spin_unlock_irqrestore(&rnp->lock, flags); <====A >> >> > rt_mutex_lock(&mtx); /* Side effect: boosts task t's priority. */ >> > rt_mutex_unlock(&mtx); /* Keep lockdep happy. */ >> > + local_irq_restore(flags); >> >> Does it help here? >> irq is enabled at A. So we still call rt_mutex_lock() with irq enabled. >> >> Seems should s/raw_spin_unlock_irqrestore/raw_spin_unlock ? > > Hmmm... The above works at least by accident, but I am clearly not > testing calling rt_mutex_lock(&mtx) and rt_mutex_unlock(&mtx) with > interrupts disabled anywhere near as heavily as I thought I was. > > I will fix this one way or the other.
Forget to mention: if we want to suppress the lockdep warning on overlapping usage of rcu_read_*()/local_irq_*() like below:
rcu_read_lock(); ... local_irq_disable(); ... rcu_read_unlock(); ... local_irq_enable();
'rt_mutex_unlock(rbmp);' must also be surrounded by local_irq_irqsave()/restore().
Untested patch is attached.
Thanks, Yong
--- From: Yong Zhang <yong.zhang0@gmail.com> Subject: [PATCH] rcu: Permit rt_mutex_unlock() with irqs disabled take#2
This make the below rcu usage really valid(AKA: lockdep will not warn on it):
rcu_read_lock(); local_irq_disable(); rcu_read_unlock(); local_irq_enable();
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com> --- kernel/rcutree_plugin.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h index e7eea74..d41a9b0 100644 --- a/kernel/rcutree_plugin.h +++ b/kernel/rcutree_plugin.h @@ -398,8 +398,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
#ifdef CONFIG_RCU_BOOST /* Unboost if we were boosted. */ - if (rbmp) + if (rbmp) { + local_irq_save(flags); rt_mutex_unlock(rbmp); + local_irq_restore(flags); + } #endif /* #ifdef CONFIG_RCU_BOOST */
/* @@ -1225,7 +1228,7 @@ static int rcu_boost(struct rcu_node *rnp) lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, "rcu_boost_mutex"); t->rcu_boost_mutex = &mtx; - raw_spin_unlock_irqrestore(&rnp->lock, flags); + raw_spin_unlock(&rnp->lock); rt_mutex_lock(&mtx); /* Side effect: boosts task t's priority. */ rt_mutex_unlock(&mtx); /* Keep lockdep happy. */ local_irq_restore(flags); -- 1.7.4.1From 7d74d1b89a4cd4c03b30e47044b716913f68bd1d Mon Sep 17 00:00:00 2001 From: Yong Zhang <yong.zhang0@gmail.com> Date: Mon, 19 Sep 2011 13:42:32 +0800 Subject: [PATCH] rcu: Permit rt_mutex_unlock() with irqs disabled take#2
This make the below rcu usage really valid(AKA: lockdep will not warn on it):
rcu_read_lock(); local_irq_disable(); rcu_read_unlock(); local_irq_enable();
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com> --- kernel/rcutree_plugin.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h index e7eea74..d41a9b0 100644 --- a/kernel/rcutree_plugin.h +++ b/kernel/rcutree_plugin.h @@ -398,8 +398,11 @@ static noinline void rcu_read_unlock_special(struct task_struct *t) #ifdef CONFIG_RCU_BOOST /* Unboost if we were boosted. */ - if (rbmp) + if (rbmp) { + local_irq_save(flags); rt_mutex_unlock(rbmp); + local_irq_restore(flags); + } #endif /* #ifdef CONFIG_RCU_BOOST */ /* @@ -1225,7 +1228,7 @@ static int rcu_boost(struct rcu_node *rnp) lockdep_set_class_and_name(&mtx.wait_lock, &rcu_boost_class, "rcu_boost_mutex"); t->rcu_boost_mutex = &mtx; - raw_spin_unlock_irqrestore(&rnp->lock, flags); + raw_spin_unlock(&rnp->lock); rt_mutex_lock(&mtx); /* Side effect: boosts task t's priority. */ rt_mutex_unlock(&mtx); /* Keep lockdep happy. */ local_irq_restore(flags); -- 1.7.4.1
| |