Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Aug 2014 13:03:13 +0000 (UTC)
From:      Xin LI <delphij@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r270809 - in stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys
Message-ID:  <201408291303.s7TD3DAg043804@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: delphij
Date: Fri Aug 29 13:03:13 2014
New Revision: 270809
URL: http://svnweb.freebsd.org/changeset/base/270809

Log:
  MFC r270383: MFV r270198:
  
  Instead of using timestamp in the AVL, use the memory address when
  comparing.
  
  Illumos issue:
      5095 panic when adding a duplicate dbuf to dn_dbufs

Modified:
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Fri Aug 29 12:48:38 2014	(r270808)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c	Fri Aug 29 13:03:13 2014	(r270809)
@@ -70,12 +70,6 @@ dbuf_cons(void *vdb, void *unused, int k
 	cv_init(&db->db_changed, NULL, CV_DEFAULT, NULL);
 	refcount_create(&db->db_holds);
 
-#if defined(illumos) || !defined(_KERNEL)
-	db->db_creation = gethrtime();
-#else
-	db->db_creation = cpu_ticks() ^ ((uint64_t)CPU_SEQID << 48);
-#endif
-
 	return (0);
 }
 
@@ -823,7 +817,7 @@ dbuf_free_range(dnode_t *dn, uint64_t st
 
 	db_search.db_level = 0;
 	db_search.db_blkid = start_blkid;
-	db_search.db_creation = 0;
+	db_search.db_state = DB_SEARCH;
 
 	mutex_enter(&dn->dn_dbufs_mtx);
 	if (start_blkid >= dn->dn_unlisted_l0_blkid) {

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Fri Aug 29 12:48:38 2014	(r270808)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c	Fri Aug 29 13:03:13 2014	(r270809)
@@ -69,33 +69,35 @@ dbuf_compare(const void *x1, const void 
 
 	if (d1->db_level < d2->db_level) {
 		return (-1);
-	} else if (d1->db_level > d2->db_level) {
+	}
+	if (d1->db_level > d2->db_level) {
 		return (1);
 	}
 
 	if (d1->db_blkid < d2->db_blkid) {
 		return (-1);
-	} else if (d1->db_blkid > d2->db_blkid) {
+	}
+	if (d1->db_blkid > d2->db_blkid) {
 		return (1);
 	}
 
-	/*
-	 * If a dbuf is being evicted while dn_dbufs_mutex is not held, we set
-	 * the db_state to DB_EVICTING but do not remove it from dn_dbufs. If
-	 * another thread creates a dbuf of the same blkid before the dbuf is
-	 * removed from dn_dbufs, we can reach a state where there are two
-	 * dbufs of the same blkid and level in db_dbufs. To maintain the avl
-	 * invariant that there cannot be duplicate items, we distinguish
-	 * between these two dbufs based on the time they were created.
-	 */
-	if (d1->db_creation < d2->db_creation) {
+	if (d1->db_state < d2->db_state) {
 		return (-1);
-	} else if (d1->db_creation > d2->db_creation) {
+	}
+	if (d1->db_state > d2->db_state) {
 		return (1);
-	} else {
-		ASSERT3P(d1, ==, d2);
-		return (0);
 	}
+
+	ASSERT3S(d1->db_state, !=, DB_SEARCH);
+	ASSERT3S(d2->db_state, !=, DB_SEARCH);
+
+	if ((uintptr_t)d1 < (uintptr_t)d2) {
+		return (-1);
+	}
+	if ((uintptr_t)d1 > (uintptr_t)d2) {
+		return (1);
+	}
+	return (0);
 }
 
 /* ARGSUSED */

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h	Fri Aug 29 12:48:38 2014	(r270808)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h	Fri Aug 29 13:03:13 2014	(r270809)
@@ -66,8 +66,13 @@ extern "C" {
  *		|			 |
  *		|			 |
  *		+--------> NOFILL -------+
+ *
+ * DB_SEARCH is an invalid state for a dbuf. It is used by dbuf_free_range
+ * to find all dbufs in a range of a dnode and must be less than any other
+ * dbuf_states_t (see comment on dn_dbufs in dnode.h).
  */
 typedef enum dbuf_states {
+	DB_SEARCH = -1,
 	DB_UNCACHED,
 	DB_FILL,
 	DB_NOFILL,
@@ -213,9 +218,6 @@ typedef struct dmu_buf_impl {
 	/* pointer to most recent dirty record for this buffer */
 	dbuf_dirty_record_t *db_last_dirty;
 
-	/* Creation time of dbuf (see comment in dbuf_compare). */
-	hrtime_t db_creation;
-
 	/*
 	 * Our link on the owner dnodes's dn_dbufs list.
 	 * Protected by its dn_dbufs_mtx.

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h	Fri Aug 29 12:48:38 2014	(r270808)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h	Fri Aug 29 13:03:13 2014	(r270809)
@@ -211,7 +211,18 @@ typedef struct dnode {
 	refcount_t dn_holds;
 
 	kmutex_t dn_dbufs_mtx;
-	avl_tree_t dn_dbufs;		/* descendent dbufs */
+	/*
+	 * Descendent dbufs, ordered by dbuf_compare. Note that dn_dbufs
+	 * can contain multiple dbufs of the same (level, blkid) when a
+	 * dbuf is marked DB_EVICTING without being removed from
+	 * dn_dbufs. To maintain the avl invariant that there cannot be
+	 * duplicate entries, we order the dbufs by an arbitrary value -
+	 * their address in memory. This means that dn_dbufs cannot be used to
+	 * directly look up a dbuf. Instead, callers must use avl_walk, have
+	 * a reference to the dbuf, or look up a non-existant node with
+	 * db_state = DB_SEARCH (see dbuf_free_range for an example).
+	 */
+	avl_tree_t dn_dbufs;
 
 	/* protected by dn_struct_rwlock */
 	struct dmu_buf_impl *dn_bonus;	/* bonus buffer dbuf */



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