Messages in this thread | | | Date | Sat, 14 Jun 2014 22:40:58 -0700 | From | "Paul E. McKenney" <> | Subject | Re: safety of *mutex_unlock() (Was: [BUG] signal: sighand unprotected when accessed by /proc) |
| |
On Fri, Jun 13, 2014 at 05:08:30PM +0200, Oleg Nesterov wrote: > On 06/12, Paul E. McKenney wrote: > > > > @@ -398,11 +399,9 @@ void rcu_read_unlock_special(struct task_struct *t) > > #ifdef CONFIG_RCU_BOOST > > if (&t->rcu_node_entry == rnp->boost_tasks) > > rnp->boost_tasks = np; > > - /* Snapshot/clear ->rcu_boost_mutex with rcu_node lock held. */ > > - if (t->rcu_boost_mutex) { > > - rbmp = t->rcu_boost_mutex; > > - t->rcu_boost_mutex = NULL; > > - } > > + /* Snapshot/clear ->boost_mutex with rcu_node lock held. */ > > + if (rt_mutex_owner(&rnp->boost_mtx) == t) > > + rbmp = &rnp->boost_mtx; > > The comment above looks confusing after this change ;) We do not clear it, > and it doesn't explain "with rcu_node lock held". > > And, with or without this change it is not obvious why do we need "rbmp", > after this patch this becomes even more unobvious. > > This is subjective of course, but perhaps it would be more understandable > to do > > bool xxx; > > ... > > // Check this under rcu_node lock to ensure that unlock below > // can't race with rt_mutex_init_proxy_locked() in progress. > xxx = rt_mutex_owner(&rnp->boost_mtx) == t; > > ... > > // rnp->lock was dropped > if (xxx) > rt_mutex_unlock(&rnp->boost_mtx); > > > But this is very minor, I won't insist of course. Mostly I am just trying > to check my understanding.
No, this is good, and I will update accordingly.
Thanx, Paul
| |