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--