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

MORITA Kazutaka morita.kazutaka at gmail.com
Thu Jan 16 15:30:40 CET 2014


At Thu, 16 Jan 2014 22:06:18 +0800,
Liu Yuan wrote:
> 
> On Thu, Jan 16, 2014 at 10:58:35PM +0900, MORITA Kazutaka wrote:
> > At Thu, 16 Jan 2014 16:32:42 +0800,
> > Liu Yuan wrote:
> > > 
> > > Since current code works well, do we really need to introduce prioritized event
> > > loop which need some overhead of sorting events?
> > > 
> > > For now with this patch set, the only benefit is that it looks more 'beautiful'
> > > that we have complete reg/unreg for worker threads, which isn't must for making
> > > code correct nor run faster.
> > > 
> > > The downside is obvious, that 1) reg/unreg in worker is worsen a bit 2) event loop
> > > is affected negatively by sorting overhead. We don't get pratical benefits.
> > > 
> > > I want to get other developer's opinion.
> > 
> > To be honest, I don't like both.
> > 
> > The current code assumes that register_event() must be called by at
> > most one thread.  However, we don't do any check about it in
> > register_event().  I don't think Hitoshi's approach is a good one,
> > too.  It adds many codes and complexity.
> > 
> > Do we really need exec_local_req_async() and the short event
> > framework?  The use case of exec_local_req_async() is:
> > 
> >     iocb = local_req_init();
> > 
> >     loop:
> >         ...
> >         exec_local_req_async(req, iocb);
> >         ...
> > 
> >     ret = local_req_wait(iocb);
> > 
> > However, we can do the similar thing with a work queue:
> > 
> >     wq = create_work_queue();
> > 
> >     loop:
> >         ...
> >         work.fn = do_work;
> >         work.done = do_main;
> >         queue_work(wq, &work);
> >         ...
> > 
> >     while (nr--) {
> >         eventfd_read(efd);
> >     }
> > 
> > ...
> > 
> >     void do_work()
> >     {
> >         exec_local_req(req);
> >     }
> > 
> >     void do_main()
> >     {
> >         eventfd_write(efd);
> >     }
> > 
> > 
> > If we can move forward without introducing an additional framework
> > like a short event, it looks much better way to me.
> > 
> 
> Looks feasible to me (looks the same as that in object cache), but can you
> provide the complete patch? I hope we can get a nice API to do async request
> without touching low level work queue and eventfd stuff.

Sure, I'll do it tomorrow.

Thanks,

Kazutaka



More information about the sheepdog mailing list