From aa4b4e17f3e1bf44330dff2727ff456000b931ed Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Mon, 28 Dec 2020 23:00:51 +0800 Subject: [PATCH] nbd: Continue revising the nbd subsystem fini logic After carefully checking the code, spdk_nbd_stop should be called in the same spdk thread which creates the io_channel. Usually, the spdk thread which handles the rpc call should be same with the thread which finalizes the thread. But it could be different. So adding another async call to make sure we should call spdk_nbd_stop on the same spdk thread. Signed-off-by: Ziye Yang Change-Id: I276eb35e78b930d31869f10137712a78aaee71ed Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5705 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Paul Luse Reviewed-by: Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto --- lib/nbd/nbd.c | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/nbd/nbd.c b/lib/nbd/nbd.c index c957637df..195b0b1e9 100644 --- a/lib/nbd/nbd.c +++ b/lib/nbd/nbd.c @@ -123,6 +123,8 @@ static struct spdk_nbd_disk_globals g_spdk_nbd; static spdk_nbd_fini_cb g_fini_cb_fn; static void *g_fini_cb_arg; +static void _nbd_fini(void *arg1); + static int nbd_submit_bdev_io(struct spdk_nbd_disk *nbd, struct nbd_io *io); @@ -134,29 +136,38 @@ spdk_nbd_init(void) return 0; } +static void +_nbd_stop_async(void *arg) +{ + struct spdk_nbd_disk *nbd = arg; + int rc; + + rc = spdk_nbd_stop(nbd); + if (rc) { + /* spdk_nbd_stop failed because some IO are still executing. Send a message + * to this thread to try again later. */ + spdk_thread_send_msg(spdk_get_thread(), + _nbd_stop_async, nbd); + } else { + _nbd_fini(NULL); + } +} + static void _nbd_fini(void *arg1) { - struct spdk_nbd_disk *nbd_idx, *nbd_tmp; - int rc = 0; + struct spdk_nbd_disk *nbd_first; - /* - * Stop running spdk_nbd_disk. - * Here, nbd removing are unnecessary, but _SAFE variant - * is needed, since internal nbd_disk_unregister will - * remove nbd from TAILQ. - */ - TAILQ_FOREACH_SAFE(nbd_idx, &g_spdk_nbd.disk_head, tailq, nbd_tmp) { - rc = spdk_nbd_stop(nbd_idx); - if (rc) { - break; - } - } - - if (!rc) { - g_fini_cb_fn(g_fini_cb_arg); + nbd_first = TAILQ_FIRST(&g_spdk_nbd.disk_head); + if (nbd_first) { + /* Stop running spdk_nbd_disk */ + spdk_thread_send_msg(spdk_io_channel_get_thread(nbd_first->ch), + _nbd_stop_async, nbd_first); } else { - spdk_thread_send_msg(spdk_get_thread(), _nbd_fini, NULL); + /* We can directly call final function here, because + spdk_subsystem_fini_next handles the case: current thread does not equal + to g_final_thread */ + g_fini_cb_fn(g_fini_cb_arg); } }