Subject: Re: CVS commit: src/sys/dev/raidframe
To: Greg Oster <oster@netbsd.org>
From: Bill Studenmund <wrstuden@netbsd.org>
List: source-changes
Date: 01/23/2004 10:19:18
--uAKRQypu60I7Lcqm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Jan 23, 2004 at 01:57:08AM +0000, Greg Oster wrote:
>=20
> Module Name:	src
> Committed By:	oster
> Date:		Fri Jan 23 01:57:08 UTC 2004
>=20
> Modified Files:
> 	src/sys/dev/raidframe: rf_stripelocks.c
>=20
> Log Message:
> Always ask for a new RF_StripeLockDesc_t "just in case", and then
> give it back if we don't need it.  If we don't allocate it before
> we take our lock, LOCKDEBUG (rightfully) complains that we're trying
> to grab something from the pool with PR_WAITOK.  This code (and the
> PR_WAITOK in particular) really needs to be revisited at some point.

How often do we go through this path? Would it make sense to have a global=
=20
single cache? Have a simplelock and a global RF_StripeLockDesc_t *. On=20
entry, if there's one in the cache, take it. Otherwise do the allocation=20
you have now. Do what you do now. Then grab the cache lock. If there's not=
=20
one in the cache, put this one in. Otherwise, free it.

I think the cache lock twiddling and comparison against NULL would be less=
=20
work than the memory allocation. :-)

This kind of change would trade off usually having one RF_StripeLockDesc_t=
=20
lying around for not having to allocate then deallocate it each pass.

With __predict_true() and __predict_false() (are those the names?), the=20
code can be efficient in the common case.

You're the only one, though, who knows if it'd be worth it. :-)

Take care,

Bill

--uAKRQypu60I7Lcqm
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFAEWWmWz+3JHUci9cRAtd5AJ9IV2IhaTJamTlX9fwookF6EHVv0QCaA9Rq
oEZ8FvNPvgGNF6iljf3Xd4s=
=jQ9T
-----END PGP SIGNATURE-----

--uAKRQypu60I7Lcqm--