Skip to content

Fix deadlock between zpool export and zfs list #3137

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.

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
Fix deadlock between zpool export and zfs list
Pool reference count is NOT checked in spa_export_common()
if the pool has been imported readonly=on, i.e. spa->spa_sync_on
is FALSE. Then zpool export and zfs list may deadlock:

1. Pool A is imported readonly.
2. zpool export A and zfs list are run concurrently.
3. zfs command gets reference on the spa, which holds a dbuf on
   on the MOS meta dnode.
4. zpool command grabs spa_namespace_lock, and tries to evict dbufs
   of the MOS meta dnode. The dbuf held by zfs command can't be
   evicted as its reference count is not 0.
5. zpool command blocks in dnode_special_close() waiting for the
   MOS meta dnode reference count to drop to 0, with
   spa_namespace_lock held.
6. zfs command tries to get the spa_namespace_lock with a reference
   on the spa held, which holds a dbuf on the MOS meta dnode.
7. Now zpool command and zfs command deadlock each other.

Also any further zfs/zpool command will block on spa_namespace_lock
forever.

The fix is to always check pool reference count in spa_export_common(),
no matter whether the pool was imported readonly or not.

Signed-off-by: Isaac Huang <he.huang@intel.com>
Closes #2034
  • Loading branch information
huangheintel committed Mar 1, 2015
commit 65cdaa78aff3c2e21ff912b9acfc523f22a3b2c4
30 changes: 17 additions & 13 deletions module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -4262,30 +4262,33 @@ spa_export_common(char *pool, int new_state, nvlist_t **oldconfig,
mutex_enter(&spa_namespace_lock);
spa_close(spa, FTAG);

if (spa->spa_state == POOL_STATE_UNINITIALIZED)
goto export_spa;
/*
* The pool will be in core if it's openable,
* in which case we can modify its state.
*/
if (spa->spa_state != POOL_STATE_UNINITIALIZED && spa->spa_sync_on) {
if (spa->spa_sync_on)
/*
* Objsets may be open only because they're dirty, so we
* have to force it to sync before checking spa_refcnt.
*/
txg_wait_synced(spa->spa_dsl_pool, 0);

/*
* A pool cannot be exported or destroyed if there are active
* references. If we are resetting a pool, allow references by
* fault injection handlers.
*/
if (!spa_refcount_zero(spa) ||
(spa->spa_inject_ref != 0 &&
new_state != POOL_STATE_UNINITIALIZED)) {
spa_async_resume(spa);
mutex_exit(&spa_namespace_lock);
return (SET_ERROR(EBUSY));
}
/*
* A pool cannot be exported or destroyed if there are active
* references. If we are resetting a pool, allow references by
* fault injection handlers.
*/
if (!spa_refcount_zero(spa) ||
(spa->spa_inject_ref != 0 &&
new_state != POOL_STATE_UNINITIALIZED)) {
spa_async_resume(spa);
mutex_exit(&spa_namespace_lock);
return (SET_ERROR(EBUSY));
}

if (spa->spa_sync_on) {
/*
* A pool cannot be exported if it has an active shared spare.
* This is to prevent other pools stealing the active spare
Expand Down Expand Up @@ -4314,6 +4317,7 @@ spa_export_common(char *pool, int new_state, nvlist_t **oldconfig,
}
}

export_spa:
spa_event_notify(spa, NULL, FM_EREPORT_ZFS_POOL_DESTROY);

if (spa->spa_state != POOL_STATE_UNINITIALIZED) {
Expand Down