[X,SRU] UBUNTU: SAUCE: (noup) Update zfs to 0.6.5.6-0ubuntu28
diff mbox series

Message ID 20190809102917.6783-1-colin.king@canonical.com
State New
Headers show
Series
  • [X,SRU] UBUNTU: SAUCE: (noup) Update zfs to 0.6.5.6-0ubuntu28
Related show

Commit Message

Colin Ian King Aug. 9, 2019, 10:29 a.m. UTC
From: Colin Ian King <colin.king@canonical.com>

BugLink: https://launchpad.net/bugs/1839521

Sync ZFS 0.6.5.6-0ubuntu28 to Xenial:

Fix shrinker deadlock with xattrs (LP: #1839521)

Upstream ZFS fix 31b6111fd92a ("Kill zp->z_xattr_parent to prevent pinning")
and ddae16a9cf0b ("xattr dir doesn't get purged during iput") fix a deadlock
in shrinker path when a xattr directory inode and its xattr inode are in the
same disposal list and the xattr dir inode is evicted before the xattr inode.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 zfs/META                    |  2 +-
 zfs/include/sys/zfs_znode.h |  1 -
 zfs/module/zfs/zfs_acl.c    | 59 ++++++++++++++++++---------------------------
 zfs/module/zfs/zfs_dir.c    |  3 ++-
 zfs/module/zfs/zfs_znode.c  | 29 +++-------------------
 5 files changed, 29 insertions(+), 65 deletions(-)

Comments

Kleber Sacilotto de Souza Aug. 9, 2019, 10:41 a.m. UTC | #1
On 09.08.19 12:29, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1839521
> 
> Sync ZFS 0.6.5.6-0ubuntu28 to Xenial:
> 
> Fix shrinker deadlock with xattrs (LP: #1839521)
> 
> Upstream ZFS fix 31b6111fd92a ("Kill zp->z_xattr_parent to prevent pinning")
> and ddae16a9cf0b ("xattr dir doesn't get purged during iput") fix a deadlock
> in shrinker path when a xattr directory inode and its xattr inode are in the
> same disposal list and the xattr dir inode is evicted before the xattr inode.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>  zfs/META                    |  2 +-
>  zfs/include/sys/zfs_znode.h |  1 -
>  zfs/module/zfs/zfs_acl.c    | 59 ++++++++++++++++++---------------------------
>  zfs/module/zfs/zfs_dir.c    |  3 ++-
>  zfs/module/zfs/zfs_znode.c  | 29 +++-------------------
>  5 files changed, 29 insertions(+), 65 deletions(-)
> 
> diff --git a/zfs/META b/zfs/META
> index 0ec36d1..2574764 100644
> --- a/zfs/META
> +++ b/zfs/META
> @@ -2,7 +2,7 @@ Meta:         1
>  Name:         zfs
>  Branch:       1.0
>  Version:      0.6.5.6
> -Release:      0ubuntu26
> +Release:      0ubuntu28
>  Release-Tags: relext
>  License:      CDDL
>  Author:       OpenZFS on Linux
> diff --git a/zfs/include/sys/zfs_znode.h b/zfs/include/sys/zfs_znode.h
> index c03bef5..abeb2ee 100644
> --- a/zfs/include/sys/zfs_znode.h
> +++ b/zfs/include/sys/zfs_znode.h
> @@ -209,7 +209,6 @@ typedef struct znode {
>  	zfs_acl_t	*z_acl_cached;	/* cached acl */
>  	krwlock_t	z_xattr_lock;	/* xattr data lock */
>  	nvlist_t	*z_xattr_cached; /* cached xattrs */
> -	struct znode	*z_xattr_parent; /* xattr parent znode */
>  	list_node_t	z_link_node;	/* all znodes in fs link */
>  	sa_handle_t	*z_sa_hdl;	/* handle to sa data */
>  	boolean_t	z_is_sa;	/* are we native sa? */
> diff --git a/zfs/module/zfs/zfs_acl.c b/zfs/module/zfs/zfs_acl.c
> index a208dea..bbb0193 100644
> --- a/zfs/module/zfs/zfs_acl.c
> +++ b/zfs/module/zfs/zfs_acl.c
> @@ -2473,53 +2473,33 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  {
>  	uint32_t	working_mode;
>  	int		error;
> -	boolean_t	check_privs;
> -	znode_t		*check_zp = zp;
> +	int		is_attr;
> +	boolean_t 	check_privs;
> +	znode_t		*xzp;
> +	znode_t 	*check_zp = zp;
>  	mode_t		needed_bits;
>  	uid_t		owner;
>  
> +	is_attr = ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode));
> +
>  	/*
>  	 * If attribute then validate against base file
>  	 */
> -	if ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode)) {
> +	if (is_attr) {
>  		uint64_t	parent;
>  
> -		rw_enter(&zp->z_xattr_lock, RW_READER);
> -		if (zp->z_xattr_parent) {
> -			check_zp = zp->z_xattr_parent;
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			/*
> -			 * Verify a lookup yields the same znode.
> -			 */
> -			ASSERT3S(sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent)), ==, 0);
> -			ASSERT3U(check_zp->z_id, ==, parent);
> -		} else {
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			error = sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent));
> -			if (error)
> -				return (error);
> +		if ((error = sa_lookup(zp->z_sa_hdl,
> +		    SA_ZPL_PARENT(ZTOZSB(zp)), &parent,
> +		    sizeof (parent))) != 0)
> +			return (error);
>  
> -			/*
> -			 * Cache the lookup on the parent file znode as
> -			 * zp->z_xattr_parent and hold a reference.  This
> -			 * effectively pins the parent in memory until all
> -			 * child xattr znodes have been destroyed and
> -			 * release their references in zfs_inode_destroy().
> -			 */
> -			error = zfs_zget(ZTOZSB(zp), parent, &check_zp);
> -			if (error)
> -				return (error);
> -
> -			rw_enter(&zp->z_xattr_lock, RW_WRITER);
> -			if (zp->z_xattr_parent == NULL)
> -				zp->z_xattr_parent = check_zp;
> -			rw_exit(&zp->z_xattr_lock);
> +		if ((error = zfs_zget(ZTOZSB(zp),
> +		    parent, &xzp)) != 0)	{
> +			return (error);
>  		}
>  
> +		check_zp = xzp;
> +
>  		/*
>  		 * fixup mode to map to xattr perms
>  		 */
> @@ -2561,11 +2541,15 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  
>  	if ((error = zfs_zaccess_common(check_zp, mode, &working_mode,
>  	    &check_privs, skipaclchk, cr)) == 0) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (secpolicy_vnode_access2(cr, ZTOI(zp), owner,
>  		    needed_bits, needed_bits));
>  	}
>  
>  	if (error && !check_privs) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (error);
>  	}
>  
> @@ -2626,6 +2610,9 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  		    needed_bits, needed_bits);
>  	}
>  
> +	if (is_attr)
> +		iput(ZTOI(xzp));
> +
>  	return (error);
>  }
>  
> diff --git a/zfs/module/zfs/zfs_dir.c b/zfs/module/zfs/zfs_dir.c
> index c1eadd0..b0c8e36 100644
> --- a/zfs/module/zfs/zfs_dir.c
> +++ b/zfs/module/zfs/zfs_dir.c
> @@ -593,7 +593,7 @@ zfs_purgedir(znode_t *dzp)
>  		if (error)
>  			skipped += 1;
>  		dmu_tx_commit(tx);
> -
> +		set_nlink(ZTOI(xzp), xzp->z_links);
>  		zfs_iput_async(ZTOI(xzp));
>  	}
>  	zap_cursor_fini(&zc);
> @@ -694,6 +694,7 @@ zfs_rmnode(znode_t *zp)
>  		mutex_enter(&xzp->z_lock);
>  		xzp->z_unlinked = B_TRUE;	/* mark xzp for deletion */
>  		xzp->z_links = 0;	/* no more links to it */
> +		set_nlink(ZTOI(xzp), 0); /* this will let iput purge us */
>  		VERIFY(0 == sa_update(xzp->z_sa_hdl, SA_ZPL_LINKS(zsb),
>  		    &xzp->z_links, sizeof (xzp->z_links), tx));
>  		mutex_exit(&xzp->z_lock);
> diff --git a/zfs/module/zfs/zfs_znode.c b/zfs/module/zfs/zfs_znode.c
> index 860354b..dc42d26 100644
> --- a/zfs/module/zfs/zfs_znode.c
> +++ b/zfs/module/zfs/zfs_znode.c
> @@ -120,7 +120,6 @@ zfs_znode_cache_constructor(void *buf, void *arg, int kmflags)
>  	zp->z_dirlocks = NULL;
>  	zp->z_acl_cached = NULL;
>  	zp->z_xattr_cached = NULL;
> -	zp->z_xattr_parent = NULL;
>  	zp->z_moved = 0;
>  	return (0);
>  }
> @@ -143,7 +142,6 @@ zfs_znode_cache_destructor(void *buf, void *arg)
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT(zp->z_acl_cached == NULL);
>  	ASSERT(zp->z_xattr_cached == NULL);
> -	ASSERT(zp->z_xattr_parent == NULL);
>  }
>  
>  static int
> @@ -435,11 +433,6 @@ zfs_inode_destroy(struct inode *ip)
>  		zp->z_xattr_cached = NULL;
>  	}
>  
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
> -
>  	kmem_cache_free(znode_cache, zp);
>  }
>  
> @@ -501,8 +494,7 @@ zfs_inode_set_ops(zfs_sb_t *zsb, struct inode *ip)
>   */
>  static znode_t *
>  zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
> -    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl,
> -    struct inode *dip)
> +    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl)
>  {
>  	znode_t	*zp;
>  	struct inode *ip;
> @@ -521,7 +513,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT3P(zp->z_acl_cached, ==, NULL);
>  	ASSERT3P(zp->z_xattr_cached, ==, NULL);
> -	ASSERT3P(zp->z_xattr_parent, ==, NULL);
>  	zp->z_moved = 0;
>  	zp->z_sa_hdl = NULL;
>  	zp->z_unlinked = 0;
> @@ -560,14 +551,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  
>  	zp->z_mode = mode;
>  
> -	/*
> -	 * xattr znodes hold a reference on their unique parent
> -	 */
> -	if (dip && zp->z_pflags & ZFS_XATTR) {
> -		igrab(dip);
> -		zp->z_xattr_parent = ITOZ(dip);
> -	}
> -
>  	ip->i_ino = obj;
>  	zfs_inode_update(zp);
>  	zfs_inode_set_ops(zsb, ip);
> @@ -913,8 +896,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
>  	VERIFY(sa_replace_all_by_template(sa_hdl, sa_attrs, cnt, tx) == 0);
>  
>  	if (!(flag & IS_ROOT_NODE)) {
> -		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl,
> -		    ZTOI(dzp));
> +		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl);
>  		VERIFY(*zpp != NULL);
>  		VERIFY(dzp != NULL);
>  	} else {
> @@ -1124,7 +1106,7 @@ again:
>  	 * bonus buffer.
>  	 */
>  	zp = zfs_znode_alloc(zsb, db, doi.doi_data_block_size,
> -	    doi.doi_bonus_type, obj_num, NULL, NULL);
> +	    doi.doi_bonus_type, obj_num, NULL);
>  	if (zp == NULL) {
>  		err = SET_ERROR(ENOENT);
>  	} else {
> @@ -1162,11 +1144,6 @@ zfs_rezget(znode_t *zp)
>  		nvlist_free(zp->z_xattr_cached);
>  		zp->z_xattr_cached = NULL;
>  	}
> -
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
>  	rw_exit(&zp->z_xattr_lock);
>  
>  	ASSERT(zp->z_sa_hdl == NULL);
>
Stefan Bader Aug. 12, 2019, 2:37 p.m. UTC | #2
On 09.08.19 12:29, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1839521
> 
> Sync ZFS 0.6.5.6-0ubuntu28 to Xenial:
> 
> Fix shrinker deadlock with xattrs (LP: #1839521)
> 
> Upstream ZFS fix 31b6111fd92a ("Kill zp->z_xattr_parent to prevent pinning")
> and ddae16a9cf0b ("xattr dir doesn't get purged during iput") fix a deadlock
> in shrinker path when a xattr directory inode and its xattr inode are in the
> same disposal list and the xattr dir inode is evicted before the xattr inode.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  zfs/META                    |  2 +-
>  zfs/include/sys/zfs_znode.h |  1 -
>  zfs/module/zfs/zfs_acl.c    | 59 ++++++++++++++++++---------------------------
>  zfs/module/zfs/zfs_dir.c    |  3 ++-
>  zfs/module/zfs/zfs_znode.c  | 29 +++-------------------
>  5 files changed, 29 insertions(+), 65 deletions(-)
> 
> diff --git a/zfs/META b/zfs/META
> index 0ec36d1..2574764 100644
> --- a/zfs/META
> +++ b/zfs/META
> @@ -2,7 +2,7 @@ Meta:         1
>  Name:         zfs
>  Branch:       1.0
>  Version:      0.6.5.6
> -Release:      0ubuntu26
> +Release:      0ubuntu28
>  Release-Tags: relext
>  License:      CDDL
>  Author:       OpenZFS on Linux
> diff --git a/zfs/include/sys/zfs_znode.h b/zfs/include/sys/zfs_znode.h
> index c03bef5..abeb2ee 100644
> --- a/zfs/include/sys/zfs_znode.h
> +++ b/zfs/include/sys/zfs_znode.h
> @@ -209,7 +209,6 @@ typedef struct znode {
>  	zfs_acl_t	*z_acl_cached;	/* cached acl */
>  	krwlock_t	z_xattr_lock;	/* xattr data lock */
>  	nvlist_t	*z_xattr_cached; /* cached xattrs */
> -	struct znode	*z_xattr_parent; /* xattr parent znode */
>  	list_node_t	z_link_node;	/* all znodes in fs link */
>  	sa_handle_t	*z_sa_hdl;	/* handle to sa data */
>  	boolean_t	z_is_sa;	/* are we native sa? */
> diff --git a/zfs/module/zfs/zfs_acl.c b/zfs/module/zfs/zfs_acl.c
> index a208dea..bbb0193 100644
> --- a/zfs/module/zfs/zfs_acl.c
> +++ b/zfs/module/zfs/zfs_acl.c
> @@ -2473,53 +2473,33 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  {
>  	uint32_t	working_mode;
>  	int		error;
> -	boolean_t	check_privs;
> -	znode_t		*check_zp = zp;
> +	int		is_attr;
> +	boolean_t 	check_privs;
> +	znode_t		*xzp;
> +	znode_t 	*check_zp = zp;
>  	mode_t		needed_bits;
>  	uid_t		owner;
>  
> +	is_attr = ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode));
> +
>  	/*
>  	 * If attribute then validate against base file
>  	 */
> -	if ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode)) {
> +	if (is_attr) {
>  		uint64_t	parent;
>  
> -		rw_enter(&zp->z_xattr_lock, RW_READER);
> -		if (zp->z_xattr_parent) {
> -			check_zp = zp->z_xattr_parent;
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			/*
> -			 * Verify a lookup yields the same znode.
> -			 */
> -			ASSERT3S(sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent)), ==, 0);
> -			ASSERT3U(check_zp->z_id, ==, parent);
> -		} else {
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			error = sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent));
> -			if (error)
> -				return (error);
> +		if ((error = sa_lookup(zp->z_sa_hdl,
> +		    SA_ZPL_PARENT(ZTOZSB(zp)), &parent,
> +		    sizeof (parent))) != 0)
> +			return (error);
>  
> -			/*
> -			 * Cache the lookup on the parent file znode as
> -			 * zp->z_xattr_parent and hold a reference.  This
> -			 * effectively pins the parent in memory until all
> -			 * child xattr znodes have been destroyed and
> -			 * release their references in zfs_inode_destroy().
> -			 */
> -			error = zfs_zget(ZTOZSB(zp), parent, &check_zp);
> -			if (error)
> -				return (error);
> -
> -			rw_enter(&zp->z_xattr_lock, RW_WRITER);
> -			if (zp->z_xattr_parent == NULL)
> -				zp->z_xattr_parent = check_zp;
> -			rw_exit(&zp->z_xattr_lock);
> +		if ((error = zfs_zget(ZTOZSB(zp),
> +		    parent, &xzp)) != 0)	{
> +			return (error);
>  		}
>  
> +		check_zp = xzp;
> +
>  		/*
>  		 * fixup mode to map to xattr perms
>  		 */
> @@ -2561,11 +2541,15 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  
>  	if ((error = zfs_zaccess_common(check_zp, mode, &working_mode,
>  	    &check_privs, skipaclchk, cr)) == 0) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (secpolicy_vnode_access2(cr, ZTOI(zp), owner,
>  		    needed_bits, needed_bits));
>  	}
>  
>  	if (error && !check_privs) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (error);
>  	}
>  
> @@ -2626,6 +2610,9 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  		    needed_bits, needed_bits);
>  	}
>  
> +	if (is_attr)
> +		iput(ZTOI(xzp));
> +
>  	return (error);
>  }
>  
> diff --git a/zfs/module/zfs/zfs_dir.c b/zfs/module/zfs/zfs_dir.c
> index c1eadd0..b0c8e36 100644
> --- a/zfs/module/zfs/zfs_dir.c
> +++ b/zfs/module/zfs/zfs_dir.c
> @@ -593,7 +593,7 @@ zfs_purgedir(znode_t *dzp)
>  		if (error)
>  			skipped += 1;
>  		dmu_tx_commit(tx);
> -
> +		set_nlink(ZTOI(xzp), xzp->z_links);
>  		zfs_iput_async(ZTOI(xzp));
>  	}
>  	zap_cursor_fini(&zc);
> @@ -694,6 +694,7 @@ zfs_rmnode(znode_t *zp)
>  		mutex_enter(&xzp->z_lock);
>  		xzp->z_unlinked = B_TRUE;	/* mark xzp for deletion */
>  		xzp->z_links = 0;	/* no more links to it */
> +		set_nlink(ZTOI(xzp), 0); /* this will let iput purge us */
>  		VERIFY(0 == sa_update(xzp->z_sa_hdl, SA_ZPL_LINKS(zsb),
>  		    &xzp->z_links, sizeof (xzp->z_links), tx));
>  		mutex_exit(&xzp->z_lock);
> diff --git a/zfs/module/zfs/zfs_znode.c b/zfs/module/zfs/zfs_znode.c
> index 860354b..dc42d26 100644
> --- a/zfs/module/zfs/zfs_znode.c
> +++ b/zfs/module/zfs/zfs_znode.c
> @@ -120,7 +120,6 @@ zfs_znode_cache_constructor(void *buf, void *arg, int kmflags)
>  	zp->z_dirlocks = NULL;
>  	zp->z_acl_cached = NULL;
>  	zp->z_xattr_cached = NULL;
> -	zp->z_xattr_parent = NULL;
>  	zp->z_moved = 0;
>  	return (0);
>  }
> @@ -143,7 +142,6 @@ zfs_znode_cache_destructor(void *buf, void *arg)
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT(zp->z_acl_cached == NULL);
>  	ASSERT(zp->z_xattr_cached == NULL);
> -	ASSERT(zp->z_xattr_parent == NULL);
>  }
>  
>  static int
> @@ -435,11 +433,6 @@ zfs_inode_destroy(struct inode *ip)
>  		zp->z_xattr_cached = NULL;
>  	}
>  
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
> -
>  	kmem_cache_free(znode_cache, zp);
>  }
>  
> @@ -501,8 +494,7 @@ zfs_inode_set_ops(zfs_sb_t *zsb, struct inode *ip)
>   */
>  static znode_t *
>  zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
> -    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl,
> -    struct inode *dip)
> +    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl)
>  {
>  	znode_t	*zp;
>  	struct inode *ip;
> @@ -521,7 +513,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT3P(zp->z_acl_cached, ==, NULL);
>  	ASSERT3P(zp->z_xattr_cached, ==, NULL);
> -	ASSERT3P(zp->z_xattr_parent, ==, NULL);
>  	zp->z_moved = 0;
>  	zp->z_sa_hdl = NULL;
>  	zp->z_unlinked = 0;
> @@ -560,14 +551,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  
>  	zp->z_mode = mode;
>  
> -	/*
> -	 * xattr znodes hold a reference on their unique parent
> -	 */
> -	if (dip && zp->z_pflags & ZFS_XATTR) {
> -		igrab(dip);
> -		zp->z_xattr_parent = ITOZ(dip);
> -	}
> -
>  	ip->i_ino = obj;
>  	zfs_inode_update(zp);
>  	zfs_inode_set_ops(zsb, ip);
> @@ -913,8 +896,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
>  	VERIFY(sa_replace_all_by_template(sa_hdl, sa_attrs, cnt, tx) == 0);
>  
>  	if (!(flag & IS_ROOT_NODE)) {
> -		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl,
> -		    ZTOI(dzp));
> +		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl);
>  		VERIFY(*zpp != NULL);
>  		VERIFY(dzp != NULL);
>  	} else {
> @@ -1124,7 +1106,7 @@ again:
>  	 * bonus buffer.
>  	 */
>  	zp = zfs_znode_alloc(zsb, db, doi.doi_data_block_size,
> -	    doi.doi_bonus_type, obj_num, NULL, NULL);
> +	    doi.doi_bonus_type, obj_num, NULL);
>  	if (zp == NULL) {
>  		err = SET_ERROR(ENOENT);
>  	} else {
> @@ -1162,11 +1144,6 @@ zfs_rezget(znode_t *zp)
>  		nvlist_free(zp->z_xattr_cached);
>  		zp->z_xattr_cached = NULL;
>  	}
> -
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
>  	rw_exit(&zp->z_xattr_lock);
>  
>  	ASSERT(zp->z_sa_hdl == NULL);
>
Khaled Elmously Aug. 12, 2019, 5:17 p.m. UTC | #3
On 2019-08-09 11:29:17 , Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1839521
> 
> Sync ZFS 0.6.5.6-0ubuntu28 to Xenial:
> 
> Fix shrinker deadlock with xattrs (LP: #1839521)
> 
> Upstream ZFS fix 31b6111fd92a ("Kill zp->z_xattr_parent to prevent pinning")
> and ddae16a9cf0b ("xattr dir doesn't get purged during iput") fix a deadlock
> in shrinker path when a xattr directory inode and its xattr inode are in the
> same disposal list and the xattr dir inode is evicted before the xattr inode.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  zfs/META                    |  2 +-
>  zfs/include/sys/zfs_znode.h |  1 -
>  zfs/module/zfs/zfs_acl.c    | 59 ++++++++++++++++++---------------------------
>  zfs/module/zfs/zfs_dir.c    |  3 ++-
>  zfs/module/zfs/zfs_znode.c  | 29 +++-------------------
>  5 files changed, 29 insertions(+), 65 deletions(-)
> 
> diff --git a/zfs/META b/zfs/META
> index 0ec36d1..2574764 100644
> --- a/zfs/META
> +++ b/zfs/META
> @@ -2,7 +2,7 @@ Meta:         1
>  Name:         zfs
>  Branch:       1.0
>  Version:      0.6.5.6
> -Release:      0ubuntu26
> +Release:      0ubuntu28
>  Release-Tags: relext
>  License:      CDDL
>  Author:       OpenZFS on Linux
> diff --git a/zfs/include/sys/zfs_znode.h b/zfs/include/sys/zfs_znode.h
> index c03bef5..abeb2ee 100644
> --- a/zfs/include/sys/zfs_znode.h
> +++ b/zfs/include/sys/zfs_znode.h
> @@ -209,7 +209,6 @@ typedef struct znode {
>  	zfs_acl_t	*z_acl_cached;	/* cached acl */
>  	krwlock_t	z_xattr_lock;	/* xattr data lock */
>  	nvlist_t	*z_xattr_cached; /* cached xattrs */
> -	struct znode	*z_xattr_parent; /* xattr parent znode */
>  	list_node_t	z_link_node;	/* all znodes in fs link */
>  	sa_handle_t	*z_sa_hdl;	/* handle to sa data */
>  	boolean_t	z_is_sa;	/* are we native sa? */
> diff --git a/zfs/module/zfs/zfs_acl.c b/zfs/module/zfs/zfs_acl.c
> index a208dea..bbb0193 100644
> --- a/zfs/module/zfs/zfs_acl.c
> +++ b/zfs/module/zfs/zfs_acl.c
> @@ -2473,53 +2473,33 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  {
>  	uint32_t	working_mode;
>  	int		error;
> -	boolean_t	check_privs;
> -	znode_t		*check_zp = zp;
> +	int		is_attr;
> +	boolean_t 	check_privs;
> +	znode_t		*xzp;
> +	znode_t 	*check_zp = zp;
>  	mode_t		needed_bits;
>  	uid_t		owner;
>  
> +	is_attr = ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode));
> +
>  	/*
>  	 * If attribute then validate against base file
>  	 */
> -	if ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode)) {
> +	if (is_attr) {
>  		uint64_t	parent;
>  
> -		rw_enter(&zp->z_xattr_lock, RW_READER);
> -		if (zp->z_xattr_parent) {
> -			check_zp = zp->z_xattr_parent;
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			/*
> -			 * Verify a lookup yields the same znode.
> -			 */
> -			ASSERT3S(sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent)), ==, 0);
> -			ASSERT3U(check_zp->z_id, ==, parent);
> -		} else {
> -			rw_exit(&zp->z_xattr_lock);
> -
> -			error = sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
> -			    ZTOZSB(zp)), &parent, sizeof (parent));
> -			if (error)
> -				return (error);
> +		if ((error = sa_lookup(zp->z_sa_hdl,
> +		    SA_ZPL_PARENT(ZTOZSB(zp)), &parent,
> +		    sizeof (parent))) != 0)
> +			return (error);
>  
> -			/*
> -			 * Cache the lookup on the parent file znode as
> -			 * zp->z_xattr_parent and hold a reference.  This
> -			 * effectively pins the parent in memory until all
> -			 * child xattr znodes have been destroyed and
> -			 * release their references in zfs_inode_destroy().
> -			 */
> -			error = zfs_zget(ZTOZSB(zp), parent, &check_zp);
> -			if (error)
> -				return (error);
> -
> -			rw_enter(&zp->z_xattr_lock, RW_WRITER);
> -			if (zp->z_xattr_parent == NULL)
> -				zp->z_xattr_parent = check_zp;
> -			rw_exit(&zp->z_xattr_lock);
> +		if ((error = zfs_zget(ZTOZSB(zp),
> +		    parent, &xzp)) != 0)	{
> +			return (error);
>  		}
>  
> +		check_zp = xzp;
> +
>  		/*
>  		 * fixup mode to map to xattr perms
>  		 */
> @@ -2561,11 +2541,15 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  
>  	if ((error = zfs_zaccess_common(check_zp, mode, &working_mode,
>  	    &check_privs, skipaclchk, cr)) == 0) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (secpolicy_vnode_access2(cr, ZTOI(zp), owner,
>  		    needed_bits, needed_bits));
>  	}
>  
>  	if (error && !check_privs) {
> +		if (is_attr)
> +			iput(ZTOI(xzp));
>  		return (error);
>  	}
>  
> @@ -2626,6 +2610,9 @@ zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
>  		    needed_bits, needed_bits);
>  	}
>  
> +	if (is_attr)
> +		iput(ZTOI(xzp));
> +
>  	return (error);
>  }
>  
> diff --git a/zfs/module/zfs/zfs_dir.c b/zfs/module/zfs/zfs_dir.c
> index c1eadd0..b0c8e36 100644
> --- a/zfs/module/zfs/zfs_dir.c
> +++ b/zfs/module/zfs/zfs_dir.c
> @@ -593,7 +593,7 @@ zfs_purgedir(znode_t *dzp)
>  		if (error)
>  			skipped += 1;
>  		dmu_tx_commit(tx);
> -
> +		set_nlink(ZTOI(xzp), xzp->z_links);
>  		zfs_iput_async(ZTOI(xzp));
>  	}
>  	zap_cursor_fini(&zc);
> @@ -694,6 +694,7 @@ zfs_rmnode(znode_t *zp)
>  		mutex_enter(&xzp->z_lock);
>  		xzp->z_unlinked = B_TRUE;	/* mark xzp for deletion */
>  		xzp->z_links = 0;	/* no more links to it */
> +		set_nlink(ZTOI(xzp), 0); /* this will let iput purge us */
>  		VERIFY(0 == sa_update(xzp->z_sa_hdl, SA_ZPL_LINKS(zsb),
>  		    &xzp->z_links, sizeof (xzp->z_links), tx));
>  		mutex_exit(&xzp->z_lock);
> diff --git a/zfs/module/zfs/zfs_znode.c b/zfs/module/zfs/zfs_znode.c
> index 860354b..dc42d26 100644
> --- a/zfs/module/zfs/zfs_znode.c
> +++ b/zfs/module/zfs/zfs_znode.c
> @@ -120,7 +120,6 @@ zfs_znode_cache_constructor(void *buf, void *arg, int kmflags)
>  	zp->z_dirlocks = NULL;
>  	zp->z_acl_cached = NULL;
>  	zp->z_xattr_cached = NULL;
> -	zp->z_xattr_parent = NULL;
>  	zp->z_moved = 0;
>  	return (0);
>  }
> @@ -143,7 +142,6 @@ zfs_znode_cache_destructor(void *buf, void *arg)
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT(zp->z_acl_cached == NULL);
>  	ASSERT(zp->z_xattr_cached == NULL);
> -	ASSERT(zp->z_xattr_parent == NULL);
>  }
>  
>  static int
> @@ -435,11 +433,6 @@ zfs_inode_destroy(struct inode *ip)
>  		zp->z_xattr_cached = NULL;
>  	}
>  
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
> -
>  	kmem_cache_free(znode_cache, zp);
>  }
>  
> @@ -501,8 +494,7 @@ zfs_inode_set_ops(zfs_sb_t *zsb, struct inode *ip)
>   */
>  static znode_t *
>  zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
> -    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl,
> -    struct inode *dip)
> +    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl)
>  {
>  	znode_t	*zp;
>  	struct inode *ip;
> @@ -521,7 +513,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  	ASSERT(zp->z_dirlocks == NULL);
>  	ASSERT3P(zp->z_acl_cached, ==, NULL);
>  	ASSERT3P(zp->z_xattr_cached, ==, NULL);
> -	ASSERT3P(zp->z_xattr_parent, ==, NULL);
>  	zp->z_moved = 0;
>  	zp->z_sa_hdl = NULL;
>  	zp->z_unlinked = 0;
> @@ -560,14 +551,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
>  
>  	zp->z_mode = mode;
>  
> -	/*
> -	 * xattr znodes hold a reference on their unique parent
> -	 */
> -	if (dip && zp->z_pflags & ZFS_XATTR) {
> -		igrab(dip);
> -		zp->z_xattr_parent = ITOZ(dip);
> -	}
> -
>  	ip->i_ino = obj;
>  	zfs_inode_update(zp);
>  	zfs_inode_set_ops(zsb, ip);
> @@ -913,8 +896,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
>  	VERIFY(sa_replace_all_by_template(sa_hdl, sa_attrs, cnt, tx) == 0);
>  
>  	if (!(flag & IS_ROOT_NODE)) {
> -		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl,
> -		    ZTOI(dzp));
> +		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl);
>  		VERIFY(*zpp != NULL);
>  		VERIFY(dzp != NULL);
>  	} else {
> @@ -1124,7 +1106,7 @@ again:
>  	 * bonus buffer.
>  	 */
>  	zp = zfs_znode_alloc(zsb, db, doi.doi_data_block_size,
> -	    doi.doi_bonus_type, obj_num, NULL, NULL);
> +	    doi.doi_bonus_type, obj_num, NULL);
>  	if (zp == NULL) {
>  		err = SET_ERROR(ENOENT);
>  	} else {
> @@ -1162,11 +1144,6 @@ zfs_rezget(znode_t *zp)
>  		nvlist_free(zp->z_xattr_cached);
>  		zp->z_xattr_cached = NULL;
>  	}
> -
> -	if (zp->z_xattr_parent) {
> -		zfs_iput_async(ZTOI(zp->z_xattr_parent));
> -		zp->z_xattr_parent = NULL;
> -	}
>  	rw_exit(&zp->z_xattr_lock);
>  
>  	ASSERT(zp->z_sa_hdl == NULL);
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Patch
diff mbox series

