[sheepdog] [PATCH 2/3] event: add deferred event register/unregister mechanism

[Hitoshi Mitake]

At Thu, 16 Jan 2014 14:11:44 +0800,
Liu Yuan wrote:
> 
> On Mon, Jan 13, 2014 at 05:40:38PM +0900, Hitoshi Mitake wrote:
> > This patch introduces deferred event register/unregister
> > mechanism. Newly added APIs are:
> >  - deferred_register_event(): thread safe register_event()
> >  - deferred_register_event_prio(): thread safe register_event_prio()
> >  - deferred_unregister_event(): thread safe unregister_event()
> 
> 'deferred' doesn't look a good name. 'wk' is better to indicate that it is used
> by worker thread context.

As you say, "deferred" is not a good prefix. I agree. But "wk" would
be too short and hard to interpret. How about "worker"
e.g. worker_register_event()?

> 
> > 
> > These functions can be called by worker threads safely. They allocate
> > data structure which represents registering/unregistering event add
> > queue it to the list shared with the main thread. After queuing,
> > the main thread registers and unregisters events in a safe way.
> > 
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> >  include/event.h |   5 +++
> >  lib/event.c     | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  2 files changed, 124 insertions(+), 13 deletions(-)
> > 
> > diff --git a/include/event.h b/include/event.h
> > index 8f8b21f..b64b06e 100644
> > --- a/include/event.h
> > +++ b/include/event.h
> > @@ -32,4 +32,9 @@ static inline int register_event(int fd, event_handler_t h, void *data)
> >  	return register_event_prio(fd, h, data, EVENT_PRIO_DEFAULT);
> >  }
> >  
> > +void deferred_register_event_prio(int fd, event_handler_t h, void *data,
> > +				  int prio);
> > +void deferred_register_event(int fd, event_handler_t h, void *data);
> > +void deferred_unregister_event(int fd);
> > +
> >  #endif
> > diff --git a/lib/event.c b/lib/event.c
> > index 88078f4..2549dcd 100644
> > --- a/lib/event.c
> > +++ b/lib/event.c
> > @@ -76,19 +76,6 @@ static int event_cmp(const struct event_info *e1, const struct event_info *e2)
> >  	return intcmp(e1->fd, e2->fd);
> >  }
> >  
> > -int init_event(int nr)
> > -{
> > -	nr_events = nr;
> > -	events = xcalloc(nr_events, sizeof(struct epoll_event));
> > -
> > -	efd = epoll_create(nr);
> > -	if (efd < 0) {
> > -		sd_err("failed to create epoll fd");
> > -		return -1;
> > -	}
> > -	return 0;
> > -}
> > -
> >  static struct event_info *lookup_event(int fd)
> >  {
> >  	struct event_info key = { .fd = fd };
> > @@ -224,3 +211,122 @@ void event_loop_prio(int timeout)
> >  {
> >  	do_event_loop(timeout, true);
> >  }
> > +
> > +struct deferred_event_info {
> > +	bool is_register;	/* true: register, false: unregister */
> > +
> > +	int fd;
> > +	event_handler_t h;
> > +	void *data;
> > +	int prio;
> > +
> > +	struct list_node list;
> > +};
> > +
> > +static LIST_HEAD(deferred_event_list);
> > +static struct sd_mutex deferred_event_mutex = SD_MUTEX_INITIALIZER;
> > +
> > +static int deferred_event_fd;
> > +
> > +static void add_deferred_event_info(struct deferred_event_info *info)
> > +{
> > +	sd_mutex_lock(&deferred_event_mutex);
> > +	list_add_tail(&info->list, &deferred_event_list);
> > +	sd_mutex_unlock(&deferred_event_mutex);
> > +
> > +	eventfd_xwrite(deferred_event_fd, 1);
> > +	event_force_refresh();
> > +}
> > +
> > +void deferred_register_event_prio(int fd, event_handler_t h, void *data,
> > +				  int prio)
> > +{
> > +	struct deferred_event_info *info = xzalloc(sizeof(*info));
> > +
> > +	info->is_register = true;
> > +
> > +	info->fd = fd;
> > +	info->h = h;
> > +	info->data = data;
> > +	info->prio = prio;
> > +
> > +	add_deferred_event_info(info);
> > +}
> > +
> > +void deferred_register_event(int fd, event_handler_t h, void *data)
> > +{
> > +	deferred_register_event_prio(fd, h, data, EVENT_PRIO_DEFAULT);
> > +}
> 
> so why deferred_register_event() (which is used by async handler reg) has the
> priority over register_event()?

Registering an event with the default priority doesn't break
the assumption of exec_local_req_async(). Because the registration
itself is done in the highest priority. Quoting from modified
init_event():

+	ret = register_event_prio(deferred_event_fd, deferred_event_handler,
+				  NULL, EVENT_PRIO_MAX);

But I found a problem that sheep doesn't use event_loop_prio(). I'll
modify the problem in the next version, thanks for your question.

Thanks,
Hitoshi