workqueues: change cancel_work_sync() to clear work->data
in [Kernel]
|
From: Tejun Heo on 24 Feb 2010 22:10 Hello, On 02/25/2010 05:20 AM, Oleg Nesterov wrote: > In short: change cancel_work_sync(work) to mark this work as "never > queued" upon return. > > When cancel_work_sync(work) succeeds, we know that this work can't be > queued or running, and since we own WORK_STRUCT_PENDING nobody can change > the bits in work->data under us. This means we can also clear the "cwq" > part along with _PENDING bit lockless before return, unless the work is > queued nobody can assume get_wq_data() is stable even under cwq->lock. > > This change can speedup the subsequent cancel/flush requests, and as > Dmitry pointed out this simplifies the usage of work_struct's which > can be queued on different workqueues. Consider this pseudo code from > the input subsystem: > > struct workqueue_struct *WQ; > struct work_struct *WORK; > > for (;;) { > WQ = create_workqueue(); > ... > if (condition()) > queue_work(WQ, WORK); > ... > cancel_work_sync(WORK); > destroy_workqueue(WQ); > } > > If condition() returns T and then F, cancel_work_sync() will crash the > kernel because WORK->data still points to the already destroyed workqueue. > With this patch the code like above becomes correct. > > Suggested-by: Dmitry Torokhov <dmitry.torokhov(a)gmail.com> > Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> Yeap, generally looks good to me and I've been doing similar things to implement non-reentrant workqueues (records the last cpu a work was on in work->dataa after the work is dispatched to a worker thread). I'll add this to the series. Thank you. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Pages: 1 Prev: 2010 Award Promotion Next: workqueues: s/2/WORK_STRUCT_STATIC/ in WORK_DATA_STATIC_INIT() |