Re: [RFC v1 1/3] work-simple: Simple work queue implemenation

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

* Daniel Wagner | 2014-07-11 15:26:11 [+0200]:

>diff --git a/kernel/sched/work-simple.c b/kernel/sched/work-simple.c
>--- /dev/null
>+++ b/kernel/sched/work-simple.c
…
>+static int swork_kthread(void *arg)
>+{
>+	struct sworker *sw = arg;
>+	struct swork_event *ev;
>+
>+	pr_info("swork_kthread enter\n");
>+
>+	for (;;) {
>+		swait_event_interruptible(sw->wq,
>+					swork_readable(sw));
>+		if (kthread_should_stop())
>+			break;
>+
>+		raw_spin_lock(&sw->lock);

why not the _irq() suffix?

>+		while (!list_empty(&sw->events)) {
>+			ev = list_first_entry(&sw->events,
>+					struct swork_event, list);
>+			list_del_init(&ev->list);
>+
>+			raw_spin_unlock(&sw->lock);
>+
>+			ev->func(ev);
>+			xchg(&ev->flags, 0);

I've been looking at this for a while and this won't work in longterm.
Why do you bother using xchg if you don't look at the return value?
Also, you need to clear that flag _before_ invoking ->func() because
while that function is beeing executed someone might do another
queue_work() and expects that that work item invoked again. Atleast this
is how queue_work() behaves and I would want the same here.

But don't worry. I fix this up to me needs so there is nothing you need
to worry about. The remaining part is simple. Thanks.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html