Skip to content

Fix inode eviction and sync writeback deadlock on Linux #17159

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.

Sign up for GitHub

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

Open
tuxoko wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix inode eviction and sync writeback deadlock on Linux #17159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion cmd/ztest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2487,7 +2487,7 @@ ztest_get_done(zgd_t *zgd, int error)
}

static int
ztest_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
ztest_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
struct lwb *lwb, zio_t *zio)
{
(void) arg2;
Expand Down
6 changes: 4 additions & 2 deletions include/sys/zil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ typedef enum {
} itx_wr_state_t;

typedef void (*zil_callback_t)(void *data, int err);
typedef void (*itx_zrele_t)(void *);

typedef struct itx {
list_node_t itx_node; /* linkage on zl_itx_list */
Expand All @@ -467,7 +468,8 @@ typedef struct itx {
void *itx_callback_data; /* User data for the callback */
size_t itx_size; /* allocated itx structure size */
uint64_t itx_oid; /* object id */
uint64_t itx_gen; /* gen number for zfs_get_data */
void *itx_znode; /* znode for zfs_get_data */
itx_zrele_t itx_zrele; /* cleanup for itx_znode */
lr_t itx_lr; /* common part of log record */
uint8_t itx_lr_data[]; /* type-specific part of lr_xx_t */
} itx_t;
Expand Down Expand Up @@ -605,7 +607,7 @@ typedef int zil_parse_blk_func_t(zilog_t *zilog, const blkptr_t *bp, void *arg,
typedef int zil_parse_lr_func_t(zilog_t *zilog, const lr_t *lr, void *arg,
uint64_t txg);
typedef int zil_replay_func_t(void *arg1, void *arg2, boolean_t byteswap);
typedef int zil_get_data_t(void *arg, uint64_t arg2, lr_write_t *lr, char *dbuf,
typedef int zil_get_data_t(void *arg, void *arg2, lr_write_t *lr, char *dbuf,
struct lwb *lwb, zio_t *zio);

extern int zil_parse(zilog_t *zilog, zil_parse_blk_func_t *parse_blk_func,
Expand Down
2 changes: 1 addition & 1 deletion include/sys/zvol_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void zvol_log_truncate(zvol_state_t *zv, dmu_tx_t *tx, uint64_t off,
uint64_t len);
void zvol_log_write(zvol_state_t *zv, dmu_tx_t *tx, uint64_t offset,
uint64_t size, boolean_t commit);
int zvol_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
int zvol_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
struct lwb *lwb, zio_t *zio);
int zvol_init_impl(void);
void zvol_fini_impl(void);
Expand Down
45 changes: 23 additions & 22 deletions module/os/freebsd/zfs/zfs_vfsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,29 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
znode_t *zp;
dsl_dir_t *dd;

ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);

if (!unmounting) {
/*
* We purge the parent filesystem's vfsp as the parent
* filesystem and all of its snapshots have their vnode's
* v_vfsp set to the parent's filesystem's vfsp. Note,
* 'z_parent' is self referential for non-snapshots.
*/
#ifdef FREEBSD_NAMECACHE
cache_purgevfs(zfsvfs->z_parent->z_vfs);
#endif
}

/*
* Close the zil. NB: Can't close the zil while zfs_inactive
* threads are blocked as zil_close can call zfs_inactive.
*/
if (zfsvfs->z_log) {
zil_close(zfsvfs->z_log);
zfsvfs->z_log = NULL;
}

/*
* If someone has not already unmounted this file system,
* drain the zrele_taskq to ensure all active references to the
Expand All @@ -1624,28 +1647,6 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
break;
}
}
ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);

if (!unmounting) {
/*
* We purge the parent filesystem's vfsp as the parent
* filesystem and all of its snapshots have their vnode's
* v_vfsp set to the parent's filesystem's vfsp. Note,
* 'z_parent' is self referential for non-snapshots.
*/
#ifdef FREEBSD_NAMECACHE
cache_purgevfs(zfsvfs->z_parent->z_vfs);
#endif
}

/*
* Close the zil. NB: Can't close the zil while zfs_inactive
* threads are blocked as zil_close can call zfs_inactive.
*/
if (zfsvfs->z_log) {
zil_close(zfsvfs->z_log);
zfsvfs->z_log = NULL;
}

ZFS_TEARDOWN_INACTIVE_ENTER_WRITE(zfsvfs);

Expand Down
44 changes: 22 additions & 22 deletions module/os/linux/zfs/zfs_vfsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,28 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)

zfs_unlinked_drain_stop_wait(zfsvfs);

ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);

if (!unmounting) {
/*
* We purge the parent filesystem's super block as the
* parent filesystem and all of its snapshots have their
* inode's super block set to the parent's filesystem's
* super block. Note, 'z_parent' is self referential
* for non-snapshots.
*/
shrink_dcache_sb(zfsvfs->z_parent->z_sb);
}

/*
* Close the zil. NB: Can't close the zil while zfs_inactive
* threads are blocked as zil_close can call zfs_inactive.
*/
if (zfsvfs->z_log) {
zil_close(zfsvfs->z_log);
zfsvfs->z_log = NULL;
}

/*
* If someone has not already unmounted this file system,
* drain the zrele_taskq to ensure all active references to the
Copy link
Contributor Author

@tuxoko tuxoko Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

znode won't be freed until all zil itxs are cleaned up. That's why I move the taskq wait after zil_close. But I'm not sure if this is the right thing to do...

Expand Down Expand Up @@ -1376,28 +1398,6 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
}
}

ZFS_TEARDOWN_ENTER_WRITE(zfsvfs, FTAG);

if (!unmounting) {
/*
* We purge the parent filesystem's super block as the
* parent filesystem and all of its snapshots have their
* inode's super block set to the parent's filesystem's
* super block. Note, 'z_parent' is self referential
* for non-snapshots.
*/
shrink_dcache_sb(zfsvfs->z_parent->z_sb);
}

