Messages in this thread | | | Date | Sat, 25 Jun 2011 13:43:31 +0200 | From | Tejun Heo <> | Subject | Re: [RFC 1/1] workqueue: Add mod_delayed_work() |
| |
Hello,
On Fri, Jun 24, 2011 at 07:27:37PM -0300, Gustavo F. Padovan wrote: > mod_delayed_work() updates a timer if the work is pending otherwise calls > queue_delayed_work_on() to queue the work with the specified delay. > > Call cancel_delayed_work_sync() and then queue_delayed_work() again to > change a timer's delays is too expensive (and requires process context). > Istead we call mod_delayed_work() to only modify the timer's timeout.
Yes, this part of the interface is lacking. It might be best to modify queue_delayed_work() to adjust the timer according to the new timeout but we would need to audit the current users to make sure nothing breaks and I agree introducing a new function probably makes sense.
> +int mod_delayed_work(struct workqueue_struct *wq, > + struct delayed_work *dwork, unsigned long delay) > +{ > + struct timer_list *timer = &dwork->timer; > + struct work_struct *work = &dwork->work; > + > + if (!test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) > + return queue_delayed_work_on(-1, wq, dwork, delay); > + > + BUG_ON(!timer_pending(timer)); > + > + mod_timer(timer, jiffies + delay); > + > + return 0; > +}
But I think the current implementation is as it is because modifying delayed work safely wasn't very simple. The above code is broken in multiple ways - a delayed work could be pending without timer pending, and timer may expire after test_bit() but before the rest of the code.
I haven't thought about it too hard but think it would require the timer sync part of __cancel_work_timer() (sans wait_on_work()) to get it correctly. Care to delve into it?
Thanks.
-- tejun
| |