Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 17 Jun 2009 18:55:29 +0000 (UTC)
From:      Sean Nicholas Barkas <snb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r194387 - head/sys/ufs/ufs
Message-ID:  <200906171855.n5HItT1G084993@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: snb
Date: Wed Jun 17 18:55:29 2009
New Revision: 194387
URL: http://svn.freebsd.org/changeset/base/194387

Log:
  Keep dirhash tailq locked throughout the entirety of ufsdirhash_destroy() to fix
  a potential race pointed out by pjd. Also use TAILQ_FOREACH_SAFE to iterate over
  dirhashes in ufsdirhash_lowmem(), so that we can continue iterating even after a
  dirhash is destroyed.
  
  Suggested by:	pjd
  Tested by:      pho
  Approved by:	dwmalone (mentor)

Modified:
  head/sys/ufs/ufs/ufs_dirhash.c

Modified: head/sys/ufs/ufs/ufs_dirhash.c
==============================================================================
--- head/sys/ufs/ufs/ufs_dirhash.c	Wed Jun 17 18:52:42 2009	(r194386)
+++ head/sys/ufs/ufs/ufs_dirhash.c	Wed Jun 17 18:55:29 2009	(r194387)
@@ -1191,16 +1191,14 @@ ufsdirhash_destroy(struct dirhash *dh)
 	mem = dh->dh_memreq;
 	dh->dh_memreq = 0;
 
-	/* Unlock everything, free the detached memory. */
+	/* Unlock dirhash and free the detached memory. */
 	ufsdirhash_release(dh);
-	DIRHASHLIST_UNLOCK();
 	for (i = 0; i < narrays; i++)
 		DIRHASH_BLKFREE(hash[i]);
 	free(hash, M_DIRHASH);
 	free(blkfree, M_DIRHASH);
 
 	/* Account for the returned memory. */
-	DIRHASHLIST_LOCK();
 	ufs_dirhashmem -= mem;
 
 	return (mem);
@@ -1247,7 +1245,7 @@ ufsdirhash_recycle(int wanted)
 static void
 ufsdirhash_lowmem()
 {
-	struct dirhash *dh;
+	struct dirhash *dh, *dh_temp;
 	int memfreed = 0;
 	/* XXX: this 10% may need to be adjusted */
 	int memwanted = ufs_dirhashmem / 10;
@@ -1259,8 +1257,7 @@ ufsdirhash_lowmem()
 	 * Delete dirhashes not used for more than ufs_dirhashreclaimage 
 	 * seconds. If we can't get a lock on the dirhash, it will be skipped.
 	 */
-	for (dh = TAILQ_FIRST(&ufsdirhash_list); dh != NULL; dh = 
-	    TAILQ_NEXT(dh, dh_list)) {
+	TAILQ_FOREACH_SAFE(dh, &ufsdirhash_list, dh_list, dh_temp) {
 		if (!sx_try_xlock(&dh->dh_lock))
 			continue;
 		if (time_second - dh->dh_lastused > ufs_dirhashreclaimage)
@@ -1275,11 +1272,14 @@ ufsdirhash_lowmem()
 	 * of the dirhash list. The ones closest to the head should be the 
 	 * oldest. 
 	 */
-	for (dh = TAILQ_FIRST(&ufsdirhash_list); memfreed < memwanted &&
-	    dh !=NULL; dh = TAILQ_NEXT(dh, dh_list)) {
-		if (!sx_try_xlock(&dh->dh_lock))
-			continue;
-		memfreed += ufsdirhash_destroy(dh);
+	if (memfreed < memwanted) {
+		TAILQ_FOREACH_SAFE(dh, &ufsdirhash_list, dh_list, dh_temp) {
+			if (!sx_try_xlock(&dh->dh_lock))
+				continue;
+			memfreed += ufsdirhash_destroy(dh);
+			if (memfreed >= memwanted)
+				break;
+		}
 	}
 	DIRHASHLIST_UNLOCK();
 }

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