Expected taskqueue behavior

[Alfred Perlstein]

On 1/5/16 7:19 AM, John Baldwin wrote:
> Given this bit of code:
>
> void
> cpu_hog(void *context, int pending)
> {
>
> 	for (;;)
> 		;
> }
>
> static struct taskqueue *tq;
> static struct task t;
>
> void
> mod_init(void)
> {
> 	int i;
>
> 	TASK_INIT(&t, 0, cpu_hog, NULL);
> 	tq = taskqueue_create("test", M_WAITOK, taskqueue_thread_enqueue,
> 	    &tq);
> 	taskqueue_start_threads(&tq, mp_ncpu, PRI_MAX_IDLE, "test taskq");
> 	
> 	for (i = 0; i < mp_ncpu; i++) {
> 		taskqueue_enqueue(tq, &t);
> 		pause("foo", 1);
> 	}
> }
>
> How many threads would you expect to be busy?  The taskqueue(9) manpage
> does not describe what happens when you queue a task that is already
> executing.  Currently our implementation will allow the task to execute
> on a second thread (or a third, fourth, fifth, etc.).  I think this
> behavior is probably "surprising" as I expect most folks assume that
> a task won't be executed concurrently and rely on this assumption to
> avoid the need for certain types of serialization in task handlers.
>
> Linux workqueues explicitly document work-items as being non-reentrant.
>
>  From https://www.kernel.org/doc/Documentation/workqueue.txt
>
> <quote>
> Note that the flag WQ_NON_REENTRANT no longer exists as all workqueues
> are now non-reentrant - any work item is guaranteed to be executed by
> at most one worker system-wide at any given time.
> </quote>
>
> Windows work queues also forbid this though indirectly becuase they
> forbid extra queueing (which bumps 'pending' on FreeBSD) outright.
>
>  From https://msdn.microsoft.com/en-us/library/windows/hardware/ff564587%28v=vs.85%29.aspx
>
> <quote>
> Do not call IoQueueWorkItem or IoQueueWorkItemEx to queue a work item that
> is already in the queue. In checked builds, this error causes a bug check. In
> retail builds, the error is not detected but can cause corruption of system
> data structures.
> </quote>
>
> I think our current behavior is "surprising" and is a bug from when
> taskqueue thread pools were added (the case simply wasn't handled).
>
> At a minimum we should document the expected behavior (and fix it if
> its wrong).  If we don't change our current behavior then the Linux
> KPI workqueue implementation will need additional changes as it can
> no longer simply call taskqueue_enqueue().
>

Agreed.  I had to debug numerous problems due to this in a Javascript 
application where the behavior was similar.

-Alfred