taskqueue_block

[Adrian Chadd]

Yes, and lets fix this. :)


-a

On 19 November 2013 06:32, Andriy Gapon <avg at freebsd.org> wrote:
>
> Forwarding this to the larger audience for a discussion.
>
> -------- Original Message --------
> Message-ID: <5287BDB9.10201 at FreeBSD.org>
> Date: Sat, 16 Nov 2013 20:47:21 +0200
> From: Andriy Gapon <avg at FreeBSD.org>
> Subject: taskqueue_block
>
>
>
> It seems that either I do not understand something about taskqueue_block code or
> it is a quite dangerous and abused API.  The fact that it is not properly
> documented does not help either.
>
> The commit message said:
>> Implement taskqueue_block() and taskqueue_unblock(). These functions allow the
>> owner of a queue to block and unblock execution of the tasks in the queue while
>> allowing tasks to continue to be added queue. Combining this with
>> taskqueue_drain() allows a queue to be safely disabled. The unblock function may
> [...]
>
> I indeed see this (anti?) pattern being used in the code.
> But what about the following case.   One thread calls taskqueue_block() and sets
> TQ_FLAGS_BLOCKED.  Another thread calls taskqueue_enqueue, this adds a task to
> the queue and sets ta_pending of the task to 1.  tq_enqueue is not called, so an
> actual queue runner is not called or waken up.   Then the first thread calls
> taskqueue_drain() on the task.  As far as I can see, the thread would then just
> wait forever because the task is pending and is not going to be executed.
>
> Additionally, it is impossible to reason about the taskqueue's state after
> taskqueue_block call, because the call just sets the flag and does not do any
> synchronization.  And as described above, it is not safe to call APIs that could
> allow the taskqueue or the task state to become known.
>
> I think that taskqueue_block() should wait on the currently active tasks to
> complete.  I don't think that this behavior could be optional.  I do see any
> reasonable and safe use for "non-blocking" taskqueue_block().
> taskqueue_drain() calls after taskqueue_block() must be removed.  The code
> should either use taskqueue_drain() or "blocking" taskqueue_block() depending on
> concrete circumstances.
>
> What do you think?
> Thank you.
> --
> Andriy Gapon
>
>
>
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"