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.
Thanks
Yuan
|
|