Messages in this thread | | | Date | Wed, 28 Apr 2010 08:44:24 +0200 | From | Tejun Heo <> | Subject | Re: [PATCH 6/8] PM: Add suspend blocking work. |
| |
Hello,
> +static void suspend_blocking_work_complete(struct suspend_blocking_work *work) > +{ > + unsigned long flags; > + > + WARN_ON(!work->active); > + spin_lock_irqsave(&work->lock, flags); > + if (!--work->active) > + suspend_unblock(&work->suspend_blocker); > + spin_unlock_irqrestore(&work->lock, flags); > +}
Maybe work->active can be an atomic_t and the lock can be removed?
> +/** > + * suspend_blocking_work_destroy - Destroy suspend_blocking_work > + * @work: The work item in question > + * > + * If the work was ever queued on more then one workqueue all but the last > + * workqueue must be flushed before calling suspend_blocking_work_destroy.
As it's calling cancel_work_sync(), the above is not true. As long as no one is trying to queue it again, suspend_blocking_work_destroy() is safe to call regardless of how the work has been used.
> +void suspend_blocking_work_destroy(struct suspend_blocking_work *work) > +{ > + cancel_suspend_blocking_work_sync(work); > + WARN_ON(work->active); > + suspend_blocker_destroy(&work->suspend_blocker); > +} > +EXPORT_SYMBOL(suspend_blocking_work_destroy);
Other than the above, it looks good to me.
Thanks.
-- tejun
| |