/*
* Close the zil. NB: Can't close the zil while zfs_inactive
* threads are blocked as zil_close can call zfs_inactive.
*/
if (zfsvfs->z_log) {
zil_close(zfsvfs->z_log);
zfsvfs->z_log = NULL;
}

rw_enter(&zfsvfs->z_teardown_inactive_lock, RW_WRITER);

/*
Expand Down
10 changes: 5 additions & 5 deletions module/zfs/zfs_log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <sys/spa.h>
#include <sys/zfs_fuid.h>
#include <sys/dsl_dataset.h>
#include <sys/zfs_vnops.h>

/*
* These zfs_log_* functions must be called within a dmu tx, in one
Expand Down Expand Up @@ -615,7 +616,7 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,
dmu_buf_impl_t *db = (dmu_buf_impl_t *)sa_get_db(zp->z_sa_hdl);
uint32_t blocksize = zp->z_blksz;
itx_wr_state_t write_state;
uint64_t gen = 0, log_size = 0;
uint64_t log_size = 0;

if (zil_replaying(zilog, tx) || zp->z_unlinked ||
zfs_xattr_owner_unlinked(zp)) {
Expand All @@ -627,9 +628,6 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,
write_state = zil_write_state(zilog, resid, blocksize, o_direct,
commit);

(void) sa_lookup(zp->z_sa_hdl, SA_ZPL_GEN(ZTOZSB(zp)), &gen,
sizeof (gen));

while (resid) {
itx_t *itx;
lr_write_t *lr;
Expand Down Expand Up @@ -683,7 +681,9 @@ zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,

itx->itx_private = ZTOZSB(zp);
itx->itx_sync = (zp->z_sync_cnt != 0);
itx->itx_gen = gen;
zhold(zp);
itx->itx_znode = zp;
itx->itx_zrele = (itx_zrele_t)zfs_zrele_async;

if (resid == len) {
itx->itx_callback = callback;
Expand Down
32 changes: 7 additions & 25 deletions module/zfs/zfs_vnops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,45 +1326,27 @@ static void zfs_get_done(zgd_t *zgd, int error);
* Get data to generate a TX_WRITE intent log record.
*/
int
zfs_get_data(void *arg, uint64_t gen, lr_write_t *lr, char *buf,
zfs_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
struct lwb *lwb, zio_t *zio)
{
zfsvfs_t *zfsvfs = arg;
objset_t *os = zfsvfs->z_os;
znode_t *zp;
znode_t *zp = arg2;
uint64_t object = lr->lr_foid;
uint64_t offset = lr->lr_offset;
uint64_t size = lr->lr_length;
zgd_t *zgd;
int error = 0;
uint64_t zp_gen;

ASSERT3P(zp, !=, NULL);
ASSERT3P(lwb, !=, NULL);
ASSERT3U(size, !=, 0);

/*
* Nothing to do if the file has been removed
*/
if (zfs_zget(zfsvfs, object, &zp) != 0)
return (SET_ERROR(ENOENT));
if (zp->z_unlinked) {
/*
* Release the vnode asynchronously as we currently have the
* txg stopped from syncing.
*/
zfs_zrele_async(zp);
if (zp->z_unlinked)
return (SET_ERROR(ENOENT));
}
/* check if generation number matches */
if (sa_lookup(zp->z_sa_hdl, SA_ZPL_GEN(zfsvfs), &zp_gen,
sizeof (zp_gen)) != 0) {
zfs_zrele_async(zp);
return (SET_ERROR(EIO));
}
if (zp_gen != gen) {
zfs_zrele_async(zp);
return (SET_ERROR(ENOENT));
}

/* Hold zp ref for ourselves */
zhold(zp);

zgd = kmem_zalloc(sizeof (zgd_t), KM_SLEEP);
zgd->zgd_lwb = lwb;
Expand Down
8 changes: 7 additions & 1 deletion module/zfs/zil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2457,7 +2457,7 @@ zil_lwb_commit(zilog_t *zilog, lwb_t *lwb, itx_t *itx)
* writing completion before the vdev cache flushing.
*/
error = zilog->zl_get_data(itx->itx_private,
itx->itx_gen, lrwb, dbuf, lwb,
itx->itx_znode, lrwb, dbuf, lwb,
lwb->lwb_child_zio);
if (dbuf != NULL && error == 0) {
/* Zero any padding bytes in the last block. */
Expand Down Expand Up @@ -2565,6 +2565,7 @@ zil_itx_clone(itx_t *oitx)
memcpy(itx, oitx, oitx->itx_size);
itx->itx_callback = NULL;
itx->itx_callback_data = NULL;
itx->itx_zrele = NULL;
return (itx);
}

Expand All @@ -2580,6 +2581,11 @@ zil_itx_destroy(itx_t *itx, int err)
if (itx->itx_callback != NULL)
itx->itx_callback(itx->itx_callback_data, err);

if (itx->itx_zrele) {
ASSERT3P(itx->itx_znode, !=, NULL);
itx->itx_zrele(itx->itx_znode);
}

zio_data_buf_free(itx, itx->itx_size);
}

Expand Down
2 changes: 1 addition & 1 deletion module/zfs/zvol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ zvol_get_done(zgd_t *zgd, int error)
* Get data to generate a TX_WRITE intent log record.
*/
int
zvol_get_data(void *arg, uint64_t arg2, lr_write_t *lr, char *buf,
zvol_get_data(void *arg, void *arg2, lr_write_t *lr, char *buf,
struct lwb *lwb, zio_t *zio)
{
zvol_state_t *zv = arg;
Expand Down
Loading