Messages in this thread | | | Date | Tue, 28 Jun 2011 15:16:54 -0300 | From | "Gustavo F. Padovan" <> | Subject | Re: [RFC 1/1] workqueue: Add mod_delayed_work() |
| |
Hi Tejun,
* Tejun Heo <tj@kernel.org> [2011-06-25 13:43:31 +0200]:
> 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?
Sure, I can do that. I'm kind of stuck in the Bluetooth changes I need to do due to this workqueue patch. I'll send a -v2 soon.
Gustavo
| |