Skip to content

Commit

Permalink
Again fix race between zil_commit() and zil_suspend().
Browse files Browse the repository at this point in the history
With zl_suspend read in zil_commit() not protected by any locks it
is possible for new ZIL writes to be in progress while zil_destroy()
called by zil_suspend() freeing them.  This patch closes the race
by taking zl_issuer_lock in zil_suspend() and adding the second
zl_suspend check to zil_get_commit_list(), protected by the lock.
It allows all already queued transactions to be logged normally,
while blocks any new ones, calling txg_wait_synced() for the TXGs.

Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
  • Loading branch information
amotin committed Jun 14, 2023
1 parent e32e326 commit 06d9df4
Showing 1 changed file with 23 additions and 4 deletions.
27 changes: 23 additions & 4 deletions module/zfs/zil.c
Original file line number Diff line number Diff line change
Expand Up @@ -2521,10 +2521,10 @@ zil_clean(zilog_t *zilog, uint64_t synced_txg)
* This function will traverse the queue of itxs that need to be
* committed, and move them onto the ZIL's zl_itx_commit_list.
*/
static void
static uint64_t
zil_get_commit_list(zilog_t *zilog)
{
uint64_t otxg, txg;
uint64_t otxg, txg, wtxg = 0;
list_t *commit_list = &zilog->zl_itx_commit_list;

ASSERT(MUTEX_HELD(&zilog->zl_issuer_lock));
Expand Down Expand Up @@ -2558,10 +2558,22 @@ zil_get_commit_list(zilog_t *zilog)
*/
ASSERT(zilog_is_dirty_in_txg(zilog, txg) ||
spa_freeze_txg(zilog->zl_spa) != UINT64_MAX);
list_move_tail(commit_list, &itxg->itxg_itxs->i_sync_list);
list_t *sync_list = &itxg->itxg_itxs->i_sync_list;
if (unlikely(zilog->zl_suspend > 0)) {
/*
* ZIL was just suspended, but we lost the race.
* Allow all earlier itxs to be committed, but ask
* caller to do txg_wait_synced(txg) for any new.
*/
if (!list_is_empty(sync_list))
wtxg = MAX(wtxg, txg);
} else {
list_move_tail(commit_list, sync_list);
}

mutex_exit(&itxg->itxg_lock);
}
return (wtxg);
}

/*
Expand Down Expand Up @@ -2954,6 +2966,7 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw)
{
list_t ilwbs;
lwb_t *lwb;
uint64_t wtxg = 0;

ASSERT(!MUTEX_HELD(&zilog->zl_lock));
ASSERT(spa_writeable(zilog->zl_spa));
Expand Down Expand Up @@ -2983,7 +2996,7 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw)

ZIL_STAT_BUMP(zilog, zil_commit_writer_count);

zil_get_commit_list(zilog);
wtxg = zil_get_commit_list(zilog);
zil_prune_commit_list(zilog);
zil_process_commit_list(zilog, zcw, &ilwbs);

Expand All @@ -2992,6 +3005,8 @@ zil_commit_writer(zilog_t *zilog, zil_commit_waiter_t *zcw)
while ((lwb = list_remove_head(&ilwbs)) != NULL)
zil_lwb_write_issue(zilog, lwb);
list_destroy(&ilwbs);
if (wtxg != 0)
txg_wait_synced(zilog->zl_dmu_pool, wtxg);
}

static void
Expand Down Expand Up @@ -3901,11 +3916,13 @@ zil_suspend(const char *osname, void **cookiep)
return (error);
zilog = dmu_objset_zil(os);

mutex_enter(&zilog->zl_issuer_lock);
mutex_enter(&zilog->zl_lock);
zh = zilog->zl_header;

if (zh->zh_flags & ZIL_REPLAY_NEEDED) { /* unplayed log */
mutex_exit(&zilog->zl_lock);
mutex_exit(&zilog->zl_issuer_lock);
dmu_objset_rele(os, suspend_tag);
return (SET_ERROR(EBUSY));
}
Expand All @@ -3919,6 +3936,7 @@ zil_suspend(const char *osname, void **cookiep)
if (cookiep == NULL && !zilog->zl_suspending &&
(zilog->zl_suspend > 0 || BP_IS_HOLE(&zh->zh_log))) {
mutex_exit(&zilog->zl_lock);
mutex_exit(&zilog->zl_issuer_lock);
dmu_objset_rele(os, suspend_tag);
return (0);
}
Expand All @@ -3927,6 +3945,7 @@ zil_suspend(const char *osname, void **cookiep)
dsl_pool_rele(dmu_objset_pool(os), suspend_tag);

zilog->zl_suspend++;
mutex_exit(&zilog->zl_issuer_lock);

if (zilog->zl_suspend > 1) {
/*
Expand Down

0 comments on commit 06d9df4

Please sign in to comment.