Skip to contentSkip to conversation

workqueue: do a sanity check on new work

RFC In review Dan Carpenter 2 participants 5 messages Apr 13, 2012 – Apr 16, 2012 Workqueue

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

RFC 1/1 workqueue: do a sanity check on new work

There may be a better way to do this. If someone tries to call shedule_work() on a work_struck before doing an INIT_WORK() then we hit the BUG_ON(!list_empty(&work->entry)) in __queue_work() and hang. Instead of that, we could just print a stack dump and return.

It only works if the work->func is NULL at the start but a lot of these things get initialized with kzalloc() so it probably catches most of them.

Reported-by Matt Renzelmann <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>
kernel/workqueue.c +5 -0
index 5abf42f..45be34f 100644
@@ -1088,6 +1088,11 @@ queue_work_on(int cpu, struct workqueue_struct *wq, struct work_struct *work)
 {
 	int ret = 0;
 
+	if (!work->func) {
+		WARN_ON(1);
+		return 1;
+	}
+
 	if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
 		__queue_work(cpu, wq, work);
 		ret = 1;
maintainer
#1 lore raw

Hello, Dan.

Quoted 4 lines · Dan Carpenter

There may be a better way to do this.  If someone tries to call shedule_work() on a work_struck before doing an INIT_WORK() then we hit the BUG_ON(!list_empty(&work->entry)) in __queue_work() and hang. Instead of that, we could just print a stack dump and return.

Hmmm... why not just change the BUG_ON() in __queue_work() to "if (WARN_ON()) return;"?

Thanks.

Signature · 1 line
tejun
author
#2 lore raw
Quoted 11 lines · Tejun Heo

Hello, Dan.

Quoted 4 lines · Dan Carpenter

There may be a better way to do this.  If someone tries to call shedule_work() on a work_struck before doing an INIT_WORK() then we hit the BUG_ON(!list_empty(&work->entry)) in __queue_work() and hang. Instead of that, we could just print a stack dump and return.

Hmmm... why not just change the BUG_ON() in __queue_work() to "if (WARN_ON()) return;"?

Good point. I'll do that.

regards,
dan carpenter

author
#3 lore raw

PATCH v2 1/1 workqueue: change BUG_ON() to WARN_ON()

In reply to Tejun Heo

This BUG_ON() can be triggered if you call schedule_work() before calling INIT_WORK(). It is a bug definitely, but it's nicer to just print a stack trace and return.

Reported-by Matt Renzelmann <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>

---
v2: the first version added a second check before the BUG_ON()

kernel/workqueue.c +4 -1
index 5abf42f..66ec08d 100644
@@ -1032,7 +1032,10 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq,
 	cwq = get_cwq(gcwq->cpu, wq);
 	trace_workqueue_queue_work(cpu, cwq, work);
 
-	BUG_ON(!list_empty(&work->entry));
+	if (WARN_ON(!list_empty(&work->entry))) {
+		spin_unlock_irqrestore(&gcwq->lock, flags);
+		return;
+	}
 
 	cwq->nr_in_flight[cwq->work_color]++;
 	work_flags = work_color_to_flags(cwq->work_color);
maintainer Applied
#4 lore raw

Re: [patch v2] workqueue: change BUG_ON() to WARN_ON()

Quoted 6 lines · Dan Carpenter

This BUG_ON() can be triggered if you call schedule_work() before calling INIT_WORK(). It is a bug definitely, but it's nicer to just print a stack trace and return.

Reported-by Matt Renzelmann <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>

Applied to wq/for-3.5. Thanks.

Signature · 1 line
tejun