Skip to content

Commit

Permalink
Workaround issue cleaning up automounted snapshots on Linux
Browse files Browse the repository at this point in the history
On Linux, sometimes, when ZFS goes to unmount an automounted snap,
it fails a VERIFY check on debug builds, because taskq_cancel_id
returned ENOENT after not finding the taskq it was trying to cancel.

This presumably happens when it already died for some reason; in this
case, we don't really mind it already being dead, since we're just
going to dispatch a new task to unmount it right after.

So we just ignore it if we get back ENOENT trying to cancel here,
retry a couple times if we get back the only other possible condition
(EBUSY), and log to dbgmsg if we got anything but ENOENT or success.

(We also add some locking around taskqid, to avoid one or two cases
of two instances of trying to cancel something at once.)

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes #11632
Closes #12670
  • Loading branch information
rincebrain authored and behlendorf committed Aug 25, 2023
1 parent 245850b commit 692f780
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions module/os/linux/zfs/zfs_ctldir.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ typedef struct {
spa_t *se_spa; /* pool spa */
uint64_t se_objsetid; /* snapshot objset id */
struct dentry *se_root_dentry; /* snapshot root dentry */
krwlock_t se_taskqid_lock; /* scheduled unmount taskqid lock */
taskqid_t se_taskqid; /* scheduled unmount taskqid */
avl_node_t se_node_name; /* zfs_snapshots_by_name link */
avl_node_t se_node_objsetid; /* zfs_snapshots_by_objsetid link */
Expand All @@ -144,6 +145,7 @@ zfsctl_snapshot_alloc(const char *full_name, const char *full_path, spa_t *spa,
se->se_objsetid = objsetid;
se->se_root_dentry = root_dentry;
se->se_taskqid = TASKQID_INVALID;
rw_init(&se->se_taskqid_lock, NULL, RW_DEFAULT, NULL);

zfs_refcount_create(&se->se_refcount);

Expand All @@ -160,6 +162,7 @@ zfsctl_snapshot_free(zfs_snapentry_t *se)
zfs_refcount_destroy(&se->se_refcount);
kmem_strfree(se->se_name);
kmem_strfree(se->se_path);
rw_destroy(se->se_taskqid_lock);

kmem_free(se, sizeof (zfs_snapentry_t));
}
Expand Down Expand Up @@ -335,7 +338,9 @@ snapentry_expire(void *data)
return;
}

rw_enter(&se->se_taskqid_lock, RW_WRITER);
se->se_taskqid = TASKQID_INVALID;
rw_exit(&se->se_taskqid_lock);
(void) zfsctl_snapshot_unmount(se->se_name, MNT_EXPIRE);
zfsctl_snapshot_rele(se);

Expand All @@ -359,8 +364,18 @@ snapentry_expire(void *data)
static void
zfsctl_snapshot_unmount_cancel(zfs_snapentry_t *se)
{
if (taskq_cancel_id(system_delay_taskq, se->se_taskqid) == 0) {
se->se_taskqid = TASKQID_INVALID;
int err = 0;
rw_enter(&se->se_taskqid_lock, RW_WRITER);
err = taskq_cancel_id(system_delay_taskq, se->se_taskqid);
/*
* if we get ENOENT, the taskq couldn't be found to be
* canceled, so we can just mark it as invalid because
* it's already gone. If we got EBUSY, then we already
* blocked until it was gone _anyway_, so we don't care.
*/
se->se_taskqid = TASKQID_INVALID;
rw_exit(&se->se_taskqid_lock);
if (err == 0) {
zfsctl_snapshot_rele(se);
}
}
Expand All @@ -371,14 +386,16 @@ zfsctl_snapshot_unmount_cancel(zfs_snapentry_t *se)
static void
zfsctl_snapshot_unmount_delay_impl(zfs_snapentry_t *se, int delay)
{
ASSERT3S(se->se_taskqid, ==, TASKQID_INVALID);

if (delay <= 0)
return;

zfsctl_snapshot_hold(se);
rw_enter(&se->se_taskqid_lock, RW_WRITER);
ASSERT3S(se->se_taskqid, ==, TASKQID_INVALID);
se->se_taskqid = taskq_dispatch_delay(system_delay_taskq,
snapentry_expire, se, TQ_SLEEP, ddi_get_lbolt() + delay * HZ);
rw_exit(&se->se_taskqid_lock);
}

/*
Expand Down

0 comments on commit 692f780

Please sign in to comment.