Messages in this thread | | | Date | Mon, 12 Apr 2010 20:32:51 +0200 | From | Oleg Nesterov <> | Subject | Re: mmotm 2010-04-05 - another RCU whinge (not network this time) |
| |
On 04/09, Paul E. McKenney wrote: > > > Oleg, looks like proc-make-collect_sigign_sigcatch-rcu-safe.patch is the > > offender here, it added the line that causes the whinge. > > If collect_sigign_sigcatch() is OK to call by updaters as well as > readers, we need something like: > > struct sighand_struct *sighand; > > sighand = rcu_dereference_check(p->sighand, > rcu_read_lock_held() || > lockdep_is_held(&???)); > > Where the "???" is replaced with whichever of the two locks is protecting > updates. My guess would be the sighand lock, but I would not rely on > my guesses in this case. ;-)
Yes, it should be p->sighand->siglock.
Actually, I was going to change another caller, do_task_stat(), to call collect_sigign_sigcatch() without ->siglock too, but now I am not sure when/if this will happen.
OK, thanks, I'll send the patch to make rcu_dereference_check() happy.
While we are here... __exit_signal() does
sighand = rcu_dereference_check(tsk->sighand, rcu_read_lock_held() || lockdep_tasklist_lock_is_held());
What is the point? We know that the single caller must hold tasklist, otherwise everything is broken. Perhaps it would be better to use rcu_dereference_raw() ?
In fact, I don't really understand why __exit_signal() needs rcu_dereference() at all.
Oleg.
| |