Skip to content

Commit

Permalink
Revert "Revert "Revert "Fix unlink/xattr deadlock"""
Browse files Browse the repository at this point in the history
This reverts commit 7973e46 which brings the basic flow of the
code back in line with the other ZFS implementations.  This
was possible due to the following related changes.

e89260a Directory xattr znodes hold a reference on their parent
6f9548c Fix deadlock in zfs_zget()
0a50679 Add zfs_iput_async() interface
4dd1893 Avoid 128K kmem allocations in mzap_upgrade()

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <ryao@gentoo.org>
Closes openzfs#457
Closes openzfs#2058
Closes openzfs#2128
Closes openzfs#2240
  • Loading branch information
behlendorf committed Aug 11, 2014
1 parent 0a50679 commit 0d5c500
Showing 1 changed file with 57 additions and 85 deletions.
142 changes: 57 additions & 85 deletions module/zfs/zfs_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,57 @@ zfs_unlinked_add(znode_t *zp, dmu_tx_t *tx)
zap_add_int(zsb->z_os, zsb->z_unlinkedobj, zp->z_id, tx));
}

/*
* Clean up any znodes that had no links when we either crashed or
* (force) umounted the file system.
*/
void
zfs_unlinked_drain(zfs_sb_t *zsb)
{
zap_cursor_t zc;
zap_attribute_t zap;
dmu_object_info_t doi;
znode_t *zp;
int error;

/*
* Iterate over the contents of the unlinked set.
*/
for (zap_cursor_init(&zc, zsb->z_os, zsb->z_unlinkedobj);
zap_cursor_retrieve(&zc, &zap) == 0;
zap_cursor_advance(&zc)) {

/*
* See what kind of object we have in list
*/

error = dmu_object_info(zsb->z_os, zap.za_first_integer, &doi);
if (error != 0)
continue;

ASSERT((doi.doi_type == DMU_OT_PLAIN_FILE_CONTENTS) ||
(doi.doi_type == DMU_OT_DIRECTORY_CONTENTS));
/*
* We need to re-mark these list entries for deletion,
* so we pull them back into core and set zp->z_unlinked.
*/
error = zfs_zget(zsb, zap.za_first_integer, &zp);

/*
* We may pick up znodes that are already marked for deletion.
* This could happen during the purge of an extended attribute
* directory. All we need to do is skip over them, since they
* are already in the system marked z_unlinked.
*/
if (error != 0)
continue;

zp->z_unlinked = B_TRUE;
iput(ZTOI(zp));
}
zap_cursor_fini(&zc);
}

/*
* Delete the entire contents of a directory. Return a count
* of the number of entries that could not be deleted. If we encounter
Expand Down Expand Up @@ -517,7 +568,8 @@ zfs_purgedir(znode_t *dzp)
continue;
}

ASSERT(S_ISREG(ZTOI(xzp)->i_mode)||S_ISLNK(ZTOI(xzp)->i_mode));
ASSERT(S_ISREG(ZTOI(xzp)->i_mode) ||
S_ISLNK(ZTOI(xzp)->i_mode));

tx = dmu_tx_create(zsb->z_os);
dmu_tx_hold_sa(tx, dzp->z_sa_hdl, B_FALSE);
Expand Down Expand Up @@ -550,71 +602,6 @@ zfs_purgedir(znode_t *dzp)
return (skipped);
}

/*
* Clean up any znodes that had no links when we either crashed or
* (force) umounted the file system.
*/
void
zfs_unlinked_drain(zfs_sb_t *zsb)
{
zap_cursor_t zc;
zap_attribute_t zap;
dmu_object_info_t doi;
znode_t *zp;
int error;

/*
* Iterate over the contents of the unlinked set.
*/
for (zap_cursor_init(&zc, zsb->z_os, zsb->z_unlinkedobj);
zap_cursor_retrieve(&zc, &zap) == 0;
zap_cursor_advance(&zc)) {

/*
* See what kind of object we have in list
*/

error = dmu_object_info(zsb->z_os, zap.za_first_integer, &doi);
if (error != 0)
continue;

ASSERT((doi.doi_type == DMU_OT_PLAIN_FILE_CONTENTS) ||
(doi.doi_type == DMU_OT_DIRECTORY_CONTENTS));
/*
* We need to re-mark these list entries for deletion,
* so we pull them back into core and set zp->z_unlinked.
*/
error = zfs_zget(zsb, zap.za_first_integer, &zp);

/*
* We may pick up znodes that are already marked for deletion.
* This could happen during the purge of an extended attribute
* directory. All we need to do is skip over them, since they
* are already in the system marked z_unlinked.
*/
if (error != 0)
continue;

zp->z_unlinked = B_TRUE;

/*
* If this is an attribute directory, purge its contents.
*/
if (S_ISDIR(ZTOI(zp)->i_mode) && (zp->z_pflags & ZFS_XATTR)) {
/*
* We don't need to check the return value of
* zfs_purgedir here, because zfs_rmnode will just
* return this xattr directory to the unlinked set
* until all of its xattrs are gone.
*/
(void) zfs_purgedir(zp);
}

iput(ZTOI(zp));
}
zap_cursor_fini(&zc);
}

void
zfs_rmnode(znode_t *zp)
{
Expand All @@ -624,7 +611,6 @@ zfs_rmnode(znode_t *zp)
dmu_tx_t *tx;
uint64_t acl_obj;
uint64_t xattr_obj;
uint64_t count;
int error;

ASSERT(zp->z_links == 0);
Expand All @@ -634,27 +620,13 @@ zfs_rmnode(znode_t *zp)
* If this is an attribute directory, purge its contents.
*/
if (S_ISDIR(ZTOI(zp)->i_mode) && (zp->z_pflags & ZFS_XATTR)) {
error = zap_count(os, zp->z_id, &count);
if (error) {
zfs_znode_dmu_fini(zp);
return;
}

if (count > 0) {
taskq_t *taskq;

if (zfs_purgedir(zp) != 0) {
/*
* There are still directory entries in this xattr
* directory. Let zfs_unlinked_drain() deal with
* them to avoid deadlocking this process in the
* zfs_purgedir()->zfs_zget()->ilookup() callpath
* on the xattr inode's I_FREEING bit.
* Not enough space to delete some xattrs.
* Leave it in the unlinked set.
*/
taskq = dsl_pool_iput_taskq(dmu_objset_pool(os));
taskq_dispatch(taskq, (task_func_t *)
zfs_unlinked_drain, zsb, TQ_SLEEP);

zfs_znode_dmu_fini(zp);

return;
}
}
Expand Down

0 comments on commit 0d5c500

Please sign in to comment.