diff --git a/zfs/META b/zfs/META
index 0ec36d1..2574764 100644
--- a/zfs/META
+++ b/zfs/META
@@ -2,7 +2,7 @@  Meta:         1
 Name:         zfs
 Branch:       1.0
 Version:      0.6.5.6
-Release:      0ubuntu26
+Release:      0ubuntu28
 Release-Tags: relext
 License:      CDDL
 Author:       OpenZFS on Linux
diff --git a/zfs/include/sys/zfs_znode.h b/zfs/include/sys/zfs_znode.h
index c03bef5..abeb2ee 100644
--- a/zfs/include/sys/zfs_znode.h
+++ b/zfs/include/sys/zfs_znode.h
@@ -209,7 +209,6 @@  typedef struct znode {
 	zfs_acl_t	*z_acl_cached;	/* cached acl */
 	krwlock_t	z_xattr_lock;	/* xattr data lock */
 	nvlist_t	*z_xattr_cached; /* cached xattrs */
-	struct znode	*z_xattr_parent; /* xattr parent znode */
 	list_node_t	z_link_node;	/* all znodes in fs link */
 	sa_handle_t	*z_sa_hdl;	/* handle to sa data */
 	boolean_t	z_is_sa;	/* are we native sa? */
diff --git a/zfs/module/zfs/zfs_acl.c b/zfs/module/zfs/zfs_acl.c
index a208dea..bbb0193 100644
--- a/zfs/module/zfs/zfs_acl.c
+++ b/zfs/module/zfs/zfs_acl.c
@@ -2473,53 +2473,33 @@  zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
 {
 	uint32_t	working_mode;
 	int		error;
-	boolean_t	check_privs;
-	znode_t		*check_zp = zp;
+	int		is_attr;
+	boolean_t 	check_privs;
+	znode_t		*xzp;
+	znode_t 	*check_zp = zp;
 	mode_t		needed_bits;
 	uid_t		owner;
 
+	is_attr = ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode));
+
 	/*
 	 * If attribute then validate against base file
 	 */
