Skip to content

Commit

Permalink
merge spa from upstream (#123)
Browse files Browse the repository at this point in the history
* Avoid dirtying the final TXGs when exporting a pool

There are two codepaths than can dirty final TXGs:

1) If calling spa_export_common()->spa_unload()->
   spa_unload_log_sm_flush_all() after the spa_final_txg is set, then
   spa_sync()->spa_flush_metaslabs() may end up dirtying the final
   TXGs. Then we have the following panic:
   Call Trace:
    <TASK>
    dump_stack_lvl+0x46/0x62
    spl_panic+0xea/0x102 [spl]
    dbuf_dirty+0xcd6/0x11b0 [zfs]
    zap_lockdir_impl+0x321/0x590 [zfs]
    zap_lockdir+0xed/0x150 [zfs]
    zap_update+0x69/0x250 [zfs]
    feature_sync+0x5f/0x190 [zfs]
    space_map_alloc+0x83/0xc0 [zfs]
    spa_generate_syncing_log_sm+0x10b/0x2f0 [zfs]
    spa_flush_metaslabs+0xb2/0x350 [zfs]
    spa_sync_iterate_to_convergence+0x15a/0x320 [zfs]
    spa_sync+0x2e0/0x840 [zfs]
    txg_sync_thread+0x2b1/0x3f0 [zfs]
    thread_generic_wrapper+0x62/0xa0 [spl]
    kthread+0x127/0x150
    ret_from_fork+0x22/0x30
    </TASK>

2) Calling vdev_*_stop_all() for a second time in spa_unload() after
   spa_export_common() unnecessarily delays the final TXGs beyond what
   spa_final_txg is set at.

Fix this by performing the check and call for
spa_unload_log_sm_flush_all() before the spa_final_txg is set in
spa_export_common(). Also check if the spa_final_txg has already been
set in spa_unload() and skip those calls in this case.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
External-issue: https://www.illumos.org/issues/9081
Closes openzfs#13048 
Closes openzfs#13098

* Add spa _os() hooks

Add hooks for when spa is created, exported, activated and
deactivated. Used by macOS to attach iokit, and lock
kext as busy (to stop unloads).

Userland, Linux, and, FreeBSD have empty stubs.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#12801

* Windows: Add spa_os() hooks

use the common spa prototypes instead of the windows specific ones

Signed-off-by: Andrew Innes <[email protected]>

Signed-off-by: Andrew Innes <[email protected]>
Co-authored-by: George Amanakis <[email protected]>
Co-authored-by: Jorgen Lundman <[email protected]>
  • Loading branch information
3 people authored Aug 24, 2022
1 parent 91da942 commit b78eb02
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 29 deletions.
4 changes: 0 additions & 4 deletions include/os/windows/zfs/sys/zfs_context_os.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,6 @@ extern void kx_qsort(void *array, size_t nm, size_t member_size,

#define strstr kmem_strstr

void spa_create_os(void *spa);
void spa_export_os(void *spa);
void spa_activate_os(void *spa);
void spa_deactivate_os(void *spa);

#define task_io_account_read(n)
#define task_io_account_write(n)
Expand Down
5 changes: 5 additions & 0 deletions include/sys/spa.h
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,11 @@ extern int spa_wait_tag(const char *name, zpool_wait_activity_t activity,
extern void spa_notify_waiters(spa_t *spa);
extern void spa_wake_waiters(spa_t *spa);

extern void spa_import_os(spa_t *spa);
extern void spa_export_os(spa_t *spa);
extern void spa_activate_os(spa_t *spa);
extern void spa_deactivate_os(spa_t *spa);

/* module param call functions */
int param_set_deadman_ziotime(ZFS_MODULE_PARAM_ARGS);
int param_set_deadman_synctime(ZFS_MODULE_PARAM_ARGS);
Expand Down
24 changes: 24 additions & 0 deletions lib/libzpool/kernel.c
Original file line number Diff line number Diff line change
Expand Up @@ -1453,3 +1453,27 @@ zfsvfs_update_fromname(const char *oldname, const char *newname)
{
(void) oldname, (void) newname;
}

void
spa_import_os(spa_t *spa)
{
(void) spa;
}

void
spa_export_os(spa_t *spa)
{
(void) spa;
}

void
spa_activate_os(spa_t *spa)
{
(void) spa;
}

void
spa_deactivate_os(spa_t *spa)
{
(void) spa;
}
24 changes: 24 additions & 0 deletions module/os/freebsd/zfs/spa_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,27 @@ spa_history_zone(void)
{
return ("freebsd");
}

void
spa_import_os(spa_t *spa)
{
(void) spa;
}

void
spa_export_os(spa_t *spa)
{
(void) spa;
}

void
spa_activate_os(spa_t *spa)
{
(void) spa;
}

void
spa_deactivate_os(spa_t *spa)
{
(void) spa;
}
24 changes: 24 additions & 0 deletions module/os/linux/zfs/spa_misc_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,27 @@ spa_history_zone(void)
{
return ("linux");
}

void
spa_import_os(spa_t *spa)
{
(void) spa;
}

void
spa_export_os(spa_t *spa)
{
(void) spa;
}

void
spa_activate_os(spa_t *spa)
{
(void) spa;
}

void
spa_deactivate_os(spa_t *spa)
{
(void) spa;
}
8 changes: 4 additions & 4 deletions module/os/windows/zfs/spa_misc_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,23 @@ spa_history_zone(void)
}

void
spa_create_os(void *arg)
spa_import_os(spa_t *arg)
{
}

void
spa_export_os(void *arg)
spa_export_os(spa_t *arg)
{
}

void
spa_activate_os(void *arg)
spa_activate_os(spa_t *arg)
{
atomic_inc_64(&zfs_module_busy);
}

void
spa_deactivate_os(void *arg)
spa_deactivate_os(spa_t *arg)
{
atomic_dec_64(&zfs_module_busy);
}
72 changes: 51 additions & 21 deletions module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1317,9 +1317,7 @@ spa_activate(spa_t *spa, spa_mode_t mode)
spa_error_entry_compare, sizeof (spa_error_entry_t),
offsetof(spa_error_entry_t, se_avl));

