Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Nov 2004 13:11:42 -0500
From:      John Baldwin <jhb@FreeBSD.org>
To:        David Schultz <das@FreeBSD.org>
Cc:        src-committers@FreeBSD.org
Subject:   Re: cvs commit: src/sys/vm vm_zeroidle.c
Message-ID:  <200411081311.42201.jhb@FreeBSD.org>
In-Reply-To: <20041106062955.GA1986@VARK.MIT.EDU>
References:  <200410311932.i9VJWvmo058193@repoman.freebsd.org> <200411011441.33067.jhb@FreeBSD.org> <20041106062955.GA1986@VARK.MIT.EDU>

next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 06 November 2004 01:29 am, David Schultz wrote:
> On Mon, Nov 01, 2004, John Baldwin wrote:
> > On Monday 01 November 2004 05:51 am, Alfred Perlstein wrote:
> > > * Alan Cox <alc@cs.rice.edu> [041031 20:53] wrote:
> > > > On Sun, Oct 31, 2004 at 07:13:17PM -0800, Alfred Perlstein wrote:
> > > > > * Alan Cox <alc@FreeBSD.org> [041031 11:33] wrote:
> > > > > > alc         2004-10-31 19:32:57 UTC
> > > > > >
> > > > > >   FreeBSD src repository
> > > > > >
> > > > > >   Modified files:
> > > > > >     sys/vm               vm_zeroidle.c
> > > > > >   Log:
> > > > > >   Introduce a Boolean variable wakeup_needed to avoid repeated,
> > > > > > unnecessary calls to wakeup() by vm_page_zero_idle_wakeup().
> > > > > >
> > > > > >   Revision  Changes    Path
> > > > > >   1.31      +9 -2      src/sys/vm/vm_zeroidle.c
> > > > >
> > > > > Why not switch to a cv?
> > > >
> > > > Calling cv_signal repeatedly would be no better than calling wakeup()
> > > > repeatedly.  Either way, a Boolean variable is desirable to prevent
> > > > unnecessary calls.
> > >
> > > Yah, I figured there would be something in the cv code to optimize
> > > the "no waiters" case.
> >
> > There is, though sometimes it might think there are waiters when there
> > actually aren't any.
>
> It doesn't look very optimized:
>
> 	void
> 	cv_signal(struct cv *cvp)
> 	{
>
> 		sleepq_lock(cvp);
> 		^^^^^^^^^^^^^^^^^
> 		if (cvp->cv_waiters > 0) {
> 			[...]
> 		} else
> 			sleepq_release(cvp);
>
> The mutex associated with the condition variable will be held on
> entry to both cv_wait() and cv_signal(), so why is the sleepqueue
> locked in the no waiters case?

It is no longer required to hold the mutex over cv_wait() and cv_signal().  I 
intentionally changed that so that you can do:

	lock()
	blah()
	unlock()
	cv_signal()

and reduce the number of context switches if you preempt in cv_signal().

-- 
John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org



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