-	if ((zp->z_pflags & ZFS_XATTR) && S_ISDIR(ZTOI(zp)->i_mode)) {
+	if (is_attr) {
 		uint64_t	parent;
 
-		rw_enter(&zp->z_xattr_lock, RW_READER);
-		if (zp->z_xattr_parent) {
-			check_zp = zp->z_xattr_parent;
-			rw_exit(&zp->z_xattr_lock);
-
-			/*
-			 * Verify a lookup yields the same znode.
-			 */
-			ASSERT3S(sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
-			    ZTOZSB(zp)), &parent, sizeof (parent)), ==, 0);
-			ASSERT3U(check_zp->z_id, ==, parent);
-		} else {
-			rw_exit(&zp->z_xattr_lock);
-
-			error = sa_lookup(zp->z_sa_hdl, SA_ZPL_PARENT(
-			    ZTOZSB(zp)), &parent, sizeof (parent));
-			if (error)
-				return (error);
+		if ((error = sa_lookup(zp->z_sa_hdl,
+		    SA_ZPL_PARENT(ZTOZSB(zp)), &parent,
+		    sizeof (parent))) != 0)
+			return (error);
 
-			/*
-			 * Cache the lookup on the parent file znode as
-			 * zp->z_xattr_parent and hold a reference.  This
-			 * effectively pins the parent in memory until all
-			 * child xattr znodes have been destroyed and
-			 * release their references in zfs_inode_destroy().
-			 */
-			error = zfs_zget(ZTOZSB(zp), parent, &check_zp);
-			if (error)
-				return (error);
-
-			rw_enter(&zp->z_xattr_lock, RW_WRITER);
-			if (zp->z_xattr_parent == NULL)
-				zp->z_xattr_parent = check_zp;
-			rw_exit(&zp->z_xattr_lock);
+		if ((error = zfs_zget(ZTOZSB(zp),
+		    parent, &xzp)) != 0)	{
+			return (error);
 		}
 
+		check_zp = xzp;
+
 		/*
 		 * fixup mode to map to xattr perms
 		 */
@@ -2561,11 +2541,15 @@  zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
 
 	if ((error = zfs_zaccess_common(check_zp, mode, &working_mode,
 	    &check_privs, skipaclchk, cr)) == 0) {
+		if (is_attr)
+			iput(ZTOI(xzp));
 		return (secpolicy_vnode_access2(cr, ZTOI(zp), owner,
 		    needed_bits, needed_bits));
 	}
 
 	if (error && !check_privs) {
+		if (is_attr)
+			iput(ZTOI(xzp));
 		return (error);
 	}
 
@@ -2626,6 +2610,9 @@  zfs_zaccess(znode_t *zp, int mode, int flags, boolean_t skipaclchk, cred_t *cr)
 		    needed_bits, needed_bits);
 	}
 
