From 7e6a239646a73fdc258ea4f931df0d2474a2cb67 Mon Sep 17 00:00:00 2001 From: Shiqing Gao Date: Wed, 6 Sep 2023 00:58:03 +0800 Subject: [PATCH] dm: improve the flexibility of the iothread support Prior to this patch, one single iothread instance is created and initialized in the `main` function. This single iothread monitors all the registered fds and handles all the corresponding requests. It leads to the limited flexibility of the iothread support. To improve the flexibility of the iothread support, this patch does: - add the support of multiple iothread instances. `iothread_create` is introduced to create a certain number of iothread instances. It shall be called at first by each virtual device owner (such as virtio-blk BE) on initialization phase. Then, `iothread_add` can be called to add the to be monitored fd to the specified iothread. - update virtio-blk BE to let the acrn-dm option `iothread` accept a number as the number of iothread instances to be created. If `iothread` is contained in the parameters, but the number is not specified, one iothread instance would be created by default. Examples to specify the number of iothread instances: 1. Create 2 iothread instances `add_virtual_device 9 virtio-blk iothread=2,mq=2,/dev/nvme1n1,writeback,aio=io_uring` 2. Create 1 iothread instances (by default) `add_virtual_device 9 virtio-blk iothread,mq=2,/dev/nvme1n1,writeback,aio=io_uring` - update virtio-blk BE to separate the request handling of different virtqueues to different iothreads. The request from one or more virtqueues can be handled in one iothread. The mapping between virtqueues and iothreads is based on round robin. v1 -> v2: * add a mutex to protect the free ioctx slot allocation Tracked-On: #8612 Signed-off-by: Shiqing Gao Acked-by: Wang, Yu1 --- devicemodel/core/iothread.c | 154 +++++++++++++++-------- devicemodel/core/main.c | 7 -- devicemodel/hw/block_if.c | 14 ++- devicemodel/hw/pci/ahci.c | 2 +- devicemodel/hw/pci/virtio/virtio.c | 4 +- devicemodel/hw/pci/virtio/virtio_block.c | 63 +++++++++- devicemodel/include/block_if.h | 5 +- devicemodel/include/iothread.h | 22 +++- devicemodel/include/virtio.h | 1 + 9 files changed, 200 insertions(+), 72 deletions(-) diff --git a/devicemodel/core/iothread.c b/devicemodel/core/iothread.c index 8d94861d3..1079653f0 100644 --- a/devicemodel/core/iothread.c +++ b/devicemodel/core/iothread.c @@ -22,13 +22,10 @@ #define MEVENT_MAX 64 -struct iothread_ctx { - pthread_t tid; - int epfd; - bool started; - pthread_mutex_t mtx; -}; -static struct iothread_ctx ioctx; +static struct iothread_ctx ioctxes[IOTHREAD_NUM]; +static int ioctx_active_cnt; +/* mutex to protect the free ioctx slot allocation */ +static pthread_mutex_t ioctxes_mutex = PTHREAD_MUTEX_INITIALIZER; static void * io_thread(void *arg) @@ -36,9 +33,10 @@ io_thread(void *arg) struct epoll_event eventlist[MEVENT_MAX]; struct iothread_mevent *aevp; int i, n; + struct iothread_ctx *ioctx_x = (struct iothread_ctx *)arg; - while(ioctx.started) { - n = epoll_wait(ioctx.epfd, eventlist, MEVENT_MAX, -1); + while(ioctx_x->started) { + n = epoll_wait(ioctx_x->epfd, eventlist, MEVENT_MAX, -1); if (n < 0) { if (errno == EINTR) { /* EINTR may happen when io_uring fd is monitored, it is harmless. */ @@ -60,36 +58,46 @@ io_thread(void *arg) } static int -iothread_start(void) +iothread_start(struct iothread_ctx *ioctx_x) { - pthread_mutex_lock(&ioctx.mtx); + char tname[MAXCOMLEN + 1]; + pthread_mutex_lock(&ioctx_x->mtx); - if (ioctx.started) { - pthread_mutex_unlock(&ioctx.mtx); + if (ioctx_x->started) { + pthread_mutex_unlock(&ioctx_x->mtx); return 0; } - if (pthread_create(&ioctx.tid, NULL, io_thread, NULL) != 0) { - pthread_mutex_unlock(&ioctx.mtx); + if (pthread_create(&ioctx_x->tid, NULL, io_thread, ioctx_x) != 0) { + pthread_mutex_unlock(&ioctx_x->mtx); pr_err("%s", "iothread create failed\r\n"); return -1; } - ioctx.started = true; - pthread_setname_np(ioctx.tid, "iothread"); - pthread_mutex_unlock(&ioctx.mtx); - pr_info("iothread started\n"); + + ioctx_x->started = true; + snprintf(tname, sizeof(tname), "iothread_%d", ioctx_x->idx); + pthread_setname_np(ioctx_x->tid, tname); + pthread_mutex_unlock(&ioctx_x->mtx); + pr_info("iothread_%d started\n", ioctx_x->idx); + return 0; } int -iothread_add(int fd, struct iothread_mevent *aevt) +iothread_add(struct iothread_ctx *ioctx_x, int fd, struct iothread_mevent *aevt) { struct epoll_event ee; int ret; + + if (ioctx_x == NULL) { + pr_err("%s: ioctx_x is NULL \n", __func__); + return -1; + } + /* Create a epoll instance before the first fd is added.*/ ee.events = EPOLLIN; ee.data.ptr = aevt; - ret = epoll_ctl(ioctx.epfd, EPOLL_CTL_ADD, fd, &ee); + ret = epoll_ctl(ioctx_x->epfd, EPOLL_CTL_ADD, fd, &ee); if (ret < 0) { pr_err("%s: failed to add fd, error is %d\n", __func__, errno); @@ -97,7 +105,7 @@ iothread_add(int fd, struct iothread_mevent *aevt) } /* Start the iothread after the first fd is added.*/ - ret = iothread_start(); + ret = iothread_start(ioctx_x); if (ret < 0) { pr_err("%s: failed to start iothread thread\n", __func__); @@ -106,12 +114,17 @@ iothread_add(int fd, struct iothread_mevent *aevt) } int -iothread_del(int fd) +iothread_del(struct iothread_ctx *ioctx_x, int fd) { int ret = 0; - if (ioctx.epfd) { - ret = epoll_ctl(ioctx.epfd, EPOLL_CTL_DEL, fd, NULL); + if (ioctx_x == NULL) { + pr_err("%s: ioctx_x is NULL \n", __func__); + return -1; + } + + if (ioctx_x->epfd) { + ret = epoll_ctl(ioctx_x->epfd, EPOLL_CTL_DEL, fd, NULL); if (ret < 0) pr_err("%s: failed to delete fd from epoll fd, error is %d\n", __func__, errno); @@ -123,40 +136,79 @@ void iothread_deinit(void) { void *jval; + int i; + struct iothread_ctx *ioctx_x; - if (ioctx.tid > 0) { - pthread_mutex_lock(&ioctx.mtx); - ioctx.started = false; - pthread_mutex_unlock(&ioctx.mtx); - pthread_kill(ioctx.tid, SIGCONT); - pthread_join(ioctx.tid, &jval); + pthread_mutex_lock(&ioctxes_mutex); + for (i = 0; i < ioctx_active_cnt; i++) { + ioctx_x = &ioctxes[i]; + + if (ioctx_x->tid > 0) { + pthread_mutex_lock(&ioctx_x->mtx); + ioctx_x->started = false; + pthread_mutex_unlock(&ioctx_x->mtx); + pthread_kill(ioctx_x->tid, SIGCONT); + pthread_join(ioctx_x->tid, &jval); + } + if (ioctx_x->epfd > 0) { + close(ioctx_x->epfd); + ioctx_x->epfd = -1; + } + pthread_mutex_destroy(&ioctx_x->mtx); + pr_info("iothread_%d stop \n", i); } - if (ioctx.epfd > 0) { - close(ioctx.epfd); - ioctx.epfd = -1; - } - pthread_mutex_destroy(&ioctx.mtx); - pr_info("iothread stop\n"); + ioctx_active_cnt = 0; + pthread_mutex_unlock(&ioctxes_mutex); } -int -iothread_init(void) +/* + * Create @ioctx_num iothread context instances + * Return NULL if fails. Otherwise, return the base of those iothread context instances. + */ +struct iothread_ctx * +iothread_create(int ioctx_num) { pthread_mutexattr_t attr; + int i, ret, base, end; + struct iothread_ctx *ioctx_x; + struct iothread_ctx *ioctx_base = NULL; + ret = 0; - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&ioctx.mtx, &attr); - pthread_mutexattr_destroy(&attr); + pthread_mutex_lock(&ioctxes_mutex); + base = ioctx_active_cnt; + end = base + ioctx_num; - ioctx.tid = 0; - ioctx.started = false; - ioctx.epfd = epoll_create1(0); + if (end > IOTHREAD_NUM) { + ret = -1; + pr_err("%s: fails to create new iothread context, max number of instances is %d \n", + __func__, IOTHREAD_NUM); + } else { + for (i = base; i < end; i++) { + ioctx_x = &ioctxes[i]; - if (ioctx.epfd < 0) { - pr_err("%s: failed to create epoll fd, error is %d\r\n", - __func__, errno); - return -1; + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&(ioctx_x->mtx), &attr); + pthread_mutexattr_destroy(&attr); + + ioctx_x->idx = i; + ioctx_x->tid = 0; + ioctx_x->started = false; + ioctx_x->epfd = epoll_create1(0); + + if (ioctx_x->epfd < 0) { + ret = -1; + pr_err("%s: failed to create epoll fd, error is %d\r\n", + __func__, errno); + break; + } + } + if (ret == 0) { + ioctx_base = &ioctxes[base]; + ioctx_active_cnt = end; + } } - return 0; + pthread_mutex_unlock(&ioctxes_mutex); + + return ioctx_base; } diff --git a/devicemodel/core/main.c b/devicemodel/core/main.c index 334c4896b..8781def26 100644 --- a/devicemodel/core/main.c +++ b/devicemodel/core/main.c @@ -1139,12 +1139,6 @@ main(int argc, char *argv[]) goto mevent_fail; } - error = iothread_init(); - if (error) { - pr_err("Unable to initialize iothread (%d)\n", errno); - goto iothread_fail; - } - pr_notice("vm_init_vdevs\n"); if (vm_init_vdevs(ctx) < 0) { pr_err("Unable to init vdev (%d)\n", errno); @@ -1232,7 +1226,6 @@ vm_fail: dev_fail: iothread_deinit(); -iothread_fail: mevent_deinit(); mevent_fail: vm_unsetup_memory(ctx); diff --git a/devicemodel/hw/block_if.c b/devicemodel/hw/block_if.c index d0cd93749..a3c9841d3 100644 --- a/devicemodel/hw/block_if.c +++ b/devicemodel/hw/block_if.c @@ -121,6 +121,7 @@ struct blockif_queue { int in_flight; struct io_uring ring; struct iothread_mevent iomvt; + struct iothread_ctx *ioctx; struct blockif_ctxt *bc; }; @@ -739,7 +740,7 @@ iou_set_iothread(struct blockif_queue *bq) bq->iomvt.run = iou_completion_cb; bq->iomvt.fd = fd; - ret = iothread_add(fd, &bq->iomvt); + ret = iothread_add(bq->ioctx, fd, &bq->iomvt); if (ret < 0) { pr_err("%s: iothread_add fails, error %d \n", __func__, ret); } @@ -752,7 +753,7 @@ iou_del_iothread(struct blockif_queue *bq) int fd = bq->ring.ring_fd; int ret = 0; - ret = iothread_del(fd); + ret = iothread_del(bq->ioctx, fd); if (ret < 0) { pr_err("%s: iothread_del fails, error %d \n", __func__, ret); } @@ -810,7 +811,7 @@ static struct blockif_ops blockif_ops_iou = { }; struct blockif_ctxt * -blockif_open(const char *optstr, const char *ident, int queue_num) +blockif_open(const char *optstr, const char *ident, int queue_num, struct iothreads_info *iothrds_info) { char tag[MAXCOMLEN + 1]; char *nopt, *xopts, *cp; @@ -1097,6 +1098,13 @@ blockif_open(const char *optstr, const char *ident, int queue_num) struct blockif_queue *bq = bc->bqs + j; bq->bc = bc; + + if ((iothrds_info != NULL) && (iothrds_info->ioctx_base != NULL) && (iothrds_info->num != 0)) { + bq->ioctx = iothrds_info->ioctx_base + j % iothrds_info->num; + } else { + bq->ioctx = NULL; + } + pthread_mutex_init(&bq->mtx, NULL); pthread_cond_init(&bq->cond, NULL); TAILQ_INIT(&bq->freeq); diff --git a/devicemodel/hw/pci/ahci.c b/devicemodel/hw/pci/ahci.c index 78da1cd39..900c42977 100644 --- a/devicemodel/hw/pci/ahci.c +++ b/devicemodel/hw/pci/ahci.c @@ -2405,7 +2405,7 @@ pci_ahci_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts, int atapi) */ snprintf(bident, sizeof(bident), "%02x:%02x:%02x", dev->slot, dev->func, p); - bctxt = blockif_open(opts, bident, 1); + bctxt = blockif_open(opts, bident, 1, NULL); if (bctxt == NULL) { ahci_dev->ports = p; ret = 1; diff --git a/devicemodel/hw/pci/virtio/virtio.c b/devicemodel/hw/pci/virtio/virtio.c index 434c65131..b232fe59c 100644 --- a/devicemodel/hw/pci/virtio/virtio.c +++ b/devicemodel/hw/pci/virtio/virtio.c @@ -112,12 +112,12 @@ virtio_set_iothread(struct virtio_base *base, vq->viothrd.iomvt.run = iothread_handler; vq->viothrd.iomvt.fd = vq->viothrd.kick_fd; - if (!iothread_add(vq->viothrd.kick_fd, &vq->viothrd.iomvt)) + if (!iothread_add(vq->viothrd.ioctx, vq->viothrd.kick_fd, &vq->viothrd.iomvt)) if (!virtio_register_ioeventfd(base, idx, true, vq->viothrd.kick_fd)) vq->viothrd.ioevent_started = true; } else { if (!virtio_register_ioeventfd(base, idx, false, vq->viothrd.kick_fd)) - if (!iothread_del(vq->viothrd.kick_fd)) { + if (!iothread_del(vq->viothrd.ioctx, vq->viothrd.kick_fd)) { vq->viothrd.ioevent_started = false; if (vq->viothrd.kick_fd) { close(vq->viothrd.kick_fd); diff --git a/devicemodel/hw/pci/virtio/virtio_block.c b/devicemodel/hw/pci/virtio/virtio_block.c index 3a85d16e5..0deb79aba 100644 --- a/devicemodel/hw/pci/virtio/virtio_block.c +++ b/devicemodel/hw/pci/virtio/virtio_block.c @@ -164,6 +164,7 @@ struct virtio_blk { struct virtio_blk_ioreq *ios; uint8_t original_wce; int num_vqs; + struct iothreads_info iothrds_info; }; static void virtio_blk_reset(void *); @@ -475,7 +476,9 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) u_char digest[16]; struct virtio_blk *blk; bool use_iothread; - int num_vqs; + struct iothread_ctx *ioctx_base = NULL; + struct iothreads_info iothrds_info; + int num_vqs, num_iothread; int i, j; pthread_mutexattr_t attr; int rc; @@ -486,6 +489,11 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) use_iothread = false; num_vqs = 1; + /* + * Create one iothread instance if DM parameters contain 'iothread', but the number is not specified. + */ + num_iothread = 1; + if (opts == NULL) { pr_err("virtio_blk: backing device required\n"); return -1; @@ -518,8 +526,29 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) char *p = opts_start; while (opts_tmp != NULL) { opt = strsep(&opts_tmp, ","); - if (strcmp("iothread", opt) == 0) { + /* + * Valid 'iothread' setting examples: + * - create 1 iothread instance for virtio-blk + * ... virtio-blk iothread,... + * + * - create 1 iothread instance for virtio-blk + * ... virtio-blk iothread=1,... + * + * - create 3 iothread instances for virtio-blk + * ... virtio-blk iothread=3,... + */ + if (!strncmp(opt, "iothread", strlen("iothread"))) { use_iothread = true; + strsep(&opt, "="); + if (opt != NULL) { + if (dm_strtoi(opt, &opt, 10, &num_iothread) || + (num_iothread <= 0)) { + WPRINTF(("%s: incorrect iothread number %s\n", + __func__, opt)); + free(opts_start); + return -1; + } + } p = opts_tmp; } else if (!strncmp(opt, "mq", strlen("mq"))) { strsep(&opt, "="); @@ -545,7 +574,27 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) break; } } - bctxt = blockif_open(p, bident, num_vqs); + + if (use_iothread) { + /* + * Creating more iothread instances than the number of virtqueues is not necessary. + * - One or more vqs can be handled in one iothread. + * - The mapping between virtqueues and iothreads is based on round robin. + */ + if (num_iothread > num_vqs) { + num_iothread = num_vqs; + } + + ioctx_base = iothread_create(num_iothread); + if (ioctx_base == NULL) { + pr_err("%s: Fails to create iothread context instance \n", __func__); + return -1; + } + } + iothrds_info.ioctx_base = ioctx_base; + iothrds_info.num = num_iothread; + + bctxt = blockif_open(p, bident, num_vqs, &iothrds_info); if (bctxt == NULL) { pr_err("Could not open backing file"); free(opts_start); @@ -564,6 +613,9 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) return -1; } + blk->iothrds_info.ioctx_base = ioctx_base; + blk->iothrds_info.num = num_iothread; + blk->bc = bctxt; /* Update virtio-blk device struct of dummy ctxt*/ blk->dummy_bctxt = dummy_bctxt; @@ -619,6 +671,9 @@ virtio_blk_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts) for (j = 0; j < num_vqs; j++) { blk->vqs[j].qsize = VIRTIO_BLK_RINGSZ; blk->vqs[j].notify = virtio_blk_notify; + if (use_iothread) { + blk->vqs[j].viothrd.ioctx = ioctx_base + j % num_iothread; + } } /* @@ -804,7 +859,7 @@ virtio_blk_rescan(struct vmctx *ctx, struct pci_vdev *dev, char *newpath) pr_err("name=%s, Path=%s, ident=%s\n", dev->name, newpath, bident); /* update the bctxt for the virtio-blk device */ - bctxt = blockif_open(newpath, bident, blk->num_vqs); + bctxt = blockif_open(newpath, bident, blk->num_vqs, &blk->iothrds_info); if (bctxt == NULL) { pr_err("Error opening backing file\n"); goto end; diff --git a/devicemodel/include/block_if.h b/devicemodel/include/block_if.h index 9475f57a7..ffa2efe4b 100644 --- a/devicemodel/include/block_if.h +++ b/devicemodel/include/block_if.h @@ -39,6 +39,8 @@ #include #include +#include "iothread.h" + #define BLOCKIF_IOV_MAX 256 /* not practical to be IOV_MAX */ struct blockif_req { @@ -52,7 +54,8 @@ struct blockif_req { }; struct blockif_ctxt; -struct blockif_ctxt *blockif_open(const char *optstr, const char *ident, int queue_num); +struct blockif_ctxt *blockif_open(const char *optstr, const char *ident, int queue_num, + struct iothreads_info *iothrds_info); off_t blockif_size(struct blockif_ctxt *bc); void blockif_chs(struct blockif_ctxt *bc, uint16_t *c, uint8_t *h, uint8_t *s); diff --git a/devicemodel/include/iothread.h b/devicemodel/include/iothread.h index fb76e9baa..415030d30 100644 --- a/devicemodel/include/iothread.h +++ b/devicemodel/include/iothread.h @@ -7,14 +7,30 @@ #ifndef _iothread_CTX_H_ #define _iothread_CTX_H_ +#define IOTHREAD_NUM 40 + struct iothread_mevent { void (*run)(void *); void *arg; int fd; }; -int iothread_add(int fd, struct iothread_mevent *aevt); -int iothread_del(int fd); -int iothread_init(void); + +struct iothread_ctx { + pthread_t tid; + int epfd; + bool started; + pthread_mutex_t mtx; + int idx; +}; + +struct iothreads_info { + struct iothread_ctx *ioctx_base; + int num; +}; + +int iothread_add(struct iothread_ctx *ioctx_x, int fd, struct iothread_mevent *aevt); +int iothread_del(struct iothread_ctx *ioctx_x, int fd); void iothread_deinit(void); +struct iothread_ctx *iothread_create(int ioctx_num); #endif diff --git a/devicemodel/include/virtio.h b/devicemodel/include/virtio.h index 718b84e5d..33870ec25 100644 --- a/devicemodel/include/virtio.h +++ b/devicemodel/include/virtio.h @@ -427,6 +427,7 @@ struct virtio_iothread { int idx; int kick_fd; bool ioevent_started; + struct iothread_ctx *ioctx; struct iothread_mevent iomvt; void (*iothread_run)(void *, struct virtio_vq_info *); };