diff options
author | Brian Behlendorf <[email protected]> | 2013-07-11 14:11:32 -0700 |
---|---|---|
committer | Brian Behlendorf <[email protected]> | 2013-07-12 10:06:53 -0700 |
commit | 76351672c222f28ea1b681097a9eff58a6791555 (patch) | |
tree | c7654dba10dcca9e875fe19a7df7bce569d5e9ab | |
parent | e34f17a8dfc0ef7650ba489f9772c2c20dc8bec4 (diff) |
Fix zfsctl_expire_snapshot() deadlock
It is possible for an automounted snapshot which is expiring to
deadlock with a manual unmount of the snapshot. This can occur
because taskq_cancel_id() will block if the task is currently
executing until it completes. But it will never complete because
zfsctl_unmount_snapshot() is holding the zsb->z_ctldir_lock which
zfsctl_expire_snapshot() must acquire.
---------------------- z_unmount/0:2153 ---------------------
mutex_lock <blocking on zsb->z_ctldir_lock>
zfsctl_unmount_snapshot
zfsctl_expire_snapshot
taskq_thread
------------------------- zfs:10690 -------------------------
taskq_wait_id <waiting for z_unmount to exit>
taskq_cancel_id
__zfsctl_unmount_snapshot
zfsctl_unmount_snapshot <takes zsb->z_ctldir_lock>
zfs_unmount_snap
zfs_ioc_destroy_snaps_nvl
zfsdev_ioctl
do_vfs_ioctl
We resolve the deadlock by dropping the zsb->z_ctldir_lock before
calling __zfsctl_unmount_snapshot(). The lock is only there to
prevent concurrent modification to the zsb->z_ctldir_snaps AVL
tree. Moreover, we're careful to remove the zfs_snapentry_t from
the AVL tree before dropping the lock which ensures no other tasks
can find it. On failure it's added back to the tree.
Signed-off-by: Brian Behlendorf <[email protected]>
Signed-off-by: Chris Dunlap <[email protected]>
Closes #1527
-rw-r--r-- | module/zfs/zfs_ctldir.c | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/module/zfs/zfs_ctldir.c b/module/zfs/zfs_ctldir.c index 4fa530b6c..168f853f1 100644 --- a/module/zfs/zfs_ctldir.c +++ b/module/zfs/zfs_ctldir.c @@ -732,7 +732,11 @@ zfsctl_unmount_snapshot(zfs_sb_t *zsb, char *name, int flags) sep = avl_find(&zsb->z_ctldir_snaps, &search, NULL); if (sep) { avl_remove(&zsb->z_ctldir_snaps, sep); + mutex_exit(&zsb->z_ctldir_lock); + error = __zfsctl_unmount_snapshot(sep, flags); + + mutex_enter(&zsb->z_ctldir_lock); if (error == EBUSY) avl_add(&zsb->z_ctldir_snaps, sep); else @@ -767,7 +771,11 @@ zfsctl_unmount_snapshots(zfs_sb_t *zsb, int flags, int *count) while (sep != NULL) { next = AVL_NEXT(&zsb->z_ctldir_snaps, sep); avl_remove(&zsb->z_ctldir_snaps, sep); + mutex_exit(&zsb->z_ctldir_lock); + error = __zfsctl_unmount_snapshot(sep, flags); + + mutex_enter(&zsb->z_ctldir_lock); if (error == EBUSY) { avl_add(&zsb->z_ctldir_snaps, sep); (*count)++; |