Skip to content

Commit

Permalink
bdev/compress: Fix base channel is freed by wrong thread when freeing…
Browse files Browse the repository at this point in the history
… comp channel

When reduce volume is unloaded at compress bdev removal, the callback
to unload did not get base bdev's I/O channel on the same thread
that opened base bdev.

Hence spdk_put_io_channel() hit assert later.

This patch fixes the bug by sending message to get I/O channel.

Fixes issue spdk#1307.

Besides, add assert to _delete_vol_unload_cb() to detect the unexpected
complex cases. Fix will be added later.

Signed-off-by: Shuhei Matsumoto <[email protected]>
Change-Id: If4983c2e06d7b0b7618f38fb80f3aa73effe4b83
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1426
Tested-by: SPDK CI Jenkins <[email protected]>
Reviewed-by: Ben Walker <[email protected]>
Reviewed-by: Jim Harris <[email protected]>
Reviewed-by: Paul Luse <[email protected]>
  • Loading branch information
Shuhei Matsumoto authored and tomzawadzki committed Apr 3, 2020
1 parent fcf8e45 commit 6a98b18
Showing 1 changed file with 27 additions and 4 deletions.
31 changes: 27 additions & 4 deletions module/bdev/compress/vbdev_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,22 @@ _reduce_destroy_cb(void *ctx, int reduce_errno)

}

static void
_delete_vol_unload_cb(void *ctx)
{
struct vbdev_compress *comp_bdev = ctx;

/* FIXME: Assert if these conditions are not satisified for now. */
assert(!comp_bdev->reduce_thread ||
comp_bdev->reduce_thread == spdk_get_thread());

/* reducelib needs a channel to comm with the backing device */
comp_bdev->base_ch = spdk_bdev_get_io_channel(comp_bdev->base_desc);

/* Clean the device before we free our resources. */
spdk_reduce_vol_destroy(&comp_bdev->backing_dev, _reduce_destroy_cb, comp_bdev);
}

/* Called by reduceLib after performing unload vol actions */
static void
delete_vol_unload_cb(void *cb_arg, int reduce_errno)
Expand All @@ -979,12 +995,19 @@ delete_vol_unload_cb(void *cb_arg, int reduce_errno)

if (reduce_errno) {
SPDK_ERRLOG("number %d\n", reduce_errno);
/* FIXME: callback should be executed. */
return;
}

pthread_mutex_lock(&comp_bdev->reduce_lock);
if (comp_bdev->reduce_thread && comp_bdev->reduce_thread != spdk_get_thread()) {
spdk_thread_send_msg(comp_bdev->reduce_thread,
_delete_vol_unload_cb, comp_bdev);
pthread_mutex_unlock(&comp_bdev->reduce_lock);
} else {
/* reducelib needs a channel to comm with the backing device */
comp_bdev->base_ch = spdk_bdev_get_io_channel(comp_bdev->base_desc);
pthread_mutex_unlock(&comp_bdev->reduce_lock);

/* Clean the device before we free our resources. */
spdk_reduce_vol_destroy(&comp_bdev->backing_dev, _reduce_destroy_cb, comp_bdev);
_delete_vol_unload_cb(comp_bdev);
}
}

Expand Down

0 comments on commit 6a98b18

Please sign in to comment.