+	if (is_attr)
+		iput(ZTOI(xzp));
+
 	return (error);
 }
 
diff --git a/zfs/module/zfs/zfs_dir.c b/zfs/module/zfs/zfs_dir.c
index c1eadd0..b0c8e36 100644
--- a/zfs/module/zfs/zfs_dir.c
+++ b/zfs/module/zfs/zfs_dir.c
@@ -593,7 +593,7 @@  zfs_purgedir(znode_t *dzp)
 		if (error)
 			skipped += 1;
 		dmu_tx_commit(tx);
-
+		set_nlink(ZTOI(xzp), xzp->z_links);
 		zfs_iput_async(ZTOI(xzp));
 	}
 	zap_cursor_fini(&zc);
@@ -694,6 +694,7 @@  zfs_rmnode(znode_t *zp)
 		mutex_enter(&xzp->z_lock);
 		xzp->z_unlinked = B_TRUE;	/* mark xzp for deletion */
 		xzp->z_links = 0;	/* no more links to it */
+		set_nlink(ZTOI(xzp), 0); /* this will let iput purge us */
 		VERIFY(0 == sa_update(xzp->z_sa_hdl, SA_ZPL_LINKS(zsb),
 		    &xzp->z_links, sizeof (xzp->z_links), tx));
 		mutex_exit(&xzp->z_lock);
