2010/4/27 Tejun Heo <tj@xxxxxxxxxx>:
> 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?
>
I need the spinlock to prevent the work from getting re-queued before
suspend_unblock.
>> +/**
>> + * 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.
>
I'm not sure what the best terminology is here, but cancel_work_sync()
only waits for work running on all the cpu-workqueues of the last
workqueue. So, if the caller queued the work on more than one
workqueue, suspend_blocking_work_destroy does not ensure that the
suspend_blocking_work structure is not still in use (it should trigger
the WARN_ON though).
>> +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
>
--
Arve Hjønnevåg
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
