Re: Notifier chains are unsafe

[Alan Stern]

On Mon, 31 Oct 2005, Chandra Seetharaman wrote:

> > #define notifier_block_enable(b)      set_wmb((b)->enabled, 1)
> > #define notifier_block_disable(b)     set_wmb((b)->enabled, 0)
> 
> I am not getting the complete picture. So, in unregister we would just
> disable and never delete the notifier_block ? Or
> notifier_block_enable/disable will be used by external entities
> directly ?

Register and unregister will continue to work as before, requiring a
process context and the ability to sleep.  notifier_block_enable/disable
should be used when:

	a callout wants to disable itself as it is running, or

	someone running in an atomic context wants to enable or disable
	a callout.

In the first case, unregister can't be used because it would hang.  In the 
second case, register/unregister can't be used because they need to be 
able to sleep.

In both cases the notifier block would have to be registered beforehand 
and unregistered later.


> > It occurred to me that there _is_ a way to do unregister for atomic chains 
> > without blocking.  Add to struct notifier_head
> > 
> > 	atomic_t num_callers;
> > 
> > Then in notifier_call_chain, do atomic_inc(&nh->num_callers) at the start
> > and atomic_dec(&nh->num_callers) at the end.  Finally, make unregister do
> > this:
> > 
> > int notifier_chain_unregister(struct notifier_head *nh,
> >         struct notifier_block *n)
> > {
> > 	if (nh->type == ATOMIC_NOTIFIER) {
> > 	        spin_lock(nh->lock);
> > 	        list_del(&n->node);
> > 		smp_mb();
> > 		while (atomic_read(&nh->num_callers) > 0)
> > 			cpu_relax();
> > 	        spin_unlock(nh->lock);
> > 	} else {
> > 	...
> > 	}
> >         return 0;
> > }
> 
> But, how is the list protected in call_chain (will you be holding the
> lock in call_chain() while incrementing the atomic variable).

No; the list _won't_ be protected in call_chain.  It will be possible to
unregister a callout while the chain is in use.  That's how the RCU
approach works -- it uses no read locks, only write locks.

Deleting an entry while the list is in use is safe, because readers will
encounter either the old or the new value of the .next pointer, and either
one will be valid.  The important thing is to make sure that no one will
ever encounter the old pointer after unregister returns; that's what the
"while" loop is for.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/