Paul E. McKenney <[EMAIL PROTECTED]> wrote:
> > spin_lock_irqsave(&tsk->sighand->siglock, flags);
> > - old = tsk->signal->session_keyring;
> > - tsk->signal->session_keyring = keyring;
> > + old = rcu_dereference(tsk->signal->session_keyring);
>
> I don't understand why rcu_dereference() is needed in this case.
> Since we are holding the lock, it should not be possible for
> this to change, right? Or am I missing something? (Quite possible,
> am not all that familiar with this code.)
Erm... you're right. I stuck the rcu_dereference() in then added the locks
back in when I realised I still needed them.
> > + synchronize_kernel();
>
> This would want to become synchronize_rcu().
I think the deprecation happened since I wrote my patch.
> > + if (tsk->signal->session_keyring) {
> > + rcu_read_lock();
> > + key = keyring_search_aux(
> > + rcu_dereference(tsk->signal->session_keyring),
> > + type, description, match);
> > + rcu_read_unlock();
> > + }
> > + else {
> > + key = keyring_search_aux(tsk->user->session_keyring,
> > + type, description, match);
>
> This one is constant, right? If not, I don't understand the locking design.
Which one? tsk->user->session_keyring is, tsk->signal->session_keyring is not.
Thanks for the review.
David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
