Re: [PATCH 19/21] workqueue: introduce worker

Louis Rilling
2009-11-17T11:51:39Z

[-- Attachment #1: Type: text/plain, Size: 3259 bytes --]

On 17/11/09 12:39 +0100, Louis Rilling wrote:
> Hi Tejun,
>
> On 17/11/09 2:15 +0900, Tejun Heo wrote:
> > Separate out worker thread related information to struct worker from
> > struct cpu_workqueue_struct and implement helper functions to deal
> > with the new struct worker. The only change which is visible outside
> > is that now workqueue worker are all named "kworker/CPUID:WORKERID"
> > where WORKERID is allocated from per-cpu ida.
> >
> > This is in preparation of concurrency managed workqueue where shared
> > multiple workers would be available per cpu.
> >
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > ---
> > kernel/workqueue.c | 219 +++++++++++++++++++++++++++++++++++++---------------
> > 1 files changed, 157 insertions(+), 62 deletions(-)
> >
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index dce5ad5..6694b3e 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
>
> [...]
>
> > @@ -413,6 +426,105 @@ int queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
> > }
> > EXPORT_SYMBOL_GPL(queue_delayed_work_on);
> >
> > +static struct worker *alloc_worker(void)
> > +{
> > + struct worker *worker;
> > +
> > + worker = kzalloc(sizeof(*worker), GFP_KERNEL);
> > + return worker;
> > +}
> > +
> > +/**
> > + * create_worker - create a new workqueue worker
> > + * @cwq: cwq the new worker will belong to
> > + * @bind: whether to set affinity to @cpu or not
> > + *
> > + * Create a new worker which is bound to @cwq. The returned worker
> > + * can be started by calling start_worker() or destroyed using
> > + * destroy_worker().
> > + *
> > + * CONTEXT:
> > + * Might sleep. Does GFP_KERNEL allocations.
> > + *
> > + * RETURNS:
> > + * Pointer to the newly created worker.
> > + */
> > +static struct worker *create_worker(struct cpu_workqueue_struct *cwq, bool bind)
> > +{
> > + int id = -1;
> > + struct worker *worker = NULL;
> > +
> > + spin_lock(&workqueue_lock);
> > + while (ida_get_new(&per_cpu(worker_ida, cwq->cpu), &id)) {
> > + spin_unlock_irq(&workqueue_lock);
> > + if (!ida_pre_get(&per_cpu(worker_ida, cwq->cpu), GFP_KERNEL))
> > + goto fail;
> > + spin_lock(&workqueue_lock);
> > + }
> > + spin_unlock_irq(&workqueue_lock);
> > +
> > + worker = alloc_worker();
> > + if (!worker)
> > + goto fail;
> > +
> > + worker->cwq = cwq;
> > + worker->id = id;
> > +
> > + worker->task = kthread_create(worker_thread, worker, "kworker/%u:%d",
> > + cwq->cpu, id);
> > + if (IS_ERR(worker->task))
> > + goto fail;
> > +
> > + if (bind)
> > + kthread_bind(worker->task, cwq->cpu);
> > +
> > + return worker;
> > +fail:
> > + if (id >= 0) {
> > + spin_lock(&workqueue_lock);
> > + ida_remove(&per_cpu(worker_ida, cwq->cpu), id);
> > + spin_unlock_irq(&workqueue_lock);
> > + }
> > + kfree(worker);
> > + return NULL;
> > +}
>
> AFAIU create_worker() should call spin_lock_irq() instead of spin_lock().

spin_unlock() instead of spin_unlock_irq() looks better in fact ;).

Louis

--
Dr Louis Rilling Kerlabs
Skype: louis.rilling Batiment Germanium
Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
http://www.kerlabs.com/ 35700 Rennes

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]