2010/4/27 Tejun Heo <tj@kernel.org>:
quoted text
> 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.

quoted text
>> +/**
>> + * 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).

quoted text
>> +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
--
unsubscribe notice
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/