--- fix.diff --- # DF-2940: mpipe teardown while dm-crypt I/O in flight # # Part 1 (sys/kern/kern_mpipe.c): mpipe_done() must not free the pipe # while buffers are still checked out, and must let queued allocation # retries complete instead of orphaning them. # Part 2 (sys/dev/disk/dm/crypt/dm_target_crypt.c): the only in-tree # teardown caller must wait for its in-flight bios to return their # mpipe buffers before starting teardown (this is the part that was # build+run validated on the guest: every dmtc_destroy entry saw # rd free=20/total=20 wr free=20/total=20 after the wait, where the # baseline repeatedly saw wr free=8..19/total=20). diff --git a/sys/kern/kern_mpipe.c b/sys/kern/kern_mpipe.c index 1111111..2222222 100644 --- a/sys/kern/kern_mpipe.c +++ b/sys/kern/kern_mpipe.c @@ -122,17 +122,37 @@ mpipe_thread(void *arg) /* * Destroy a previously initialized mpipe. This routine can also safely be * called on an uninitialized mpipe structure if it was zero'd or mpipe_done() * was previously called on it. */ void mpipe_done(malloc_pipe_t mpipe) { void *buf; + struct mpipe_callback *mcb; int n; + /* + * DF-2940: wait for buffers that are still checked out to be + * returned by in-flight users before freeing anything. Callers + * must guarantee no new allocations are started before calling + * mpipe_done(); in-flight frees are what we are waiting for. + */ + for (n = 0; n < 600 && mpipe->free_count != mpipe->total_count; ++n) + tsleep(mpipe, 0, "mpipedn", 1); + KKASSERT(mpipe->free_count == mpipe->total_count); /* no outstanding mem */ /* * Clean up the kthread */ lwkt_gettoken(&mpipe->token); + /* + * Let the support thread run queued allocation retries to + * completion (free_count == total_count guarantees the retries + * can obtain buffers), instead of orphaning the parked bios. + */ + while (STAILQ_FIRST(&mpipe->queue) != NULL) { + wakeup(&mpipe->queue); + lwkt_reltoken(&mpipe->token); + tsleep(&mpipe->queue, 0, "mpipedq", 1); + lwkt_gettoken(&mpipe->token); + } mpipe->mpflags |= MPF_EXITING; while (mpipe->thread) { wakeup(&mpipe->queue); tsleep(mpipe, 0, "mpipex", 1); } diff --git a/sys/dev/disk/dm/crypt/dm_target_crypt.c b/sys/dev/disk/dm/crypt/dm_target_crypt.c index 3333333..4444444 100644 --- a/sys/dev/disk/dm/crypt/dm_target_crypt.c +++ b/sys/dev/disk/dm/crypt/dm_target_crypt.c @@ -200,10 +200,32 @@ dmtc_zero_mpipe_buffer(void *buffer, void *priv __unused) explicit_bzero(buffer, DMTC_BUF_SIZE); } +/* + * DF-2940: wait until every in-flight crypt bio has returned its mpipe + * buffer and no allocation retry is still parked in the mpipe queue. + * + * The dm framework releases the table reference as soon as the target's + * strategy routine returns, so bios issued through this table can still + * be executing the encrypt/decrypt path when the table is destroyed + * (DM_DEV_REMOVE, or suspend/load/resume table switch). Without this + * wait, mpipe_done() is entered with buffers checked out (observed + * 1..12 outstanding on the reproducer), tripping its KKASSERT on + * INVARIANTS kernels and freeing live buffers otherwise. + */ +static void +dmtc_wait_mpipe_drain(struct malloc_pipe *mpipe) +{ + int n; + + for (n = 0; n < 600; ++n) { /* up to ~60 seconds */ + lwkt_gettoken(&mpipe->token); + if (mpipe->free_count == mpipe->total_count && + STAILQ_FIRST(&mpipe->queue) == NULL) { + lwkt_reltoken(&mpipe->token); + return; + } + lwkt_reltoken(&mpipe->token); + tsleep(mpipe, 0, "dmtcdrn", hz / 10 + 1); + } + kprintf("dm_target_crypt: mpipe drain timeout, tearing down anyway\n"); +} + static void dmtc_init_mpipe(struct target_crypt_config *priv) { @@ -605,6 +627,10 @@ dm_target_crypt_destroy(dm_table_entry_t *table_en) priv = table_en->target_config; if (priv == NULL) return 0; + + dmtc_wait_mpipe_drain(&priv->read_mpipe); + dmtc_wait_mpipe_drain(&priv->write_mpipe); + dm_pdev_decr(priv->pdev); dmtc_destroy_mpipe(priv);