Messages in this thread | | | Date | Wed, 28 Jun 2006 14:32:42 -0700 | From | Andrew Morton <> | Subject | Re: [RFC] Adding queue_delayed_work_on interface for workqueues |
| |
Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> wrote: > > > This patch is a part of cpufreq patches for ondemand governor optimizations > and entire series is actually posted to cpufreq mailing list. > Subject "minor optimizations to ondemand governor" > > The following patch however is a generic change to workqueue interface and > I wanted to get comments on this on lkml. > > ... > > Add queue_delayed_work_on() interface for workqueues.
It looks sensible to me.
> +extern int FASTCALL(queue_delayed_work_on(int cpu, struct workqueue_struct *wq, struct work_struct *work, unsigned long delay));
Please wrap at 80-cols.
I wouldn't bother making this FASTCALL. It's an ugly thing, and why this particular function? And this isn't fastpath stuff.
> > -extern int schedule_delayed_work_on(int cpu, struct work_struct *work, unsigned long delay); > +extern int FASTCALL(schedule_delayed_work_on(int cpu, struct work_struct *work, unsigned long delay));
Ditto.
> } > > +int fastcall queue_delayed_work_on(int cpu, struct workqueue_struct *wq, > + struct work_struct *work, unsigned long delay) > +{ > + int ret = 0; > + struct timer_list *timer = &work->timer; > + > + if (!test_and_set_bit(0, &work->pending)) { > + BUG_ON(timer_pending(timer)); > + BUG_ON(!list_empty(&work->entry)); > + > + /* This stores wq for the moment, for the timer_fn */ > + work->wq_data = wq; > + timer->expires = jiffies + delay; > + timer->data = (unsigned long)work; > + timer->function = delayed_work_timer_fn; > + add_timer_on(timer, cpu); > + ret = 1; > + } > + return ret; > +}
Feel free to add some kernel-doc for this function ;)
> @@ -608,6 +615,7 @@ void init_workqueues(void) > EXPORT_SYMBOL_GPL(__create_workqueue); > EXPORT_SYMBOL_GPL(queue_work); > EXPORT_SYMBOL_GPL(queue_delayed_work); > +EXPORT_SYMBOL_GPL(queue_delayed_work_on); > EXPORT_SYMBOL_GPL(flush_workqueue); > EXPORT_SYMBOL_GPL(destroy_workqueue);
Opinions vary a bit, but I think we mostly prefer to put the EXPORT_SYMBOL()s at the site of the thing which is being exported:
foo() { } EXPORT_SYMBOL(foo);
because it keeps all the info in the same place. (We don't declare functions to be static or to return char* or to be __init at a different place in the source file..).
Then again, it's legit to follow existing local style too. Someone will come along later and fix it all in a single hit. Whatever.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |