From patchwork Thu Feb 29 12:24:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maxime Coquelin X-Patchwork-Id: 137482 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 00CA843C36; Thu, 29 Feb 2024 13:25:29 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4A73242D95; Thu, 29 Feb 2024 13:25:16 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 41000427E3 for ; Thu, 29 Feb 2024 13:25:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709209513; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SpHAD+sEUkONSYBX2n/Fr7MTwj+jPzpXhuPbvGo0StU=; b=jFzOb6Aw/0Pi+lt8eUXbpOIsbrTvtE+IO+ZAaMeKWGXbl1MOlJCSCjM5wC1SG6Wd+BcTJ1 GxRugv8Z3BBEO5ZmzWtJV9c2fGKFLakrZPvmIhMpBFVrf8hGhi5CaXFCjD+KjvXUNASlhS u5axEbufE0TfOc9maneB4uoSnavI8vA= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-68-UuMLBIgxNOucC_5Gfa9p9A-1; Thu, 29 Feb 2024 07:25:12 -0500 X-MC-Unique: UuMLBIgxNOucC_5Gfa9p9A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 192753C0FC8C; Thu, 29 Feb 2024 12:25:12 +0000 (UTC) Received: from max-p1.redhat.com (unknown [10.39.208.20]) by smtp.corp.redhat.com (Postfix) with ESMTP id C9024112132A; Thu, 29 Feb 2024 12:25:10 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org, david.marchand@redhat.com, chenbox@nvidia.com Cc: Maxime Coquelin Subject: [PATCH 4/7] vhost: hide synchronization within FD manager Date: Thu, 29 Feb 2024 13:24:59 +0100 Message-ID: <20240229122502.2572343-5-maxime.coquelin@redhat.com> In-Reply-To: <20240229122502.2572343-1-maxime.coquelin@redhat.com> References: <20240229122502.2572343-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org This patch forces synchronization for all FDs additions or deletions in the FD set. With that, it is no more necessary for the user to know about the FD set pipe, so hide its initialization in the FD manager. Signed-off-by: Maxime Coquelin --- lib/vhost/fd_man.c | 174 +++++++++++++++++++++++++-------------------- lib/vhost/fd_man.h | 7 +- lib/vhost/socket.c | 12 +--- lib/vhost/vduse.c | 18 +---- 4 files changed, 101 insertions(+), 110 deletions(-) diff --git a/lib/vhost/fd_man.c b/lib/vhost/fd_man.c index d33036a171..0ae481b785 100644 --- a/lib/vhost/fd_man.c +++ b/lib/vhost/fd_man.c @@ -2,7 +2,9 @@ * Copyright(c) 2010-2014 Intel Corporation */ +#include #include +#include #include #include @@ -17,6 +19,87 @@ RTE_LOG_REGISTER_SUFFIX(vhost_fdset_logtype, fdset, INFO); #define FDPOLLERR (POLLERR | POLLHUP | POLLNVAL) +static void +fdset_pipe_read_cb(int readfd, void *dat, + int *remove __rte_unused) +{ + char charbuf[16]; + struct fdset *fdset = dat; + int r = read(readfd, charbuf, sizeof(charbuf)); + /* + * Just an optimization, we don't care if read() failed + * so ignore explicitly its return value to make the + * compiler happy + */ + RTE_SET_USED(r); + + pthread_mutex_lock(&fdset->sync_mutex); + fdset->sync = true; + pthread_cond_broadcast(&fdset->sync_cond); + pthread_mutex_unlock(&fdset->sync_mutex); +} + +static void +fdset_pipe_uninit(struct fdset *fdset) +{ + fdset_del(fdset, fdset->u.readfd); + close(fdset->u.readfd); + fdset->u.readfd = -1; + close(fdset->u.writefd); + fdset->u.writefd = -1; +} + +static int +fdset_pipe_init(struct fdset *fdset) +{ + int ret; + + pthread_mutex_init(&fdset->sync_mutex, NULL); + pthread_cond_init(&fdset->sync_cond, NULL); + + if (pipe(fdset->u.pipefd) < 0) { + VHOST_FDMAN_LOG(ERR, + "failed to create pipe for vhost fdset"); + return -1; + } + + ret = fdset_add(fdset, fdset->u.readfd, + fdset_pipe_read_cb, NULL, fdset); + if (ret < 0) { + VHOST_FDMAN_LOG(ERR, + "failed to add pipe readfd %d into vhost server fdset", + fdset->u.readfd); + + fdset_pipe_uninit(fdset); + return -1; + } + + return 0; +} + +static void +fdset_sync(struct fdset *fdset) +{ + int ret; + + pthread_mutex_lock(&fdset->sync_mutex); + + fdset->sync = false; + ret = write(fdset->u.writefd, "1", 1); + if (ret < 0) { + VHOST_FDMAN_LOG(ERR, + "Failed to write to notification pipe: %s", + strerror(errno)); + goto out_unlock; + } + + while (!fdset->sync) + pthread_cond_wait(&fdset->sync_cond, &fdset->sync_mutex); + +out_unlock: + pthread_mutex_unlock(&fdset->sync_mutex); +} + static int get_last_valid_idx(struct fdset *pfdset, int last_valid_idx) { @@ -96,6 +179,12 @@ fdset_add_fd(struct fdset *pfdset, int idx, int fd, pfd->revents = 0; } +void +fdset_uninit(struct fdset *pfdset) +{ + fdset_pipe_uninit(pfdset); +} + int fdset_init(struct fdset *pfdset) { @@ -113,7 +202,7 @@ fdset_init(struct fdset *pfdset) } pfdset->num = 0; - return 0; + return fdset_pipe_init(pfdset); } /** @@ -143,6 +232,8 @@ fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat) fdset_add_fd(pfdset, i, fd, rcb, wcb, dat); pthread_mutex_unlock(&pfdset->fd_mutex); + fdset_sync(pfdset); + return 0; } @@ -174,6 +265,8 @@ fdset_del(struct fdset *pfdset, int fd) pthread_mutex_unlock(&pfdset->fd_mutex); } while (i != -1); + fdset_sync(pfdset); + return dat; } @@ -207,6 +300,9 @@ fdset_try_del(struct fdset *pfdset, int fd) } pthread_mutex_unlock(&pfdset->fd_mutex); + + fdset_sync(pfdset); + return 0; } @@ -312,79 +408,3 @@ fdset_event_dispatch(void *arg) return 0; } - -static void -fdset_pipe_read_cb(int readfd, void *dat, - int *remove __rte_unused) -{ - char charbuf[16]; - struct fdset *fdset = dat; - int r = read(readfd, charbuf, sizeof(charbuf)); - /* - * Just an optimization, we don't care if read() failed - * so ignore explicitly its return value to make the - * compiler happy - */ - RTE_SET_USED(r); - - pthread_mutex_lock(&fdset->sync_mutex); - fdset->sync = true; - pthread_cond_broadcast(&fdset->sync_cond); - pthread_mutex_unlock(&fdset->sync_mutex); -} - -void -fdset_pipe_uninit(struct fdset *fdset) -{ - fdset_del(fdset, fdset->u.readfd); - close(fdset->u.readfd); - close(fdset->u.writefd); -} - -int -fdset_pipe_init(struct fdset *fdset) -{ - int ret; - - if (pipe(fdset->u.pipefd) < 0) { - VHOST_FDMAN_LOG(ERR, - "failed to create pipe for vhost fdset"); - return -1; - } - - ret = fdset_add(fdset, fdset->u.readfd, - fdset_pipe_read_cb, NULL, fdset); - - if (ret < 0) { - VHOST_FDMAN_LOG(ERR, - "failed to add pipe readfd %d into vhost server fdset", - fdset->u.readfd); - - fdset_pipe_uninit(fdset); - return -1; - } - - return 0; -} - -void -fdset_pipe_notify(struct fdset *fdset) -{ - int r; - - pthread_mutex_lock(&fdset->sync_mutex); - - fdset->sync = false; - r = write(fdset->u.writefd, "1", 1); - /* - * Just an optimization, we don't care if write() failed - * so ignore explicitly its return value to make the - * compiler happy - */ - RTE_SET_USED(r); - - while (!fdset->sync) - pthread_cond_wait(&fdset->sync_cond, &fdset->sync_mutex); - - pthread_mutex_unlock(&fdset->sync_mutex); -} diff --git a/lib/vhost/fd_man.h b/lib/vhost/fd_man.h index 92d24d8591..c18e3a435c 100644 --- a/lib/vhost/fd_man.h +++ b/lib/vhost/fd_man.h @@ -42,6 +42,7 @@ struct fdset { bool sync; }; +void fdset_uninit(struct fdset *pfdset); int fdset_init(struct fdset *pfdset); @@ -53,10 +54,4 @@ int fdset_try_del(struct fdset *pfdset, int fd); uint32_t fdset_event_dispatch(void *arg); -int fdset_pipe_init(struct fdset *fdset); - -void fdset_pipe_uninit(struct fdset *fdset); - -void fdset_pipe_notify(struct fdset *fdset); - #endif diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c index b544e39be7..2f93d48c31 100644 --- a/lib/vhost/socket.c +++ b/lib/vhost/socket.c @@ -278,7 +278,6 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) TAILQ_INSERT_TAIL(&vsocket->conn_list, conn, next); pthread_mutex_unlock(&vsocket->conn_mutex); - fdset_pipe_notify(&vhost_user.fdset); return; err_cleanup: @@ -1186,20 +1185,11 @@ rte_vhost_driver_start(const char *path) return -1; } - /** - * create a pipe which will be waited by poll and notified to - * rebuild the wait list of poll. - */ - if (fdset_pipe_init(&vhost_user.fdset) < 0) { - VHOST_CONFIG_LOG(path, ERR, "failed to create pipe for vhost fdset"); - return -1; - } - int ret = rte_thread_create_internal_control(&fdset_tid, "vhost-evt", fdset_event_dispatch, &vhost_user.fdset); if (ret != 0) { VHOST_CONFIG_LOG(path, ERR, "failed to create fdset handling thread"); - fdset_pipe_uninit(&vhost_user.fdset); + fdset_uninit(&vhost_user.fdset); return -1; } } diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c index d83d7b0d7c..257285a89f 100644 --- a/lib/vhost/vduse.c +++ b/lib/vhost/vduse.c @@ -225,7 +225,6 @@ vduse_vring_setup(struct virtio_net *dev, unsigned int index) close(vq->kickfd); vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD; } - fdset_pipe_notify(&vduse.fdset); vhost_enable_guest_notification(dev, vq, 1); VHOST_CONFIG_LOG(dev->ifname, INFO, "Ctrl queue event handler installed"); } @@ -238,10 +237,8 @@ vduse_vring_cleanup(struct virtio_net *dev, unsigned int index) struct vduse_vq_eventfd vq_efd; int ret; - if (vq == dev->cvq && vq->kickfd >= 0) { + if (vq == dev->cvq && vq->kickfd >= 0) fdset_del(&vduse.fdset, vq->kickfd); - fdset_pipe_notify(&vduse.fdset); - } vq_efd.index = index; vq_efd.fd = VDUSE_EVENTFD_DEASSIGN; @@ -432,20 +429,11 @@ vduse_device_create(const char *path, bool compliant_ol_flags) return -1; } - /** - * create a pipe which will be waited by poll and notified to - * rebuild the wait list of poll. - */ - if (fdset_pipe_init(&vduse.fdset) < 0) { - VHOST_CONFIG_LOG(path, ERR, "failed to create pipe for vduse fdset"); - return -1; - } - ret = rte_thread_create_internal_control(&fdset_tid, "vduse-evt", fdset_event_dispatch, &vduse.fdset); if (ret != 0) { VHOST_CONFIG_LOG(path, ERR, "failed to create vduse fdset handling thread"); - fdset_pipe_uninit(&vduse.fdset); + fdset_uninit(&vduse.fdset); return -1; } @@ -573,7 +561,6 @@ vduse_device_create(const char *path, bool compliant_ol_flags) dev->vduse_dev_fd); goto out_dev_destroy; } - fdset_pipe_notify(&vduse.fdset); free(dev_config); @@ -616,7 +603,6 @@ vduse_device_destroy(const char *path) vduse_device_stop(dev); fdset_del(&vduse.fdset, dev->vduse_dev_fd); - fdset_pipe_notify(&vduse.fdset); if (dev->vduse_dev_fd >= 0) { close(dev->vduse_dev_fd);