#if defined(_KERNEL) && defined(_WIN32)
spa_activate_os(spa);
#endif

spa_keystore_init(&spa->spa_keystore);

Expand Down Expand Up @@ -1457,9 +1455,9 @@ spa_deactivate(spa_t *spa)
thread_join(spa->spa_did);
spa->spa_did = 0;
}
#if defined(_KERNEL) && defined(_WIN32)

spa_deactivate_os(spa);
#endif

}

/*
Expand Down Expand Up @@ -1606,25 +1604,33 @@ spa_unload(spa_t *spa)
spa_wake_waiters(spa);

/*
* If the log space map feature is enabled and the pool is getting
* exported (but not destroyed), we want to spend some time flushing
* as many metaslabs as we can in an attempt to destroy log space
* maps and save import time.
* If we have set the spa_final_txg, we have already performed the
* tasks below in spa_export_common(). We should not redo it here since
* we delay the final TXGs beyond what spa_final_txg is set at.
*/
if (spa_should_flush_logs_on_unload(spa))
spa_unload_log_sm_flush_all(spa);
if (spa->spa_final_txg == UINT64_MAX) {
/*
* If the log space map feature is enabled and the pool is
* getting exported (but not destroyed), we want to spend some
* time flushing as many metaslabs as we can in an attempt to
* destroy log space maps and save import time.
*/
if (spa_should_flush_logs_on_unload(spa))
spa_unload_log_sm_flush_all(spa);

/*
* Stop async tasks.
*/
spa_async_suspend(spa);
/*
* Stop async tasks.
*/
spa_async_suspend(spa);

if (spa->spa_root_vdev) {
vdev_t *root_vdev = spa->spa_root_vdev;
vdev_initialize_stop_all(root_vdev, VDEV_INITIALIZE_ACTIVE);
vdev_trim_stop_all(root_vdev, VDEV_TRIM_ACTIVE);
vdev_autotrim_stop_all(spa);
vdev_rebuild_stop_all(spa);
if (spa->spa_root_vdev) {
vdev_t *root_vdev = spa->spa_root_vdev;
vdev_initialize_stop_all(root_vdev,
VDEV_INITIALIZE_ACTIVE);
vdev_trim_stop_all(root_vdev, VDEV_TRIM_ACTIVE);
vdev_autotrim_stop_all(spa);
vdev_rebuild_stop_all(spa);
}
}

/*
Expand Down Expand Up @@ -6032,6 +6038,8 @@ spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
spa->spa_minref = zfs_refcount_count(&spa->spa_refcount);
spa->spa_load_state = SPA_LOAD_NONE;

spa_import_os(spa);

mutex_exit(&spa_namespace_lock);

return (0);
Expand Down Expand Up @@ -6215,6 +6223,8 @@ spa_import(char *pool, nvlist_t *config, nvlist_t *props, uint64_t flags)

zvol_create_minors_recursive(pool);

spa_import_os(spa);

return (0);
}

Expand Down Expand Up @@ -6436,14 +6446,34 @@ spa_export_common(const char *pool, int new_state, nvlist_t **oldconfig,
if (new_state != POOL_STATE_UNINITIALIZED && !hardforce) {
spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER);
spa->spa_state = new_state;
vdev_config_dirty(spa->spa_root_vdev);
spa_config_exit(spa, SCL_ALL, FTAG);
}

/*
* If the log space map feature is enabled and the pool is
* getting exported (but not destroyed), we want to spend some
* time flushing as many metaslabs as we can in an attempt to
* destroy log space maps and save import time. This has to be
* done before we set the spa_final_txg, otherwise
* spa_sync() -> spa_flush_metaslabs() may dirty the final TXGs.
* spa_should_flush_logs_on_unload() should be called after
* spa_state has been set to the new_state.
*/
if (spa_should_flush_logs_on_unload(spa))
spa_unload_log_sm_flush_all(spa);

if (new_state != POOL_STATE_UNINITIALIZED && !hardforce) {
spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER);
spa->spa_final_txg = spa_last_synced_txg(spa) +
TXG_DEFER_SIZE + 1;
vdev_config_dirty(spa->spa_root_vdev);
spa_config_exit(spa, SCL_ALL, FTAG);
}
}

export_spa:
spa_export_os(spa);

if (new_state == POOL_STATE_DESTROYED)
spa_event_notify(spa, NULL, NULL, ESC_ZFS_POOL_DESTROY);
else if (new_state == POOL_STATE_EXPORTED)
Expand Down

0 comments on commit b78eb02

Please sign in to comment.