Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zfs_zget() deadlock #2237

Closed
wants to merge 3 commits into from
Closed

zfs_zget() deadlock #2237

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ac6729
Revert "Fixed a use-after-free bug in zfs_zget()."
behlendorf Apr 3, 2014
65cafd8
Fix deadlock in zfs_zget()
ryao Mar 25, 2014
1360bd4
Fix locking order in zfs_zget()
ryao Mar 25, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 22 additions & 24 deletions module/zfs/zfs_znode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,19 +859,15 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
znode_t *zp;
int err;
sa_handle_t *hdl;
struct inode *ip;

*zpp = NULL;

again:
ip = ilookup(zsb->z_sb, obj_num);

ZFS_OBJ_HOLD_ENTER(zsb, obj_num);

err = sa_buf_hold(zsb->z_os, obj_num, NULL, &db);
if (err) {
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
iput(ip);
return (err);
}

Expand All @@ -882,28 +878,14 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
doi.doi_bonus_size < sizeof (znode_phys_t)))) {
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
iput(ip);
return (SET_ERROR(EINVAL));
}

hdl = dmu_buf_get_user(db);
if (hdl != NULL) {
if (ip == NULL) {
/*
* ilookup returned NULL, which means
* the znode is dying - but the SA handle isn't
* quite dead yet, we need to drop any locks
* we're holding, re-schedule the task and try again.
*/
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);

schedule();
goto again;
}

zp = sa_get_userdata(hdl);


/*
* Since "SA" does immediate eviction we
* should never find a sa handle that doesn't
Expand All @@ -917,19 +899,35 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
if (zp->z_unlinked) {
err = SET_ERROR(ENOENT);
} else {
igrab(ZTOI(zp));
/*
* If igrab() returns NULL the VFS has independently
* determined the inode should be evicted and has
* called iput_final() to start the eviction process.
* The SA handle is still valid but because the VFS
* requires that the eviction succeed we must drop
* our locks and references to allow the eviction to
* complete. The zfs_zget() may then be retried.
*
* This unlikely case could be optimized by registering
* a sops->drop_inode() callback. The callback would
* need to detect the active SA hold thereby informing
* the VFS that this inode should not be evicted.
*/
if (igrab(ZTOI(zp)) == NULL) {
mutex_exit(&zp->z_lock);
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
goto again;
}
*zpp = zp;
err = 0;
}
sa_buf_rele(db, NULL);
mutex_exit(&zp->z_lock);
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
iput(ip);
return (err);
}

ASSERT3P(ip, ==, NULL);

/*
* Not found create new znode/vnode but only if file exists.
*
Expand Down