lkml.org 
[lkml]   [2009]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH] create workqueue threads only when needed
    On Tue, Jan 27, 2009 at 02:46:51AM +0100, Oleg Nesterov wrote:
    > Frederic Weisbecker <fweisbec@gmail.com> wrote:
    > >
    > > static void insert_work(struct cpu_workqueue_struct *cwq,
    > > struct work_struct *work, struct list_head *head)
    > > {
    > > - trace_workqueue_insertion(cwq->thread, work);
    > > + trace_workqueue_insertion(cwq->thread, work, cwq->wq->singlethread);
    > >
    > > set_wq_data(work, cwq);
    > > /*
    > > @@ -148,6 +176,9 @@ static void __queue_work(struct cpu_workqueue_struct *cwq,
    > > {
    > > unsigned long flags;
    > >
    > > + if (!cwq->thread)
    > > + create_wq_thread_late(cwq);
    > > +
    >
    > [...snip...]
    >
    > > +static void create_wq_thread_late_work(struct work_struct *work)
    > > +{
    > > + struct late_workqueue_creation_data *l;
    > > + struct cpu_workqueue_struct *cwq;
    > > + int cpu = smp_processor_id();
    > > + int err = 0;
    > > +
    > > + l = container_of(work, struct late_workqueue_creation_data, work);
    > > + cwq = l->cwq;
    > > +
    > > + if (is_wq_single_threaded(cwq->wq)) {
    > > + err = create_workqueue_thread(cwq, singlethread_cpu);
    > > + start_workqueue_thread(cwq, -1);
    > > + } else {
    > > + err = create_workqueue_thread(cwq, cpu);
    > > + start_workqueue_thread(cwq, cpu);
    > > + }
    > > + WARN_ON_ONCE(err);
    > > + kfree(l);
    > > +}
    >
    > Let's suppose the workqueue was just created, and cwq->thared == NULL
    > on (say) CPU 0.
    >
    > Then CPU 0 does
    >
    > queue_work(wq, work1);
    > queue_work(wq, work2);
    >
    > Both these calls will notice cwq->thread == NULL, both will schedule
    > the work wilth ->func = create_wq_thread_late_work.
    >
    > The first work correctly creates cwq->thread, the second one creates
    > the new thread too and replaces cwq->thread? Now we have two threads
    > which run in parallel doing the same work, but the first thread is
    > "stealth", no?


    You're right. I will put a mutex + a recheck of the cwq->thread inside
    create_wq_thread_late_work to be sure there is no race during creation.


    > > @@ -904,9 +967,12 @@ static void cleanup_workqueue_thread(struct cpu_workqueue_struct *cwq)
    > > * checks list_empty(), and a "normal" queue_work() can't use
    > > * a dead CPU.
    > > */
    > > - trace_workqueue_destruction(cwq->thread);
    > > - kthread_stop(cwq->thread);
    > > - cwq->thread = NULL;
    > > +
    > > + if (cwq->thread) {
    > > + trace_workqueue_destruction(cwq->thread, cwq->wq->singlethread);
    > > + kthread_stop(cwq->thread);
    > > + cwq->thread = NULL;
    > > + }
    >
    > cleanup_workqueue_thread() has already checked cwq->thread != NULL,
    > how can it become NULL ?


    Right.


    > And let's suppose a user does:
    >
    > wq = create_workqueue(...., when_needed => 1);
    > queue_work(wq, some_work);
    > destroy_workqueue(wq);
    >
    > This can return before create_wq_thread_late() populates the necessary
    > cwq->thread. We can destroy/free workqueue with the pending work_structs,
    > no?
    >
    > Oleg.
    >

    Totally right. I 'll fix these bugs.

    Thanks!



    \
     
     \ /
      Last update: 2009-01-27 09:43    [W:0.054 / U:58.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site