From owner-freebsd-stable@FreeBSD.ORG Wed Dec 4 21:37:12 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 16940AB7; Wed, 4 Dec 2013 21:37:12 +0000 (UTC) Received: from mail-lb0-x236.google.com (mail-lb0-x236.google.com [IPv6:2a00:1450:4010:c04::236]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 35D4C1303; Wed, 4 Dec 2013 21:37:11 +0000 (UTC) Received: by mail-lb0-f182.google.com with SMTP id u14so9791193lbd.13 for ; Wed, 04 Dec 2013 13:37:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=PU3XHrea0YIi7Z+C0lo7H5VIWALD/hy+NbfD8Rm0Rxo=; b=C7xkOi3d0jMIbDso7Vj9BDNBVz0IHSYfXYx1idBKqD6/Rv77l3uzgFRPu78Q60OUXW GZH554rdAluHNoHJ5PEc16mUWBDJGibfpggzfrpHBRj1cqWL/u2OWiJ8GnP58AX7mLeM Mkfbw5OCRiWp+GQIMdj5+Mi0OmvqSG0ANFVeI1swTo2zu+eu29cIWEfY+rAXnnKsoTY5 PU3h4W+prNs0OphKi5KXVw1Rj0NyaxF/zW44ot24ukPYPgoWUj3qj6xUIxI6HShRqn9m F36QtzKTVsDY7ypEFy9Bwmsg3aAWHsSiBYvNhfaMihE8NB2oO4iqHQ7D2RIc2ulFp5ag sm3A== X-Received: by 10.112.17.39 with SMTP id l7mr1368125lbd.51.1386193028562; Wed, 04 Dec 2013 13:37:08 -0800 (PST) Received: from localhost ([178.150.115.244]) by mx.google.com with ESMTPSA id c15sm33940986lbq.11.2013.12.04.13.37.06 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 04 Dec 2013 13:37:07 -0800 (PST) Sender: Mikolaj Golub Date: Wed, 4 Dec 2013 23:37:05 +0200 From: Mikolaj Golub To: David Xu Subject: Re: Hast locking up under 9.2 Message-ID: <20131204213704.GD4005@gmail.com> References: <20131121203711.GA3736@gmail.com> <20131123215950.GA17292@gmail.com> <20131125083223.GE1398@garage.freebsd.pl> <20131125094111.GA22396@gmail.com> <529D40B5.5040605@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <529D40B5.5040605@freebsd.org> User-Agent: Mutt/1.5.22 (2013-10-16) 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: Wed, 04 Dec 2013 21:37:12 -0000 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? -- Mikolaj Golub