[PATCH] Avoid a pipe read in aio result scheduling

Jeremy Allison jra at samba.org
Tue Aug 23 03:25:03 UTC 2016


On Mon, Aug 22, 2016 at 12:09:29PM -0700, Jeremy Allison wrote:
> On Mon, Aug 22, 2016 at 05:27:35PM +0200, Volker Lendecke wrote:
> > Hi!
> > 
> > Attached find a patchset that is supposed to improve our async I/O
> > result scheduling. For a more verbose explanation I'd recommend the
> > commit message for [PATCH 04/23].
> > 
> > Jeremy, I did not look deeply enough into your threaded messaging
> > tevent code to see whether the tevent_thread_proxy_schedule()
> > implementation can be based upon tevent_threaded_schedule_immediate().
> > 
> > Thanks metze for very deep review and insights.
> > 
> > Comments?
> 
> Oh it removes the asys stuff ! Very cool (deleted code is always
> good :-).
> 
> I'll take some time and go over this really carefully asap (next
> day or so).

The pthreadpool_tevent() paradigm is *really* cool, makes use
of the threadpool code idiomatic tevent ! Well done !

So, I'm ready to push with my review on most if it  (it passes make
test, and also valgrind and helgrind tests). However I would like
clarification on one thing I don't understand (EREVIEWERTOOSTUPID :-).

But I don't understand exactly what:

static int pthreadpool_tevent_job_destructor(struct pthreadpool_tevent_job_state *state)
{
        if (state->pool == NULL) {
                return 0;
        }

        /*
         * We should never be called with state->req != NULL,
         * state->pool must be cleared before the 2nd talloc_free().
         */
        if (state->req != NULL) {
                abort();
        }

        /*
         * We need to reparent to a long term context.
         */
        (void)talloc_reparent(state->req, state->pool, state);
        state->req = NULL;
        return -1;
}

is doing :-(.

I get the 'normal' case where pthreadpool_tevent_job_done()
is called, so state->pool = NULL and all is well. Can you
explain the failure mode you're explicity guarding against
here with (state->req != NULL) ?



More information about the samba-technical mailing list