From owner-svn-src-all@freebsd.org Mon Nov 7 20:31:07 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DF862C35357; Mon, 7 Nov 2016 20:31:07 +0000 (UTC) (envelope-from gonzo@id.bluezbox.com) Received: from id.bluezbox.com (id.bluezbox.com [45.55.20.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B999AF4B; Mon, 7 Nov 2016 20:31:07 +0000 (UTC) (envelope-from gonzo@id.bluezbox.com) Received: from [208.184.220.60] (helo=[10.112.202.233]) by id.bluezbox.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87 (FreeBSD)) (envelope-from ) id 1c3qZN-000Mta-Pf; Mon, 07 Nov 2016 12:31:06 -0800 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.1 \(3251\)) Subject: Re: svn commit: r308424 - head/sys/arm/broadcom/bcm2835 From: Oleksandr Tymoshenko In-Reply-To: <2b03afc6-9bfa-1124-7b70-2532751bfed9@selasky.org> Date: Mon, 7 Nov 2016 12:30:35 -0800 Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <201611071738.uA7HceYu045944@repo.freebsd.org> <680D84F2-65BF-48DD-8D11-311B1F65A634@freebsd.org> <2b03afc6-9bfa-1124-7b70-2532751bfed9@selasky.org> To: Hans Petter Selasky X-Mailer: Apple Mail (2.3251) Sender: gonzo@id.bluezbox.com X-Spam-Level: -- X-Spam-Report: Spam detection software, running on the system "id.bluezbox.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see The administrator of that system for details. Content preview: > On Nov 7, 2016, at 11:51 AM, Hans Petter Selasky wrote: > > On 11/07/16 20:32, Hans Petter Selasky wrote: >> On 11/07/16 20:23, Oleksandr Tymoshenko wrote: >>> >>>> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky >>>> wrote: >>>> >>>> On 11/07/16 18:38, Oleksandr Tymoshenko wrote: >>>>> + bcm2835_audio_unlock(sc); >>>>> + cv_signal(&sc->worker_cv); >>>> >>>> >>>> Shouldn't cv_signal() be done locked, so that you don't loose any >>>> transactions? CV's only wakeup the treads that are sleeping right >>>> there and then. >>> >>> Hi Hans, >>> >>> In this case it doesn’t matter. bcm2835_audio_xxx lock functions are >>> used to keep channel state consistent. The actual audio hw >>> reprogramming happens in worker thread which only picks up latest >>> state of the virtual channel, there is no need to run every >>> transaction in sequence. >>> >> >> Hi, >> >> It is not about running in sequence, but that if the worker thread is >> not sleeping, but on the way to sleep, it will never get woken up unless >> you use proper locks here! >> >> --HPS > > Hi, > > Also the teardown sequence for the worker thread looks a bit broken, that it doesn't wait for the thread to exit. > > I've made a patch, attached which I think is the right way to do it. > > Try opening and closing /dev/dsp in a loop with some DSP ioctls and see what happens. [...] Content analysis details: (-2.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP 0.0 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Nov 2016 20:31:08 -0000 > On Nov 7, 2016, at 11:51 AM, Hans Petter Selasky = wrote: >=20 > On 11/07/16 20:32, Hans Petter Selasky wrote: >> On 11/07/16 20:23, Oleksandr Tymoshenko wrote: >>>=20 >>>> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky >>>> wrote: >>>>=20 >>>> On 11/07/16 18:38, Oleksandr Tymoshenko wrote: >>>>> + bcm2835_audio_unlock(sc); >>>>> + cv_signal(&sc->worker_cv); >>>>=20 >>>>=20 >>>> Shouldn't cv_signal() be done locked, so that you don't loose any >>>> transactions? CV's only wakeup the treads that are sleeping right >>>> there and then. >>>=20 >>> Hi Hans, >>>=20 >>> In this case it doesn=E2=80=99t matter. bcm2835_audio_xxx lock = functions are >>> used to keep channel state consistent. The actual audio hw >>> reprogramming happens in worker thread which only picks up latest >>> state of the virtual channel, there is no need to run every >>> transaction in sequence. >>>=20 >>=20 >> Hi, >>=20 >> It is not about running in sequence, but that if the worker thread is >> not sleeping, but on the way to sleep, it will never get woken up = unless >> you use proper locks here! >>=20 >> --HPS >=20 > Hi, >=20 > Also the teardown sequence for the worker thread looks a bit broken, = that it doesn't wait for the thread to exit. >=20 > I've made a patch, attached which I think is the right way to do it. >=20 > Try opening and closing /dev/dsp in a loop with some DSP ioctls and = see what happens. Thanks for patch Hans. Looks good to me. I=E2=80=99ll test and commit = it.=20=