Skip to content

Commit

Permalink
Fix deadlocks resulting from the post-kmem rework merge
Browse files Browse the repository at this point in the history 
Since many more allocations are allowed to sleep following the post-kmem
rework merge, there are many more opportunities for deadlocks related
to the pre-superblock z_hold_mtx array.  This patch puts all threads
locking z_hold_mtx under PF_STRANS to disallow re-entry into the zfs code.

... WIP patch, still ...
Previous comments which no longer apply:

	Put the following functions under PF_FSTRANS to prevent deadlocks
	due to re-entering the filesystem during reclaim:

		dbuf_hold()
		dmu_prefetch()
		sa_buf_hold()

	Testing this one now:
		dmu_zfetch_fetch() - z_hold_mtx[] via zget and zf_rwlock via dmu_zfetch
  • Loading branch information
@dweeezil
dweeezil committed Mar 27, 2015
1 parent 4c7b7ee commit 8b5bec7
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 11 deletions.
3 changes: 2 additions & 1 deletion module/zfs/dbuf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2088,7 +2088,8 @@ dmu_buf_impl_t *
dbuf_hold(dnode_t *dn, uint64_t blkid, void *tag)
{
dmu_buf_impl_t *db;
int err = dbuf_hold_impl(dn, 0, blkid, FALSE, tag, &db);
int err;
err = dbuf_hold_impl(dn, 0, blkid, FALSE, tag, &db);
return (err ? NULL : db);
}

Expand Down
3 changes: 3 additions & 0 deletions module/zfs/dmu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,13 @@ dmu_prefetch(objset_t *os, uint64_t object, uint64_t offset, uint64_t len)

if (nblks != 0) {
int i;
fstrans_cookie_t cookie;

blkid = dbuf_whichblock(dn, offset);
cookie = spl_fstrans_mark();
for (i = 0; i < nblks; i++)
dbuf_prefetch(dn, blkid + i, ZIO_PRIORITY_SYNC_READ);
spl_fstrans_unmark(cookie);
}

rw_exit(&dn->dn_struct_rwlock);
Expand Down
3 changes: 3 additions & 0 deletions module/zfs/dmu_zfetch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,15 @@ dmu_zfetch_fetch(dnode_t *dn, uint64_t blkid, uint64_t nblks)
{
uint64_t fetchsz;
uint64_t i;
fstrans_cookie_t cookie;

fetchsz = dmu_zfetch_fetchsz(dn, blkid, nblks);

cookie = spl_fstrans_mark();
for (i = 0; i < fetchsz; i++) {
dbuf_prefetch(dn, blkid + i, ZIO_PRIORITY_ASYNC_READ);
}
spl_fstrans_unmark(cookie);

return (fetchsz);
}
Expand Down
5 changes: 4 additions & 1 deletion module/zfs/sa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,10 @@ sa_handle_get(objset_t *objset, uint64_t objid, void *userp,
int
sa_buf_hold(objset_t *objset, uint64_t obj_num, void *tag, dmu_buf_t **db)
{
return (dmu_bonus_hold(objset, obj_num, tag, db));
int rval;

rval = (dmu_bonus_hold(objset, obj_num, tag, db));
return (rval);
}

void
Expand Down
33 changes: 24 additions & 9 deletions module/zfs/zfs_znode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
sa_bulk_attr_t *sa_attrs;
int cnt = 0;
zfs_acl_locator_cb_t locate = { 0 };
fstrans_cookie_t cookie;

if (zsb->z_replay) {
obj = vap->va_nodeid;
Expand Down Expand Up @@ -610,6 +611,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
}
}

cookie = spl_fstrans_mark();
ZFS_OBJ_HOLD_ENTER(zsb, obj);
VERIFY(0 == sa_buf_hold(zsb->z_os, obj, NULL, &db));

Expand Down Expand Up @@ -785,6 +787,7 @@ zfs_mknode(znode_t *dzp, vattr_t *vap, dmu_tx_t *tx, cred_t *cr,
}
kmem_free(sa_attrs, sizeof (sa_bulk_attr_t) * ZPL_END);
ZFS_OBJ_HOLD_EXIT(zsb, obj);
spl_fstrans_unmark(cookie);
}

