ZIL: allow zil_commit() to fail with error

This changes zil_commit() to have an int return, and updates all callers
to check it. There are no corresponding internal changes yet; it will
always return 0.

Since zil_commit() is an indication that the caller _really_ wants the
associated data to be durability stored, I've annotated it with the
__warn_unused_result__ compiler attribute (via __must_check), to emit a
warning if it's ever ussd without doing something with the return code.
I hope this will mean we never misuse it in the future.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #17398
This commit is contained in:
Rob Norris 2025-02-24 15:14:23 +11:00 committed by Brian Behlendorf
parent 1f8c39ddb2
commit 967b15b888
16 changed files with 147 additions and 85 deletions

View File

@ -2965,7 +2965,7 @@ ztest_zil_commit(ztest_ds_t *zd, uint64_t id)
(void) pthread_rwlock_rdlock(&zd->zd_zilog_lock);
zil_commit(zilog, ztest_random(ZTEST_OBJECTS));
VERIFY0(zil_commit(zilog, ztest_random(ZTEST_OBJECTS)));
/*
* Remember the committed values in zd, which is in parent/child
@ -7936,7 +7936,7 @@ ztest_freeze(void)
*/
while (BP_IS_HOLE(&zd->zd_zilog->zl_header->zh_log)) {
ztest_dmu_object_alloc_free(zd, 0);
zil_commit(zd->zd_zilog, 0);
VERIFY0(zil_commit(zd->zd_zilog, 0));
}
txg_wait_synced(spa_get_dsl(spa), 0);
@ -7978,7 +7978,7 @@ ztest_freeze(void)
/*
* Commit all of the changes we just generated.
*/
zil_commit(zd->zd_zilog, 0);
VERIFY0(zil_commit(zd->zd_zilog, 0));
txg_wait_synced(spa_get_dsl(spa), 0);
/*

View File

@ -69,6 +69,10 @@
#define __maybe_unused __attribute__((unused))
#endif
#ifndef __must_check
#define __must_check __attribute__((__warn_unused_result__))
#endif
/*
* Without this, we see warnings from objtool during normal Linux builds when
* the kernel is built with CONFIG_STACK_VALIDATION=y:

View File

@ -69,6 +69,10 @@
#define __maybe_unused __attribute__((unused))
#endif
#ifndef __must_check
#define __must_check __attribute__((__warn_unused_result__))
#endif
/*
* Without this, we see warnings from objtool during normal Linux builds when
* the kernel is built with CONFIG_STACK_VALIDATION=y:

View File

@ -610,8 +610,7 @@ extern void zil_itx_destroy(itx_t *itx);
extern void zil_itx_assign(zilog_t *zilog, itx_t *itx, dmu_tx_t *tx);
extern void zil_async_to_sync(zilog_t *zilog, uint64_t oid);
extern void zil_commit(zilog_t *zilog, uint64_t oid);
extern void zil_commit_impl(zilog_t *zilog, uint64_t oid);
extern int __must_check zil_commit(zilog_t *zilog, uint64_t oid);
extern void zil_remove_async(zilog_t *zilog, uint64_t oid);
extern int zil_reset(const char *osname, void *txarg);

View File

@ -38,4 +38,8 @@
#define __maybe_unused __attribute__((unused))
#endif
#ifndef __must_check
#define __must_check __attribute__((warn_unused_result))
#endif
#endif

View File

@ -455,8 +455,13 @@ zfs_sync(vfs_t *vfsp, int waitfor)
return (0);
}
if (zfsvfs->z_log != NULL)
zil_commit(zfsvfs->z_log, 0);
if (zfsvfs->z_log != NULL) {
error = zil_commit(zfsvfs->z_log, 0);
if (error != 0) {
zfs_exit(zfsvfs, FTAG);
return (error);
}
}
zfs_exit(zfsvfs, FTAG);
} else {

View File

@ -1193,8 +1193,8 @@ out:
*zpp = zp;
}
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1323,9 +1323,8 @@ out:
if (xzp)
vrele(ZTOV(xzp));
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1556,8 +1555,8 @@ out:
getnewvnode_drop_reserve();
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1637,8 +1636,8 @@ zfs_rmdir_(vnode_t *dvp, vnode_t *vp, const char *name, cred_t *cr)
if (zfsvfs->z_use_namecache)
cache_vop_rmdir(dvp, vp);
out:
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -3009,8 +3008,8 @@ out:
}
out2:
if (os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (err == 0 && os->os_sync == ZFS_SYNC_ALWAYS)
err = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (err);
@ -3539,7 +3538,7 @@ out_seq:
out:
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -3731,7 +3730,7 @@ zfs_symlink(znode_t *dzp, const char *name, vattr_t *vap,
*zpp = zp;
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
error = zil_commit(zilog, 0);
}
zfs_exit(zfsvfs, FTAG);
@ -3921,8 +3920,8 @@ zfs_link(znode_t *tdzp, znode_t *szp, const char *name, cred_t *cr,
vnevent_link(ZTOV(szp), ct);
}
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -4510,8 +4509,13 @@ zfs_putpages(struct vnode *vp, vm_page_t *ma, size_t len, int flags,
out:
zfs_rangelock_exit(lr);
if (commit)
zil_commit(zfsvfs->z_log, zp->z_id);
if (commit) {
err = zil_commit(zfsvfs->z_log, zp->z_id);
if (err != 0) {
zfs_exit(zfsvfs, FTAG);
return (err);
}
}
dataset_kstats_update_write_kstats(&zfsvfs->z_kstat, len);
@ -6773,9 +6777,11 @@ zfs_deallocate(struct vop_deallocate_args *ap)
if (error == 0) {
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS ||
(ap->a_ioflag & IO_SYNC) != 0)
zil_commit(zilog, zp->z_id);
*ap->a_offset = off + len;
*ap->a_len = 0;
error = zil_commit(zilog, zp->z_id);
if (error == 0) {
*ap->a_offset = off + len;
*ap->a_len = 0;
}
}
zfs_exit(zfsvfs, FTAG);

View File

@ -727,9 +727,9 @@ unlock:
break;
}
if (commit) {
if (error == 0 && commit) {
commit:
zil_commit(zv->zv_zilog, ZVOL_OBJ);
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
}
resume:
rw_exit(&zv->zv_suspend_lock);
@ -906,8 +906,8 @@ zvol_cdev_write(struct cdev *dev, struct uio *uio_s, int ioflag)
zfs_rangelock_exit(lr);
int64_t nwritten = start_resid - zfs_uio_resid(&uio);
dataset_kstats_update_write_kstats(&zv->zv_kstat, nwritten);
if (commit)
zil_commit(zv->zv_zilog, ZVOL_OBJ);
if (error == 0 && commit)
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
rw_exit(&zv->zv_suspend_lock);
return (error);
@ -1117,7 +1117,7 @@ zvol_cdev_ioctl(struct cdev *dev, ulong_t cmd, caddr_t data,
case DIOCGFLUSH:
rw_enter(&zv->zv_suspend_lock, ZVOL_RW_READER);
if (zv->zv_zilog != NULL)
zil_commit(zv->zv_zilog, ZVOL_OBJ);
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
rw_exit(&zv->zv_suspend_lock);
break;
case DIOCGDELETE:
@ -1152,7 +1152,7 @@ zvol_cdev_ioctl(struct cdev *dev, ulong_t cmd, caddr_t data,
}
zfs_rangelock_exit(lr);
if (sync)
zil_commit(zv->zv_zilog, ZVOL_OBJ);
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
rw_exit(&zv->zv_suspend_lock);
break;
case DIOCGSTRIPESIZE:

View File

@ -288,10 +288,10 @@ zfs_sync(struct super_block *sb, int wait, cred_t *cr)
return (SET_ERROR(EIO));
}
zil_commit(zfsvfs->z_log, 0);
err = zil_commit(zfsvfs->z_log, 0);
zfs_exit(zfsvfs, FTAG);
return (0);
return (err);
}
static void

View File

@ -841,8 +841,8 @@ out:
*zpp = zp;
}
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1203,8 +1203,8 @@ out:
zfs_zrele_async(xzp);
}
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1392,14 +1392,15 @@ out:
zfs_dirent_unlock(dl);
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error != 0) {
zrele(zp);
} else {
zfs_znode_update_vfs(dzp);
zfs_znode_update_vfs(zp);
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
}
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1528,8 +1529,8 @@ out:
zfs_znode_update_vfs(zp);
zrele(zp);
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -2630,8 +2631,8 @@ out:
}
out2:
if (os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (err == 0 && os->os_sync == ZFS_SYNC_ALWAYS)
err = zil_commit(zilog, 0);
out3:
kmem_free(xattr_bulk, sizeof (sa_bulk_attr_t) * bulks);
@ -3235,8 +3236,8 @@ out:
zfs_dirent_unlock(sdl);
zfs_dirent_unlock(tdl);
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -3436,7 +3437,7 @@ top:
*zpp = zp;
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
error = zil_commit(zilog, 0);
} else {
zrele(zp);
}
@ -3670,18 +3671,20 @@ top:
zfs_dirent_unlock(dl);
if (!is_tmpfile && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0) {
if (!is_tmpfile && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
if (is_tmpfile && zfsvfs->z_os->os_sync != ZFS_SYNC_DISABLED) {
txg_wait_flag_t wait_flags =
spa_get_failmode(dmu_objset_spa(zfsvfs->z_os)) ==
ZIO_FAILURE_MODE_CONTINUE ? TXG_WAIT_SUSPEND : 0;
error = txg_wait_synced_flags(dmu_objset_pool(zfsvfs->z_os),
txg, wait_flags);
if (error != 0) {
ASSERT3U(error, ==, ESHUTDOWN);
error = SET_ERROR(EIO);
if (is_tmpfile && zfsvfs->z_os->os_sync != ZFS_SYNC_DISABLED) {
txg_wait_flag_t wait_flags =
spa_get_failmode(dmu_objset_spa(zfsvfs->z_os)) ==
ZIO_FAILURE_MODE_CONTINUE ? TXG_WAIT_SUSPEND : 0;
error = txg_wait_synced_flags(
dmu_objset_pool(zfsvfs->z_os), txg, wait_flags);
if (error != 0) {
ASSERT3U(error, ==, ESHUTDOWN);
error = SET_ERROR(EIO);
}
}
}
@ -3939,8 +3942,13 @@ zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc,
zfs_rangelock_exit(lr);
if (need_commit)
zil_commit(zfsvfs->z_log, zp->z_id);
if (need_commit) {
err = zil_commit(zfsvfs->z_log, zp->z_id);
if (err != 0) {
zfs_exit(zfsvfs, FTAG);
return (err);
}
}
dataset_kstats_update_write_kstats(&zfsvfs->z_kstat, pglen);

View File

@ -22,6 +22,7 @@
/*
* Copyright (c) 2011, Lawrence Livermore National Security, LLC.
* Copyright (c) 2015 by Chunwei Chen. All rights reserved.
* Copyright (c) 2025, Klara, Inc.
*/
@ -495,9 +496,18 @@ zpl_writepages(struct address_space *mapping, struct writeback_control *wbc)
if ((result = zpl_enter_verify_zp(zfsvfs, zp, FTAG)) != 0)
return (result);
if (zfsvfs->z_log != NULL)
zil_commit(zfsvfs->z_log, zp->z_id);
result = -zil_commit(zfsvfs->z_log, zp->z_id);
zpl_exit(zfsvfs, FTAG);
/*
* If zil_commit() failed, it's unclear what state things
* are currently in. putpage() has written back out what
* it can to the DMU, but it may not be on disk. We have
* little choice but to escape.
*/
if (result != 0)
return (result);
/*
* We need to call write_cache_pages() again (we can't just
* return after the commit) because the previous call in

View File

@ -208,8 +208,14 @@ zvol_write(zv_request_t *zvr)
disk = zv->zv_zso->zvo_disk;
/* bio marked as FLUSH need to flush before write */
if (io_is_flush(bio, rq))
zil_commit(zv->zv_zilog, ZVOL_OBJ);
if (io_is_flush(bio, rq)) {
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
if (error != 0) {
rw_exit(&zv->zv_suspend_lock);
zvol_end_io(bio, rq, -error);
return;
}
}
/* Some requests are just for flush and nothing else. */
if (io_size(bio, rq) == 0) {
@ -273,8 +279,8 @@ zvol_write(zv_request_t *zvr)
dataset_kstats_update_write_kstats(&zv->zv_kstat, nwritten);
task_io_account_write(nwritten);
if (sync)
zil_commit(zv->zv_zilog, ZVOL_OBJ);
if (error == 0 && sync)
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
rw_exit(&zv->zv_suspend_lock);
@ -361,7 +367,7 @@ zvol_discard(zv_request_t *zvr)
zfs_rangelock_exit(lr);
if (error == 0 && sync)
zil_commit(zv->zv_zilog, ZVOL_OBJ);
error = zil_commit(zv->zv_zilog, ZVOL_OBJ);
unlock:
rw_exit(&zv->zv_suspend_lock);

View File

@ -286,7 +286,7 @@ zfs_sa_set_xattr(znode_t *zp, const char *name, const void *value, size_t vsize)
dmu_tx_commit(tx);
if (logsaxattr && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
error = zil_commit(zilog, 0);
}
out_free:
vmem_free(obj, size);

View File

@ -27,6 +27,7 @@
* Copyright 2017 Nexenta Systems, Inc.
* Copyright (c) 2021, 2022 by Pawel Jakub Dawidek
* Copyright (c) 2025, Rob Norris <robn@despairlabs.com>
* Copyright (c) 2025, Klara, Inc.
*/
/* Portions Copyright 2007 Jeremy Teo */
@ -116,7 +117,7 @@ zfs_fsync(znode_t *zp, int syncflag, cred_t *cr)
if (zfsvfs->z_os->os_sync != ZFS_SYNC_DISABLED) {
if ((error = zfs_enter_verify_zp(zfsvfs, zp, FTAG)) != 0)
return (error);
zil_commit(zfsvfs->z_log, zp->z_id);
error = zil_commit(zfsvfs->z_log, zp->z_id);
zfs_exit(zfsvfs, FTAG);
}
return (error);
@ -375,8 +376,13 @@ zfs_read(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
frsync = !!(ioflag & FRSYNC);
#endif
if (zfsvfs->z_log &&
(frsync || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS))
zil_commit(zfsvfs->z_log, zp->z_id);
(frsync || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)) {
error = zil_commit(zfsvfs->z_log, zp->z_id);
if (error != 0) {
zfs_exit(zfsvfs, FTAG);
return (error);
}
}
/*
* Lock the range against changes.
@ -1074,8 +1080,13 @@ zfs_write(znode_t *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
return (error);
}
if (commit)
zil_commit(zilog, zp->z_id);
if (commit) {
error = zil_commit(zilog, zp->z_id);
if (error != 0) {
zfs_exit(zfsvfs, FTAG);
return (error);
}
}
int64_t nwritten = start_resid - zfs_uio_resid(uio);
dataset_kstats_update_write_kstats(&zfsvfs->z_kstat, nwritten);
@ -1260,8 +1271,8 @@ zfs_setsecattr(znode_t *zp, vsecattr_t *vsecp, int flag, cred_t *cr)
zilog = zfsvfs->z_log;
error = zfs_setacl(zp, vsecp, skipaclchk, cr);
if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
zil_commit(zilog, 0);
if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
error = zil_commit(zilog, 0);
zfs_exit(zfsvfs, FTAG);
return (error);
@ -1946,7 +1957,7 @@ unlock:
ZFS_ACCESSTIME_STAMP(inzfsvfs, inzp);
if (outos->os_sync == ZFS_SYNC_ALWAYS) {
zil_commit(zilog, outzp->z_id);
error = zil_commit(zilog, outzp->z_id);
}
*inoffp += done;

View File

@ -3640,7 +3640,9 @@ zil_commit_itx_assign(zilog_t *zilog, zil_commit_waiter_t *zcw)
* but the order in which they complete will be the same order in
* which they were created.
*/
void
static int zil_commit_impl(zilog_t *zilog, uint64_t foid);
int
zil_commit(zilog_t *zilog, uint64_t foid)
{
/*
@ -3659,7 +3661,7 @@ zil_commit(zilog_t *zilog, uint64_t foid)
ASSERT3B(dmu_objset_is_snapshot(zilog->zl_os), ==, B_FALSE);
if (zilog->zl_sync == ZFS_SYNC_DISABLED)
return;
return (0);
if (!spa_writeable(zilog->zl_spa)) {
/*
@ -3673,7 +3675,7 @@ zil_commit(zilog_t *zilog, uint64_t foid)
ASSERT0P(zilog->zl_last_lwb_opened);
for (int i = 0; i < TXG_SIZE; i++)
ASSERT0P(zilog->zl_itxg[i].itxg_itxs);
return;
return (0);
}
/*
@ -3686,13 +3688,13 @@ zil_commit(zilog_t *zilog, uint64_t foid)
if (zilog->zl_suspend > 0) {
ZIL_STAT_BUMP(zilog, zil_commit_suspend_count);
txg_wait_synced(zilog->zl_dmu_pool, 0);
return;
return (0);
}
zil_commit_impl(zilog, foid);
return (zil_commit_impl(zilog, foid));
}
void
static int
zil_commit_impl(zilog_t *zilog, uint64_t foid)
{
ZIL_STAT_BUMP(zilog, zil_commit_count);
@ -3748,6 +3750,8 @@ zil_commit_impl(zilog_t *zilog, uint64_t foid)
}
zil_free_commit_waiter(zcw);
return (0);
}
/*
@ -4025,7 +4029,8 @@ zil_close(zilog_t *zilog)
uint64_t txg;
if (!dmu_objset_is_snapshot(zilog->zl_os)) {
zil_commit(zilog, 0);
if (zil_commit(zilog, 0) != 0)
txg_wait_synced(zilog->zl_dmu_pool, 0);
} else {
ASSERT(list_is_empty(&zilog->zl_lwb_list));
ASSERT0(zilog->zl_dirty_max_txg);

View File

@ -772,7 +772,7 @@ zvol_clone_range(zvol_state_t *zv_src, uint64_t inoff, zvol_state_t *zv_dst,
zfs_rangelock_exit(outlr);
zfs_rangelock_exit(inlr);
if (error == 0 && zv_dst->zv_objset->os_sync == ZFS_SYNC_ALWAYS) {
zil_commit(zilog_dst, ZVOL_OBJ);
error = zil_commit(zilog_dst, ZVOL_OBJ);
}
out:
if (zv_src != zv_dst)