lkml.org 
[lkml]   [2007]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 6/6] Convert tasklets to work queues
    From
    Date
    On Fri, 2007-06-22 at 00:00 -0400, Steven Rostedt wrote:
    > plain text document attachment (tasklets-to-workqueues.patch)
    > This patch creates an alternative for drivers from using tasklets.
    > It creates a "work_tasklet". When configured to use work_tasklets
    > instead of tasklets, instead of creating tasklets, a work queue
    > is made in its place. The API is still the same, and the drivers
    > don't know that a work queue is being used.
    >
    > This is (for now) a proof of concept approach to using work queues
    > instead of tasklets. More can be done. For one thing, we could make
    > individual work queues for each tasklet instead of using the global
    > ktasklet_wq. But for now it's just easier to do this.
    >
    > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    >
    >
    > Index: linux-2.6-test/kernel/Kconfig.preempt
    > ===================================================================
    > --- linux-2.6-test.orig/kernel/Kconfig.preempt
    > +++ linux-2.6-test/kernel/Kconfig.preempt
    > @@ -63,3 +63,18 @@ config PREEMPT_BKL
    > Say Y here if you are building a kernel for a desktop system.
    > Say N if you are unsure.
    >
    > +config TASKLETS_AS_WORKQUEUES
    > + bool "Treat tasklets as workqueues (EXPERIMENTAL)"
    > + depends on EXPERIMENTAL
    > + help
    > + Tasklets are an old solution to an old problem with respect
    > + to SMP. But today they are not necessary anymore.
    > + There are better solutions to the problem that tasklets
    > + are trying to solve.
    > +
    > + This option converts tasklets into workqueues and
    > + removes the tasklet softirq. This is a clean up until
    > + we get rid of all tasklets that are currently in
    > + the kernel.
    > +
    > + Say Y if you are unsure and brave (not very well tested code!).
    > Index: linux-2.6-test/kernel/Makefile
    > ===================================================================
    > --- linux-2.6-test.orig/kernel/Makefile
    > +++ linux-2.6-test/kernel/Makefile
    > @@ -21,7 +21,11 @@ obj-$(CONFIG_FUTEX) += futex.o
    > ifeq ($(CONFIG_COMPAT),y)
    > obj-$(CONFIG_FUTEX) += futex_compat.o
    > endif
    > +ifeq ($(CONFIG_TASKLETS_AS_WORKQUEUES), y)
    > +obj-y += tasklet_work.o
    > +else
    > obj-y += tasklet.o
    > +endif
    > obj-$(CONFIG_RT_MUTEXES) += rtmutex.o
    > obj-$(CONFIG_DEBUG_RT_MUTEXES) += rtmutex-debug.o
    > obj-$(CONFIG_RT_MUTEX_TESTER) += rtmutex-tester.o
    > Index: linux-2.6-test/init/main.c
    > ===================================================================
    > --- linux-2.6-test.orig/init/main.c
    > +++ linux-2.6-test/init/main.c
    > @@ -121,6 +121,11 @@ extern void time_init(void);
    > /* Default late time init is NULL. archs can override this later. */
    > void (*late_time_init)(void);
    > extern void softirq_init(void);
    > +#ifdef CONFIG_TASKLETS_AS_WORKQUEUES
    > + extern void init_tasklets(void);
    > +#else
    > +# define init_tasklets() do { } while(0)
    > +#endif
    >
    > /* Untouched command line saved by arch-specific code. */
    > char __initdata boot_command_line[COMMAND_LINE_SIZE];
    > @@ -706,6 +711,7 @@ static void __init do_basic_setup(void)
    > {
    > /* drivers will send hotplug events */
    > init_workqueues();
    > + init_tasklets();
    > usermodehelper_init();
    > driver_init();
    > init_irq_proc();
    > Index: linux-2.6-test/include/linux/tasklet.h
    > ===================================================================
    > --- linux-2.6-test.orig/include/linux/tasklet.h
    > +++ linux-2.6-test/include/linux/tasklet.h
    > @@ -1,6 +1,10 @@
    > #ifndef _LINUX_TASKLET_H
    > #define _LINUX_TASKLET_H
    >
    > -#include <linux/tasklet_softirq.h>
    > +#ifdef CONFIG_TASKLETS_AS_WORKQUEUES
    > +# include <linux/tasklet_work.h>
    > +#else
    > +# include <linux/tasklet_softirq.h>
    > +#endif
    >
    > #endif
    > Index: linux-2.6-test/include/linux/tasklet_work.h
    > ===================================================================
    > --- /dev/null
    > +++ linux-2.6-test/include/linux/tasklet_work.h
    > @@ -0,0 +1,62 @@
    > +#ifndef _LINUX_WORK_TASKLET_H
    > +#define _LINUX_WORK_TASKLET_H
    > +
    > +#ifndef _LINUX_INTERRUPT_H
    > +# error "Do not include this header directly! use linux/interrupt.h"
    > +#endif
    > +
    > +#include <linux/workqueue.h>
    > +
    > +extern void work_tasklet_exec(struct work_struct *work);
    > +
    > +struct tasklet_struct
    > +{
    > + struct work_struct work;
    > + struct list_head list;
    > + unsigned long state;
    > + atomic_t count;
    > + void (*func)(unsigned long);
    > + unsigned long data;
    > + char *n;
    > +};
    > +
    > +#define DECLARE_TASKLET(name, func, data) \
    > + struct tasklet_struct name = { \
    > + __WORK_INITIALIZER((name).work, work_tasklet_exec), \
    > + LIST_HEAD_INIT((name).list), \
    > + 0, \
    > + ATOMIC_INIT(0), \
    > + func, \
    > + data, \
    > + #name \
    > + }
    > +
    > +#define DECLARE_TASKLET_DISABLED(name, func, data) \
    > + struct tasklet_struct name = { \
    > + __WORK_INITIALIZER((name).work, work_tasklet_exec), \
    > + LIST_HEAD_INIT((name).list), \
    > + 0, \
    > + ATOMIC_INIT(1), \
    > + func, \
    > + data, \
    > + #name \
    > + }
    > +
    > +void tasklet_schedule(struct tasklet_struct *t);
    > +#define tasklet_hi_schedule tasklet_schedule
    > +extern fastcall void tasklet_enable(struct tasklet_struct *t);
    > +#define tasklet_hi_enable tasklet_enable
    > +
    > +void tasklet_disable_nosync(struct tasklet_struct *t);
    > +void tasklet_disable(struct tasklet_struct *t);
    > +
    > +extern int tasklet_is_scheduled(struct tasklet_struct *t);
    > +
    > +extern void tasklet_kill(struct tasklet_struct *t);
    > +extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu);
    > +extern void tasklet_init(struct tasklet_struct *t,
    > + void (*func)(unsigned long), unsigned long data);
    > +void takeover_tasklets(unsigned int cpu);
    > +
    > +
    > +#endif /* _LINUX_WORK_TASKLET_H */
    > Index: linux-2.6-test/kernel/tasklet_work.c
    > ===================================================================
    > --- /dev/null
    > +++ linux-2.6-test/kernel/tasklet_work.c
    > @@ -0,0 +1,138 @@
    > +/*
    > + * linux/kernel/work_tasklet.c
    > + *
    > + * Copyright (C) 2007 Steven Rostedt, Red Hat
    > + *
    > + */
    > +
    > +#include <linux/interrupt.h>
    > +
    > +static struct workqueue_struct *ktaskletd_wq;
    > +
    > +enum
    > +{
    > + TASKLET_STATE_SCHED, /* Tasklet is scheduled for execution */
    > + TASKLET_STATE_RUN, /* Tasklet is running (SMP only) */
    > + TASKLET_STATE_PENDING /* Tasklet is pending */
    > +};
    > +
    > +#define TASKLET_STATEF_SCHED (1 << TASKLET_STATE_SCHED)
    > +#define TASKLET_STATEF_RUN (1 << TASKLET_STATE_RUN)
    > +#define TASKLET_STATEF_PENDING (1 << TASKLET_STATE_PENDING)
    > +
    > +void tasklet_schedule(struct tasklet_struct *t)
    > +{
    > + BUG_ON(!ktaskletd_wq);
    > + pr_debug("scheduling tasklet %s %p\n", t->n, t);

    I'd change these pr_debug lines to "tasklet : scheduling %s %p\n" for
    readability ..

    > + queue_work(ktaskletd_wq, &t->work);
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_schedule);
    > +
    > +int tasklet_is_scheduled(struct tasklet_struct *t)
    > +{
    > + int ret;
    > + ret = work_pending(&t->work);
    > + pr_debug("sched %s pending=%d\n", t->n, ret);
    > + return ret;
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_is_scheduled);
    > +
    > +void tasklet_disable_nosync(struct tasklet_struct *t)
    > +{
    > + pr_debug("disable tasklet %s %p\n", t->n, t);
    > + atomic_inc(&t->count);
    > + smp_mb__after_atomic_inc();
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_disable_nosync);
    > +
    > +void tasklet_disable(struct tasklet_struct *t)
    > +{
    > + tasklet_disable_nosync(t);
    > + pr_debug("flush tasklet %s %p\n", t->n, t);
    > + flush_workqueue(ktaskletd_wq);
    > + smp_mb();
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_disable);
    > +
    > +void work_tasklet_exec(struct work_struct *work)
    > +{
    > + struct tasklet_struct *t =
    > + container_of(work, struct tasklet_struct, work);
    > +
    > + if (unlikely(atomic_read(&t->count))) {
    > + pr_debug("tasklet disabled %s %p\n", t->n, t);
    > + set_bit(TASKLET_STATE_PENDING, &t->state);
    > + smp_mb();
    > + /* make sure we were not just enabled */
    > + if (likely(atomic_read(&t->count)))
    > + goto out;
    > + clear_bit(TASKLET_STATE_PENDING, &t->state);

    smp_mb__before_clear_bit ?

    > + }
    > +
    > + local_bh_disable();
    > + pr_debug("run tasklet %s %p\n", t->n, t);
    > + t->func(t->data);
    > + local_bh_enable();
    > +
    > +out:
    > + return;
    > +}
    > +
    > +EXPORT_SYMBOL(work_tasklet_exec);
    > +
    > +void __init softirq_init(void)
    > +{
    > +}

    ifdef's ?

    > +void init_tasklets(void)
    > +{
    > + ktaskletd_wq = create_workqueue("tasklets");
    > + BUG_ON(!ktaskletd_wq);
    > +}
    > +
    > +void takeover_tasklets(unsigned int cpu)
    > +{
    > + pr_debug("Implement takeover tasklets??\n");
    > +}

    hmm .. Looks like it's for migration of tasklets .. I take it your not
    sure that's handled ? Try cpu hotplug ..

    > +void tasklet_init(struct tasklet_struct *t,
    > + void (*func)(unsigned long), unsigned long data)
    > +{
    > + INIT_WORK(&t->work, work_tasklet_exec);
    > + INIT_LIST_HEAD(&t->list);
    > + t->state = 0;
    > + atomic_set(&t->count, 0);
    > + t->func = func;
    > + t->data = data;
    > + t->n = "anonymous";

    Is this "anonymous" just used for debugging ? Is so you could fill it
    with a kallsyms lookup with the __builtin_return_address() ..

    > + pr_debug("anonymous tasklet %p set at %p\n",
    > + t, __builtin_return_address(0));
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_init);
    > +
    > +void fastcall tasklet_enable(struct tasklet_struct *t)
    > +{
    > + pr_debug("enable tasklet %s (count was %d)\n",
    > + t->n, atomic_read(&t->count));
    > + if (!atomic_dec_and_test(&t->count))
    > + return;
    > + if (test_and_clear_bit(TASKLET_STATE_PENDING, &t->state)) {
    > + pr_debug("tasklet %s was pending\n", t->n);
    > + tasklet_schedule(t);
    > + }
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_enable);
    > +
    > +void tasklet_kill(struct tasklet_struct *t)
    > +{
    > + pr_debug("kill tasklet %s\n", t->n);
    > + flush_workqueue(ktaskletd_wq);
    > +}
    > +
    > +EXPORT_SYMBOL(tasklet_kill);
    >

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-06-22 09:13    [W:0.043 / U:0.092 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site