On 02/18/2014 10:30 AM, Tejun Heo wrote:
On Mon, Feb 17, 2014 at 09:29:34PM -0500, Peter Hurley wrote:
It never would have occurred to me that you could safely change the
function for a work item that is already scheduled or running.
Especially given that PREPARE_WORK() is just a simple assignment (i.e.
no serialisation).
process_one_work() has an established order that safely allows for
resetting the work function and scheduling the work, and further
guaranteeing that the new work function will run.
Further, existing memory barriers ensure that
1. The new work function is visible on all cpus before testing if
the work is already pending.
2. The new work function is stored as the worker's current function
before the work is marked as not pending.
If this wasn't possible, then single-threaded workqueues could
not be used for multiple functions without flushing work.
I wonder if the floppy driver is broken too.
Ugh... I'd just rather remove PREPARE_WORK altogether.
Ok.
That doesn't make the use-case go away; it simply moves it outside
the workqueue subsystem.
For example, in the case of the firewire subsystem, this technique
was used to essentially single-thread per-device work using only one
designated workqueue for all devices. The possibility of accidentally
running a work item 2x is a non-issue since the device state is
managed atomically.
Of the other use cases in the kernel, it seems only the floppy
driver uses a similar technique. But maybe that's ok because it's
on a single-threaded workqueue.
USB and AFS use PREPARE_{DELAYED}_WORK to reschedule from within
the current work function to a new function, which seems ok.
fwserial already serializes its use of PREPARE_WORK with &peer->lock
(and checks if the work is already pending).
It's a pretty dumb thing to do anyway.
Fragile, yes; dumb, no. At least not from the point-of-view of the
documentation and what the workqueue actually did. But obviously from
your reaction, unintentional design.
I'll look into it.
Thanks.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html