From owner-freebsd-hackers@FreeBSD.ORG Tue Nov 19 14:33:19 2013 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id CE331D71; Tue, 19 Nov 2013 14:33:19 +0000 (UTC) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 90B8A240A; Tue, 19 Nov 2013 14:33:18 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id QAA07017; Tue, 19 Nov 2013 16:33:10 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1VimMX-000Kce-Ts; Tue, 19 Nov 2013 16:33:09 +0200 Message-ID: <528B7681.6090806@FreeBSD.org> Date: Tue, 19 Nov 2013 16:32:33 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: freebsd-hackers@FreeBSD.org Subject: Fwd: taskqueue_block References: <5287BDB9.10201@FreeBSD.org> In-Reply-To: <5287BDB9.10201@FreeBSD.org> X-Enigmail-Version: 1.6 X-Forwarded-Message-Id: <5287BDB9.10201@FreeBSD.org> Content-Type: text/plain; charset=x-viet-vps Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Nov 2013 14:33:19 -0000 Forwarding this to the larger audience for a discussion. -------- Original Message -------- Message-ID: <5287BDB9.10201@FreeBSD.org> Date: Sat, 16 Nov 2013 20:47:21 +0200 From: Andriy Gapon 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