From owner-freebsd-stable@FreeBSD.ORG Thu Dec 5 02:59:22 2013 Return-Path: Delivered-To: freebsd-stable@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 C5EDAEEF; Thu, 5 Dec 2013 02:59:22 +0000 (UTC) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 94FEC15D1; Thu, 5 Dec 2013 02:59:22 +0000 (UTC) Received: from xyf.my.dom (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.7/8.14.7) with ESMTP id rB52xKko082353; Thu, 5 Dec 2013 02:59:20 GMT (envelope-from davidxu@freebsd.org) Message-ID: <529FEC15.3080509@freebsd.org> Date: Thu, 05 Dec 2013 10:59:33 +0800 From: David Xu User-Agent: Mozilla/5.0 (X11; FreeBSD i386; rv:17.0) Gecko/20130416 Thunderbird/17.0.5 MIME-Version: 1.0 To: Mikolaj Golub Subject: Re: Hast locking up under 9.2 References: <20131121203711.GA3736@gmail.com> <20131123215950.GA17292@gmail.com> <20131125083223.GE1398@garage.freebsd.pl> <20131125094111.GA22396@gmail.com> <529D40B5.5040605@freebsd.org> <20131204213704.GD4005@gmail.com> In-Reply-To: <20131204213704.GD4005@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-stable@freebsd.org, Pete French X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Dec 2013 02:59:22 -0000 On 2013/12/05 05:37, Mikolaj Golub wrote: > On Tue, Dec 03, 2013 at 10:23:49AM +0800, David Xu wrote: >> Also I found the following code in hastd: >> >> #define QUEUE_INSERT1(hio, name, ncomp) do { \ >> bool _wakeup; \ >> \ >> mtx_lock(&hio_##name##_list_lock[(ncomp)]); \ >> _wakeup = TAILQ_EMPTY(&hio_##name##_list[(ncomp)]); \ >> TAILQ_INSERT_TAIL(&hio_##name##_list[(ncomp)], (hio), \ >> hio_next[(ncomp)]); \ >> hio_##name##_list_size[(ncomp)]++; \ >> mtx_unlock(&hio_##name##_list_lock[ncomp]); \ >> if (_wakeup) \ >> cv_broadcast(&hio_##name##_list_cond[(ncomp)]); >> >> Our thread library does optimize the condition variable's wait/signal >> lock contention, we had implemented wait queue morphying, so it is not >> needed to unlock mutex first, then call cv_broadcast, such code really >> increases spurious wakeups, and can be worse in some cases: >> for example, before cv_broadcast is called, the producer thread is >> preempted, and a consumer thread removes elements in the queue and >> sleeps again, and the producer thread is scheduled again and it blindly >> calls cv_broadcast, and a consumer thread then find nothing in the >> queue, and sleeps again. >> >> I think following code is enough for our thread library, and works >> better. >> >> #define QUEUE_INSERT1(hio, name, ncomp) do { \ >> mtx_lock(&hio_##name##_list_lock[(ncomp)]); \ >> if (TAILQ_EMPTY(&hio_##name##_list[(ncomp)])) \ >> cv_broadcast(&hio_##name##_list_cond[(ncomp)]); \ >> TAILQ_INSERT_TAIL(&hio_##name##_list[(ncomp)], (hio), \ >> hio_next[(ncomp)]); \ >> hio_##name##_list_size[(ncomp)]++; \ >> mtx_unlock(&hio_##name##_list_lock[ncomp]); \ >> } while (0) > > Ok. I have updated all three macros we have accordingly: > > http://people.freebsd.org/~trociny/patches/hast.queue_insert_wakeup.1.patch > > Pawel, what do you think? > No problem. ;-)