Re: workqueue code needing preemption disabled

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 2013-03-18 at 11:21 -0700, Tejun Heo wrote:

> I've been thinking about it and AFAICS the only way that BUG_ON()
> could trigger from preemption is if preemption happens while the
> idle_list head is becoming or stopping being empty.
> ie. pool->worklist is half updated so list_empty() isn't true but the
> first next entry is already pointing back to itself.  If there's a
> crashdump, it shouldn't be too difficult to verify and wrapping the
> above two functions should resolve it.

I like the theory, but it has one flaw. I agree that the update should
be wrapped in preempt_disable() but since this bug happens on the same
CPU, the state of the list will be the same when it was preempted to
when it bugged. That said:

static inline int list_empty(const struct list_head *head)
{
	return head->next == head;
}

That means when the task was preempted, head->next will either be
pointing to the next element or back to the list head. Which means if we
get preempted while updating the list, it will either see the head->next
== head or head->next == the next element.

first_worker() returns list_first_entry() which returns head->next. I
can't see how it would see the list_head and have list_empty() return
false.

-- Steve