From 22412af1d206882fa28628cb9e9ee6d0865d5bdc Mon Sep 17 00:00:00 2001 From: Liu Xiaodong Date: Fri, 5 Feb 2021 11:24:01 -0500 Subject: [PATCH] nbd: get nbd_stop procedure in async It is possible that nbd pthread is created but not executed, then spdk_nbd_stop is call before nbd_pthread's execution, but nbd pthread starts to execute while nbd is totally stopped. This patch can get spdk_stop_nbd aligned with nbd pthread. Change-Id: I57cc92b94d36cd706616c9058134f716f0812892 Signed-off-by: Liu Xiaodong Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6278 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu Reviewed-by: --- lib/nbd/nbd.c | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/nbd/nbd.c b/lib/nbd/nbd.c index 81d3826df..84b670407 100644 --- a/lib/nbd/nbd.c +++ b/lib/nbd/nbd.c @@ -50,6 +50,7 @@ #define GET_IO_LOOP_COUNT 16 #define NBD_START_BUSY_WAITING_MS 1000 +#define NBD_STOP_BUSY_WAITING_MS 10000 #define NBD_BUSY_POLLING_INTERVAL_US 20000 #define NBD_IO_TIMEOUT_S 60 @@ -108,6 +109,8 @@ struct spdk_nbd_disk { struct spdk_poller *retry_poller; int retry_count; + /* Synchronize nbd_start_kernel pthread and nbd_stop */ + bool has_nbd_pthread; struct nbd_io *io_in_recv; TAILQ_HEAD(, nbd_io) received_io_list; @@ -370,9 +373,11 @@ nbd_cleanup_io(struct spdk_nbd_disk *nbd) return 0; } -static void -_nbd_stop(struct spdk_nbd_disk *nbd) +static int +_nbd_stop(void *arg) { + struct spdk_nbd_disk *nbd = arg; + if (nbd->nbd_poller) { spdk_poller_unregister(&nbd->nbd_poller); } @@ -383,10 +388,32 @@ _nbd_stop(struct spdk_nbd_disk *nbd) if (nbd->spdk_sp_fd >= 0) { close(nbd->spdk_sp_fd); + nbd->spdk_sp_fd = -1; } if (nbd->kernel_sp_fd >= 0) { close(nbd->kernel_sp_fd); + nbd->kernel_sp_fd = -1; + } + + /* Continue the stop procedure after the exit of nbd_start_kernel pthread */ + if (nbd->has_nbd_pthread) { + if (nbd->retry_poller == NULL) { + nbd->retry_count = NBD_STOP_BUSY_WAITING_MS * 1000ULL / NBD_BUSY_POLLING_INTERVAL_US; + nbd->retry_poller = SPDK_POLLER_REGISTER(_nbd_stop, nbd, + NBD_BUSY_POLLING_INTERVAL_US); + return SPDK_POLLER_BUSY; + } + + if (nbd->retry_count-- > 0) { + return SPDK_POLLER_BUSY; + } + + SPDK_ERRLOG("Failed to wait for returning of NBD_DO_IT ioctl.\n"); + } + + if (nbd->retry_poller) { + spdk_poller_unregister(&nbd->retry_poller); } if (nbd->dev_fd >= 0) { @@ -404,15 +431,19 @@ _nbd_stop(struct spdk_nbd_disk *nbd) if (nbd->ch) { spdk_put_io_channel(nbd->ch); + nbd->ch = NULL; } if (nbd->bdev_desc) { spdk_bdev_close(nbd->bdev_desc); + nbd->bdev_desc = NULL; } nbd_disk_unregister(nbd); free(nbd); + + return 0; } int @@ -876,12 +907,14 @@ nbd_poll(void *arg) static void * nbd_start_kernel(void *arg) { - int dev_fd = (int)(intptr_t)arg; + struct spdk_nbd_disk *nbd = arg; spdk_unaffinitize_thread(); /* This will block in the kernel until we close the spdk_sp_fd. */ - ioctl(dev_fd, NBD_DO_IT); + ioctl(nbd->dev_fd, NBD_DO_IT); + + nbd->has_nbd_pthread = false; pthread_exit(NULL); } @@ -960,8 +993,10 @@ nbd_start_complete(struct spdk_nbd_start_ctx *ctx) } } - rc = pthread_create(&tid, NULL, nbd_start_kernel, (void *)(intptr_t)ctx->nbd->dev_fd); + ctx->nbd->has_nbd_pthread = true; + rc = pthread_create(&tid, NULL, nbd_start_kernel, ctx->nbd); if (rc != 0) { + ctx->nbd->has_nbd_pthread = false; SPDK_ERRLOG("could not create thread: %s\n", spdk_strerror(rc)); rc = -rc; goto err;