Skip to contentSkip to conversation

cancel_rearming_delayed_work infinite loop fix

PATCH In review Michael Buesch 2 participants 2 messages Jul 10, 2006 – Jul 11, 2006

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cancel_rearming_delayed_work infinite loop fix

cancel_rearming_delayed_work{queue} is broken, because it is possible to enter an infinite loop if:
We call the function on a work that is currently not executing or pending.

But, as this is a synchronization function and as its only purpose is to synchronize the work, that should not loop infinite.

This patch rewrites the function.
It now first cancels the work, but throws the retval of that function away, because it is meaningless in this context. (The retval is 1, if the work was pending and 0, if not)
After that we flush the queue to make sure any work function returns. Now, if it was indeed running, it rescheduled itself. That is caught by the test_bit(). If it rescheduled itself, we have the schedule-delay time to cancel the work again (without it rescheduling itself again). So we redo the loop. Most likely it will succeed on the second loop iteration at least.

kernel/workqueue.c +3 -1
Index: wireless-dev-dscapeports/kernel/workqueue.c
===================================================================
@@ -461,8 +461,10 @@
 void cancel_rearming_delayed_workqueue(struct workqueue_struct *wq,
 				       struct work_struct *work)
 {
-	while (!cancel_delayed_work(work))
+	do {
+		cancel_delayed_work(work);
 		flush_workqueue(wq);
+	} while (test_bit(0, &work->pending));
 }
 EXPORT_SYMBOL(cancel_rearming_delayed_workqueue);
 
Signature · 1 line
Greetings Michael.
#1 lore raw
Quoted 3 lines · Michael Buesch

cancel_rearming_delayed_work{queue} is broken, because it is possible to enter an infinite loop if:
We call the function on a work that is currently not executing or pending.

Why are you calling it on a work that was never scheduled? Sounds like a bug to me.

kernel/workqueue.c line 467

Quoted 8 lines

void cancel_rearming_delayed_workqueue(struct workqueue_struct *wq,
struct work_struct *work)
{

kernel/workqueue.c +3 -1
-       while (!cancel_delayed_work(work))
+       do {
+               cancel_delayed_work(work);
               flush_workqueue(wq);
+       } while (test_bit(0, &work->pending));

This is broken. If the work just starts running before your test_bit you'd exit without cancelling it properly.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt