Date: Wed, 3 Jun 2009 23:06:52 +0200 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Sean Nicholas Barkas <snb@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r193375 - head/sys/ufs/ufs Message-ID: <20090603210652.GD3821@garage.freebsd.pl> In-Reply-To: <200906030944.n539iM2K045164@svn.freebsd.org> References: <200906030944.n539iM2K045164@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--O5XBE6gyVG5Rl6Rj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 03, 2009 at 09:44:22AM +0000, Sean Nicholas Barkas wrote: > Author: snb > Date: Wed Jun 3 09:44:22 2009 > New Revision: 193375 > URL: http://svn.freebsd.org/changeset/base/193375 >=20 > Log: > Add vm_lowmem event handler for dirhash. This will cause dirhashes to be > deleted when the system is low on memory. This ought to allow an increa= se to > vfs.ufs.dirhash_maxmem on machines that have lots of memory, without > degrading performance by having too much memory reserved for dirhash wh= en > other things need it. The default value for dirhash_maxmem is being kep= t at > 2MB for now, though. > =20 > This work was mostly done during the 2008 Google Summer of Code. > =20 > Approved by: dwmalone (mentor), re > MFC after: 3 months [...] > +static int > +ufsdirhash_destroy(struct dirhash *dh) > +{ [...] > + /* Remove it from the list and detach its memory. */ > + TAILQ_REMOVE(&ufsdirhash_list, dh, dh_list); [...] > +static void > +ufsdirhash_lowmem() > +{ [...] > + /*=20 > + * Delete dirhashes not used for more than ufs_dirhashreclaimage=20 > + * seconds. If we can't get a lock on the dirhash, it will be skipped. > + */ > + for (dh =3D TAILQ_FIRST(&ufsdirhash_list); dh !=3D NULL; dh =3D=20 > + TAILQ_NEXT(dh, dh_list)) { > + if (!sx_try_xlock(&dh->dh_lock)) > + continue; > + if (time_second - dh->dh_lastused > ufs_dirhashreclaimage) > + memfreed +=3D ufsdirhash_destroy(dh); > + /* Unlock if we didn't delete the dirhash */ > + else > + ufsdirhash_release(dh); > + } > + > + /*=20 > + * If not enough memory was freed, keep deleting hashes from the head= =20 > + * of the dirhash list. The ones closest to the head should be the=20 > + * oldest.=20 > + */ > + for (dh =3D TAILQ_FIRST(&ufsdirhash_list); memfreed < memwanted && > + dh !=3DNULL; dh =3D TAILQ_NEXT(dh, dh_list)) { > + if (!sx_try_xlock(&dh->dh_lock)) > + continue; > + memfreed +=3D ufsdirhash_destroy(dh); > + } > + DIRHASHLIST_UNLOCK(); > +} I don't see how that works. If you remove dh from the tailq in ufsdirhash_destroy(), you can't do 'dh =3D TAILQ_NEXT(dh, dh_list)' at the end of the loop. You should use TAILQ_FOREACH_SAFE(3). In the second case you also need to move this extra check into the loop, probably. In addition you drop DIRHASHLIST lock in ufsdirhash_destroy() during the loop. Can't the tailq be modified from elsewhere? Or even from parallel call to ufsdirhash_lowmem() (I don't think we serialize those)? --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --O5XBE6gyVG5Rl6Rj Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFKJuXrForvXbEpPzQRAmzyAKD0A66qjlntobqzZuTayARF3hjSPgCgnAIh 8xMU3V+3afLvKTb3KThI2GY= =LIJ2 -----END PGP SIGNATURE----- --O5XBE6gyVG5Rl6Rj--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090603210652.GD3821>