Re: [Lse-tech] Subject: [RFC][PATCH] Fix for unsafe notifier chain mechanism

[Chandra Seetharaman <sekharan@us.ibm.com>]

On Fri, 2005-11-11 at 17:44 -0800, Paul E. McKenney wrote:

Thanks for the comments Paul.


> > +	rcu_assign_pointer(n->next, *nl);
> 
> The above can simply be "n->next = *nl;".  The reason is that this change
> of state is not visible to RCU readers until after the following statement,
> and it therefore need not be an RCU-reader-safe assignment.  You only need
> to use rcu_assign_pointer() when the results of the assignment are
> immediately visible to RCU readers.

will do.

> 
> > +	rcu_assign_pointer(*nl, n);
> > +	up_write(&nh->rwsem);
> > +	if (nh->type == ATOMIC_NOTIFIER)
> > +		synchronize_rcu();
> 
> This "if" statement and the "synchronize_rcu()" are not needed.  Nothing
> has been removed from the list, so nothing will be freed, so no need to
> wait for readers to get done.
> 
> In contrast, the synchronize_rcu() in notifier_chain_unregister() -is-
> needed, since we need to free the removed element.

will do


> > +	if (nh->type == ATOMIC_NOTIFIER)
> > +		rcu_read_lock();
> > +	else
> > +		down_read(&nh->rwsem);
> 
> Is it possible for the value of nh->type to change?  If so, there needs
> to be some additional mechanism to guard against such a change.  However,
> if this field is constant, this code is just fine as is.

No, it is not supposed to change.

-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------