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

MORITA Kazutaka morita.kazutaka at gmail.com
Thu Jan 16 14:58:35 CET 2014


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.

Thanks,

Kazutaka



More information about the sheepdog mailing list