| 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 | expand |
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); >
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); >
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
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);