Re: Notifier chains are unsafe

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

On Thu, 2005-10-27 at 17:21 -0400, Alan Stern wrote:

> > How does the following code look (only change w.r.t the existing usage
> > model is that unregister can now return -EAGAIN, if the list is busy).
> > 
> > One assumption the following code makes is that the store of a pointer
> > (next in the list) is atomic. If that assumption is unacceptable, we can
> > do one of two things:
> >     1. change notify_register to return -EAGAIN if list is busy.
> >     2. move the chain list in call_chain under lock and use that
> >        list instead of using the chain in the head, and restore it back
> >        before returning.
> 
> I see a couple of problems (aside from the trivial one where you increment 
> nh->readers before the early exit).

Just a programmatic error. shouldn't be a problem.

> 
> The biggest problem is allowing unregister to return an error.  None of 
> its callers will expect that, and they all will have to be changed.  There 
> are a lot more calls to unregister than there are chain definitions.

IMO, we will not be changing the interface so it should be fine.

> The other problem is that you violated Keith's statement that 
> notifier_call_chain shouldn't take any locks.  On the other hand, if we

I would interpret Keith's comment like this: callout should not be
called with any locks held (because that would limit the callouts from
blocking). 

Keith, can you please clarify
>  
> put together all the requirements people have listed for notifier chains, 
> the resulting set is inconsistent!  That's part of the reason why I 
> suggested implementing two different kinds of chains.
> 
> Alan Stern
> 
> 
-- 

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