diff --git a/zfs/module/zfs/zfs_znode.c b/zfs/module/zfs/zfs_znode.c
index 860354b..dc42d26 100644
--- a/zfs/module/zfs/zfs_znode.c
+++ b/zfs/module/zfs/zfs_znode.c
@@ -120,7 +120,6 @@  zfs_znode_cache_constructor(void *buf, void *arg, int kmflags)
 	zp->z_dirlocks = NULL;
 	zp->z_acl_cached = NULL;
 	zp->z_xattr_cached = NULL;
-	zp->z_xattr_parent = NULL;
 	zp->z_moved = 0;
 	return (0);
 }
@@ -143,7 +142,6 @@  zfs_znode_cache_destructor(void *buf, void *arg)
 	ASSERT(zp->z_dirlocks == NULL);
 	ASSERT(zp->z_acl_cached == NULL);
 	ASSERT(zp->z_xattr_cached == NULL);
-	ASSERT(zp->z_xattr_parent == NULL);
 }
 
 static int
@@ -435,11 +433,6 @@  zfs_inode_destroy(struct inode *ip)
 		zp->z_xattr_cached = NULL;
 	}
 
-	if (zp->z_xattr_parent) {
-		zfs_iput_async(ZTOI(zp->z_xattr_parent));
-		zp->z_xattr_parent = NULL;
-	}
-
 	kmem_cache_free(znode_cache, zp);
 }
 
