From owner-svn-src-head@FreeBSD.ORG Fri Aug 22 23:13:55 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 1D4CBB36; Fri, 22 Aug 2014 23:13:55 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F293C3F04; Fri, 22 Aug 2014 23:13:54 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id s7MNDsV2024288; Fri, 22 Aug 2014 23:13:54 GMT (envelope-from delphij@FreeBSD.org) Received: (from delphij@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id s7MNDspp024281; Fri, 22 Aug 2014 23:13:54 GMT (envelope-from delphij@FreeBSD.org) Message-Id: <201408222313.s7MNDspp024281@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: delphij set sender to delphij@FreeBSD.org using -f From: Xin LI Date: Fri, 22 Aug 2014 23:13:54 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r270383 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Aug 2014 23:13:55 -0000 Author: delphij Date: Fri Aug 22 23:13:53 2014 New Revision: 270383 URL: http://svnweb.freebsd.org/changeset/base/270383 Log: 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 MFC after: 3 days Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h Directory Properties: head/sys/cddl/contrib/opensolaris/ (props changed) Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c Fri Aug 22 22:13:36 2014 (r270382) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dbuf.c Fri Aug 22 23:13:53 2014 (r270383) @@ -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: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c Fri Aug 22 22:13:36 2014 (r270382) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dnode.c Fri Aug 22 23:13:53 2014 (r270383) @@ -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: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h Fri Aug 22 22:13:36 2014 (r270382) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dbuf.h Fri Aug 22 23:13:53 2014 (r270383) @@ -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: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h Fri Aug 22 22:13:36 2014 (r270382) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h Fri Aug 22 23:13:53 2014 (r270383) @@ -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 */