Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 09 Nov 2016 07:02:36 -0800
From:      John Baldwin <jhb@freebsd.org>
To:        Hans Petter Selasky <hps@selasky.org>
Cc:        Oleksandr Tymoshenko <gonzo@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r308424 - head/sys/arm/broadcom/bcm2835
Message-ID:  <3214848.geWV8qu7rM@ralph.baldwin.cx>
In-Reply-To: <c7cd871d-9007-6de4-7063-2680e259713f@selasky.org>
References:  <201611071738.uA7HceYu045944@repo.freebsd.org> <680D84F2-65BF-48DD-8D11-311B1F65A634@freebsd.org> <c7cd871d-9007-6de4-7063-2680e259713f@selasky.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, November 07, 2016 08:32:17 PM Hans Petter Selasky wrote:
> On 11/07/16 20:23, Oleksandr Tymoshenko wrote:
> >
> >> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky <hps@selasky.org>=
 wrote:
> >>
> >> On 11/07/16 18:38, Oleksandr Tymoshenko wrote:
> >>> +=09=09bcm2835_audio_unlock(sc);
> >>> +=09=09cv_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=E2=80=99t matter. bcm2835_audio_xxx lock func=
tions are used to keep channel state consistent. The actual audio hw re=
programming happens in worker thread which only picks up latest state o=
f the virtual channel, there is no need to run every transaction in seq=
uence.
> >
>=20
> Hi,
>=20
> It is not about running in sequence, but that if the worker thread is=
=20
> not sleeping, but on the way to sleep, it will never get woken up unl=
ess=20
> you use proper locks here!

You do not have to hold locks across cv_signal/broadcast or wakeup.  Yo=
u do
have to hold them across the check determining whether you should sleep=
.
Take this simple example:

=09lock(&m);
=09while (should_sleep)
=09=09cv_wait(&cv, &m);
=09unlock(&m);

=09...


=09lock(&m);
=09should_sleep =3D true;
=09unlock(&m);
=09cv_signal(&cv);

A thread that locks 'm' after the 'unlock' but before the cv_signal wil=
l see
'should_sleep' as false and will not call cv_wait().  The cv_signal of =
course
might then wakeup a second thread prematurely, but that's why you shoul=
d
always use a while loop with cv wait operations (same is true with pthr=
ead
condvars btw).  On the other hand, doing the wakeup outside of the lock=

avoids preempting during the wakeup only to immediately block on the lo=
ck and
switch back to the thread that did the wakeup.

--=20
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3214848.geWV8qu7rM>