@@ -501,8 +494,7 @@  zfs_inode_set_ops(zfs_sb_t *zsb, struct inode *ip)
  */
 static znode_t *
 zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
-    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl,
-    struct inode *dip)
+    dmu_object_type_t obj_type, uint64_t obj, sa_handle_t *hdl)
 {
 	znode_t	*zp;
 	struct inode *ip;
@@ -521,7 +513,6 @@  zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
 	ASSERT(zp->z_dirlocks == NULL);
 	ASSERT3P(zp->z_acl_cached, ==, NULL);
 	ASSERT3P(zp->z_xattr_cached, ==, NULL);
-	ASSERT3P(zp->z_xattr_parent, ==, NULL);
 	zp->z_moved = 0;
 	zp->z_sa_hdl = NULL;
 	zp->z_unlinked = 0;
@@ -560,14 +551,6 @@  zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
 
 	zp->z_mode = mode;
 
-	/*
-	 * xattr znodes hold a reference on their unique parent
-	 */
-	if (dip && zp->z_pflags & ZFS_XATTR) {
-		igrab(dip);
-		zp->z_xattr_parent = ITOZ(dip);
-	}
-
 	ip->i_ino = obj;
 	zfs_inode_update(zp);
 	zfs_inode_set_ops(zsb, ip);
@@ -913,8 +896,7 @@  zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
 	VERIFY(sa_replace_all_by_template(sa_hdl, sa_attrs, cnt, tx) == 0);
 
 	if (!(flag & IS_ROOT_NODE)) {
-		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl,
-		    ZTOI(dzp));
+		*zpp = zfs_znode_alloc(zsb, db, 0, obj_type, obj, sa_hdl);
 		VERIFY(*zpp != NULL);
 		VERIFY(dzp != NULL);
 	} else {
@@ -1124,7 +1106,7 @@  again:
 	 * bonus buffer.
 	 */
 	zp = zfs_znode_alloc(zsb, db, doi.doi_data_block_size,
-	    doi.doi_bonus_type, obj_num, NULL, NULL);
+	    doi.doi_bonus_type, obj_num, NULL);
 	if (zp == NULL) {
 		err = SET_ERROR(ENOENT);
 	} else {
@@ -1162,11 +1144,6 @@  zfs_rezget(znode_t *zp)
 		nvlist_free(zp->z_xattr_cached);
 		zp->z_xattr_cached = NULL;
 	}
-
-	if (zp->z_xattr_parent) {
-		zfs_iput_async(ZTOI(zp->z_xattr_parent));
-		zp->z_xattr_parent = NULL;
-	}
 	rw_exit(&zp->z_xattr_lock);
 
 	ASSERT(zp->z_sa_hdl == NULL);