On Sat, Feb 22, 2014 at 07:11:51AM -0500, Peter Hurley wrote: > Users of the workqueue api may assume the workqueue provides a > memory ordering guarantee for re-queued work items; ie., that > if a work item is not queue-able then the previously queued > work instance is not running and so any memory operations > which occur before queuing the work will be visible to the work > function. > > For example, code of the form: > add new data to work on > queue work > assumes that this latest data is acted upon, either by the > newly queued instance (if it could be queued) or by the not-yet- > running instance (if a new instance could not be queued). > > Provide this implicit memory ordering guarantee; prevent > speculative loads in the work function from occurring before > the current work instance is marked not pending (and thus has > started). This, in turn, guarantees that stores occurring before > schedule_work/queue_work are visible to either the not-yet-running > work instance (if new work could not be queued) or that new work > is queued (and thus ensuring the new data is acted upon). > > Note that preventing early stores is unnecessary because no > conclusion can be reached about the state of the work function > from outside the work function by ordering early stores after > clearing PENDING (other than testing PENDING). > > Signed-off-by: Peter Hurley <pe...@hurleysoftware.com> > --- > kernel/workqueue.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index 82ef9f3..a4b241d 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -2201,6 +2201,16 @@ __acquires(&pool->lock) > > spin_unlock_irq(&pool->lock); > > + /* > + * Paired with the implied mb of test_and_set_bit(PENDING). > + * Guarantees speculative loads which occur in the work item > + * function do not complete before PENDING is cleared in > + * set_work_pool_and_clear_pending() above. In turn, this > + * ensures that stores are either visible to the not-yet- > + * running work instance or a new instance is queueable. > + */ > + smp_rmb();
Wouldn't it make more sense to have the above right after clear_pending? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/