/*
Expand Down Expand Up @@ -890,6 +893,7 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
znode_t *zp;
int err;
sa_handle_t *hdl;
fstrans_cookie_t cookie = spl_fstrans_mark();

*zpp = NULL;

Expand All @@ -899,6 +903,7 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
err = sa_buf_hold(zsb->z_os, obj_num, NULL, &db);
if (err) {
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (err);
}

Expand All @@ -909,6 +914,7 @@ 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);
spl_fstrans_unmark(cookie);
return (SET_ERROR(EINVAL));
}

Expand Down Expand Up @@ -956,6 +962,7 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
mutex_exit(&zp->z_lock);
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (err);
}

Expand All @@ -977,6 +984,7 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
*zpp = zp;
}
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (err);
}

Expand All @@ -992,7 +1000,9 @@ zfs_rezget(znode_t *zp)
int err;
int count = 0;
uint64_t gen;
fstrans_cookie_t cookie;

cookie = spl_fstrans_mark();
ZFS_OBJ_HOLD_ENTER(zsb, obj_num);

mutex_enter(&zp->z_acl_lock);
Expand All @@ -1018,6 +1028,7 @@ zfs_rezget(znode_t *zp)
err = sa_buf_hold(zsb->z_os, obj_num, NULL, &db);
if (err) {
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (err);
}

Expand All @@ -1028,6 +1039,7 @@ zfs_rezget(znode_t *zp)
doi.doi_bonus_size < sizeof (znode_phys_t)))) {
sa_buf_rele(db, NULL);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (SET_ERROR(EINVAL));
}

Expand All @@ -1054,6 +1066,7 @@ zfs_rezget(znode_t *zp)
if (sa_bulk_lookup(zp->z_sa_hdl, bulk, count)) {
zfs_znode_dmu_fini(zp);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (SET_ERROR(EIO));
}

Expand All @@ -1062,6 +1075,7 @@ zfs_rezget(znode_t *zp)
if (gen != zp->z_gen) {
zfs_znode_dmu_fini(zp);
ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);
return (SET_ERROR(EIO));
}

Expand All @@ -1070,6 +1084,7 @@ zfs_rezget(znode_t *zp)
zfs_inode_update(zp);

ZFS_OBJ_HOLD_EXIT(zsb, obj_num);
spl_fstrans_unmark(cookie);

return (0);
}
Expand All @@ -1081,6 +1096,7 @@ zfs_znode_delete(znode_t *zp, dmu_tx_t *tx)
objset_t *os = zsb->z_os;
uint64_t obj = zp->z_id;
uint64_t acl_obj = zfs_external_acl(zp);
fstrans_cookie_t cookie = spl_fstrans_mark();

ZFS_OBJ_HOLD_ENTER(zsb, obj);
if (acl_obj) {
Expand All @@ -1090,14 +1106,15 @@ zfs_znode_delete(znode_t *zp, dmu_tx_t *tx)
VERIFY(0 == dmu_object_free(os, obj, tx));
zfs_znode_dmu_fini(zp);
ZFS_OBJ_HOLD_EXIT(zsb, obj);
spl_fstrans_unmark(cookie);
}

void
zfs_zinactive(znode_t *zp)
{
zfs_sb_t *zsb = ZTOZSB(zp);
uint64_t z_id = zp->z_id;
boolean_t drop_mutex = 0;
fstrans_cookie_t cookie;

ASSERT(zp->z_sa_hdl);

Expand All @@ -1110,10 +1127,8 @@ zfs_zinactive(znode_t *zp)
* zfs_zinactive() call path. To avoid this deadlock the process
* must not reacquire the mutex when it is already holding it.
*/
if (!ZFS_OBJ_HOLD_OWNED(zsb, z_id)) {
ZFS_OBJ_HOLD_ENTER(zsb, z_id);
drop_mutex = 1;
}
cookie = spl_fstrans_mark();
ZFS_OBJ_HOLD_ENTER(zsb, z_id);

mutex_enter(&zp->z_lock);

Expand All @@ -1124,8 +1139,8 @@ zfs_zinactive(znode_t *zp)
if (zp->z_unlinked) {
mutex_exit(&zp->z_lock);

if (drop_mutex)
ZFS_OBJ_HOLD_EXIT(zsb, z_id);
ZFS_OBJ_HOLD_EXIT(zsb, z_id);
spl_fstrans_unmark(cookie);

zfs_rmnode(zp);
return;
Expand All @@ -1134,8 +1149,8 @@ zfs_zinactive(znode_t *zp)
mutex_exit(&zp->z_lock);
zfs_znode_dmu_fini(zp);

if (drop_mutex)
ZFS_OBJ_HOLD_EXIT(zsb, z_id);
ZFS_OBJ_HOLD_EXIT(zsb, z_id);
spl_fstrans_unmark(cookie);
}

static inline int
Expand Down

0 comments on commit 8b5bec7

Please sign in to comment.