lkml.org 
[lkml]   [2008]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] workqueues: insert_work: use "list_head *" instead of "int tail"
    On 06/12, Peter Zijlstra wrote:
    >
    > On Thu, 2008-06-12 at 21:44 +0400, Oleg Nesterov wrote:
    >
    > > > Hence that idea of flush context and completions.
    > >
    > > Do you mean something like (just for example) below? If yes, then yes
    > > sure, flush_work() is limited. But I can't see how it is possible to
    > > "generalize" this idea.
    > >
    > > (hmm... actually, if we add flush_work(), we can speedup schedule_on_each_cpu(),
    > > instead of flush_workqueue(keventd_wq) we can do
    > >
    > > for_each_online_cpu(cpu)
    > > flush_work(per_cpu_ptr(works, cpu));
    > >
    > > not sure this really makes sense though).
    >
    > Speedups are always nice ;-),

    OK, I'm sending the patch.

    > but the below also gets us there.

    yeah, and it needs only 1 wakeup. But otoh it is much more complex :(

    > > +struct xxx
    > > +{
    > > + atomic_t count;
    > > + struct completion done;
    > > + work_func_t func;
    > > +};
    > > +
    > > +struct yyy
    > > +{
    > > + struct work_struct work;
    > > + struct xxx *xxx;
    > > +};
    > > +
    > > +static void yyy_func(struct work_struct *work)
    > > +{
    > > + struct xxx *xxx = container_of(work, struct yyy, work)->xxx;
    > > + xxx->func(work);
    > > +
    > > + if (atomic_dec_and_test(&xxx->count))
    > > + complete(&xxx->done);
    > > +}
    > > ...
    >
    > Yes, along those lines.
    >
    > you can call xxx a flush_context and create an interface like:
    >
    > int queue_work_contex(struct workqueue_struct *wq,
    > struct flush_context *fc, struct work_struct *work)
    > {
    > work->context = fc;
    > return queue_work(wq, work);
    > }
    >
    > void flush_workqueue_context(struct workqueue_strucy *wq, t
    > struct flush_context *fc)
    > {
    > if (atomic_read(&context->count))
    > wait_for_completion(&fc->completion);
    > /* except that the above is racy, wait_event() comes to mind */
    > }
    >
    > of course run_workqueue() would then need to be augmented with something
    > like:
    >
    > context = work->context;
    > ...
    > f(work);
    > ...
    > if (context && atomic_dec_and_test(&context->count))
    > complete(&context->done);

    > also, I seem to have quitely ignored the fact that struct work doesn't
    > have the context pointer, and growing it unconditionally like this isn't
    > nice - hummm,. perhaps we have a bit left in data and can signify a
    > larger struct work_struct.. ?

    Yes, we have a free bit... but afaics we can do better.

    struct context_barrier {
    struct work_struct work;
    struct flush_context *fc;
    ...
    }

    void context_barrier_barrier_func(struct work_struct *work)
    {
    struct flush_context *fc = container_of();
    if (atomic_dec_and_test())
    ...
    }

    void insert_context_barrier(work, barr)
    {
    ...insert barr after work, like flush_work() does...
    }

    queue_work_contex(struct workqueue_struct *wq,
    struct work_struct *work,
    struct flush_context *fc)
    {
    int ret = queue_work(wq, work);
    if (ret)
    insert_context_barrier(work, barr);
    return ret;
    }

    this way we shouldn't change run_workqueue() and introduce a "parallel"
    larger work_struct which needs its own INIT_()/etc.

    However I'm a bit sceptical this will be widely used... I may be wrong.

    > making all this PI savvy for -rt is going to be fun though.. I guess we
    > can just queue a normal barrier of the flusher's priority, and cancel it
    > once we complete.. hey - that doesn't sound hard at all :-)

    Yes!!! I think this is much better (because _much_ simple) than re-ordering
    the pending work_struct's, we can just boost the whole ->worklist. We can
    implement flush_work_pi() in the same manner as queue_work_contex() above.
    That is why I said previously that flush_() should govern the priority,
    not queue.

    But we can also implement queue_work_pi(struct work_struct_pi *work).

    Oleg.



    \
     
     \ /
      Last update: 2008-06-13 16:29    [W:0.031 / U:30.080 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site