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>