From 6a98b18fb2a0b375d4ad2ab2238571b73cecb83a Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 26 Mar 2020 16:02:06 +0900 Subject: [PATCH] bdev/compress: Fix base channel is freed by wrong thread when freeing 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 #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 Change-Id: If4983c2e06d7b0b7618f38fb80f3aa73effe4b83 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1426 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Paul Luse --- module/bdev/compress/vbdev_compress.c | 33 +++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/module/bdev/compress/vbdev_compress.c b/module/bdev/compress/vbdev_compress.c index 9c6845c2f..9997652cc 100644 --- a/module/bdev/compress/vbdev_compress.c +++ b/module/bdev/compress/vbdev_compress.c @@ -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) @@ -979,12 +995,19 @@ delete_vol_unload_cb(void *cb_arg, int reduce_errno) if (reduce_errno) { SPDK_ERRLOG("number %d\n", reduce_errno); - } else { - /* reducelib needs a channel to comm with the backing device */ - comp_bdev->base_ch = spdk_bdev_get_io_channel(comp_bdev->base_desc); + /* FIXME: callback should be executed. */ + return; + } - /* Clean the device before we free our resources. */ - spdk_reduce_vol_destroy(&comp_bdev->backing_dev, _reduce_destroy_cb, comp_bdev); + 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 { + pthread_mutex_unlock(&comp_bdev->reduce_lock); + + _delete_vol_unload_cb(comp_bdev); } }