[sheepdog] [PATCH v2 1/3] sheep: make {register, unregister}_event thread safe

[Liu Yuan]

On Tue, Dec 17, 2013 at 04:26:09PM +0900, Hitoshi Mitake wrote:
> At Tue, 17 Dec 2013 15:11:30 +0800,
> Liu Yuan wrote:
> > 
> > On Tue, Dec 17, 2013 at 03:58:07PM +0900, Hitoshi Mitake wrote:
> > > At Tue, 17 Dec 2013 14:50:19 +0800,
> > > Liu Yuan wrote:
> > > > 
> > > > On Tue, Dec 17, 2013 at 03:42:29PM +0900, Hitoshi Mitake wrote:
> > > > > At Tue, 17 Dec 2013 14:31:56 +0800,
> > > > > Liu Yuan wrote:
> > > > > > 
> > > > > > This allow us to call even handling functions in worker thread
> > > > > > 
> > > > > > Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> > > > > > ---
> > > > > 
> > > > > I think this change is dangerous. This permits worker threads to
> > > > > unregister events even if these events are processed in the main
> > > > > thread. Making a new work queue and delegate it to register/unregister
> > > > > events would be safer.
> > > > 
> > > > This scheme is not pratical for async request.
> > > > 
> > > > This is just internal API, which are supported to be called by programmers
> > > > and check it is correct.
> > > 
> > > The checking will cost us lots of time. The problem is a race
> > > condition caused by multiple threads.
> > 
> > Why we would face this kind of problem? we can make sure at any time, there will
> > be a single thread manipulate it, be it worker or main thread.
> > 
> > I think we are talking about different issues, you are supposed that events
> > handling will be generically thread-safe for multiple threads. But this wouldn't
> > happen I think. Instead, assumption 'one event will be only be manipulated by
> > single entity (thus no multiple threads case)' will hold true in the long run.
> 
> As you say, your patch doesn't violate the above condition (one event will be
> only be manipulated by single entity). But we cannot express that
> register_event() and unregister_event() are thread safe in the above special
> condition. So users (we programmers) would use in an invalid way in the future
> and debugging will be hard.

How about following commit log:

    sheep: allow {register,unregister}_event to be called in worker thread
    
    For now we can only call them in the main thread, which is designed for long
    running or infrequent events. This would be inefficient if we want to deal with
    short running and frequent events that register/unregister the events in the
    worker thread
    
    1. avoid to be trapped to main thread for performance
    2. make sure registeration is done before some other events.
    
    This doesn't mean we can manipulate the same event with multiple threads
    simultaneously, instead we still adhere to the assumpioin that
    
    - one event will only be manipluated by a single entity.

Thanks
Yuan