d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Kill zp->z_xattr_parent to prevent pinning

Browse Source 
zp->z_xattr_parent will pin the parent. This will cause huge issue
when unlink a file with xattr. Because the unlinked file is pinned, it
will never get purged immediately. And because of that, the xattr
stuff will never be marked as unlinked. So the whole unlinked stuff
will stay there until shrink cache or umount.

This change partially reverts e89260a.  This is safe because only the
zp->z_xattr_parent optimization is removed, zpl_xattr_security_init()
is still called from the zpl outside the inode lock.

Signed-off-by: Chunwei Chen <david.chen@osnexus.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chris Dunlop <chris@onthe.net.au>
Issue #4359
Issue #3508
Issue #4413
Issue #4827
This commit is contained in:
Chunwei Chen 2016-07-05 17:24:36 -07:00 committed by Brian Behlendorf
parent ddae16a9cf
commit 31b6111fd9
3 changed files with 26 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/sys/zfs_znode.h
View File

@ -197,7 +197,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? */

59
module/zfs/zfs_acl.c
View File

@ -2471,53 +2471,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);
if ((error = sa_lookup(zp->z_sa_hdl,
SA_ZPL_PARENT(ZTOZSB(zp)), &parent,
sizeof (parent))) != 0)
return (error);
/*
* 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);
/*
* 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
*/
@ -2559,11 +2539,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);
}
@ -2624,6 +2608,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);
}

29
module/zfs/zfs_znode.c
View File

@ -119,7 +119,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);
}
@ -141,7 +140,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
@ -433,11 +431,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);
}
@ -582,8 +575,7 @@ zfs_inode_update(znode_t *zp)
*/
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;
@ -603,7 +595,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;
@ -645,14 +636,6 @@ zfs_znode_alloc(zfs_sb_t *zsb, dmu_buf_t *db, int blksz,
zp->z_mode = mode;
ip->i_generation = (uint32_t)tmp_gen;
/*
* 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_new(zp);
zfs_inode_set_ops(zsb, ip);
@ -940,8 +923,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 {
@ -1152,7 +1134,7 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
* 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 {
@ -1200,11 +1182,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);