> On Feb 6, 2025, at 16:41, Maxime Coquelin <maxime.coque...@redhat.com> wrote:
>
> External email: Use caution opening links or attachments
>
>
> With the recent rework of the FD manager to use epoll,
> an error message is emitted with Vhost-user at FD entry
> removal:
>
> ERR|VHOST_FDMAN: could not remove 102 fd from 101 epfd: No such file or
> directory
>
> It occurs because the read callback closes the file
> descriptor before it is removed from the FD set.
> This patch defers the close() after the FD entry is removed
> from the set.
>
> Fixes: 0e38b42bf61c ("vhost: manage FD with epoll")
> Cc: sta...@dpdk.org
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> ---
> lib/vhost/fd_man.c | 12 +++++++-----
> lib/vhost/fd_man.h | 2 +-
> lib/vhost/socket.c | 11 +++++------
> lib/vhost/vduse.c | 9 ++++-----
> 4 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/lib/vhost/fd_man.c b/lib/vhost/fd_man.c
> index 9bc7e50b93..f9147edee7 100644
> --- a/lib/vhost/fd_man.c
> +++ b/lib/vhost/fd_man.c
> @@ -333,7 +333,7 @@ fdset_event_dispatch(void *arg)
> fd_cb rcb, wcb;
> void *dat;
> int fd, numfds;
> - int remove1, remove2;
> + int close1, close2;
> struct fdset *pfdset = arg;
>
> if (pfdset == NULL)
> @@ -357,7 +357,7 @@ fdset_event_dispatch(void *arg)
> continue;
> }
>
> - remove1 = remove2 = 0;
> + close1 = close2 = 0;
>
> rcb = pfdentry->rcb;
> wcb = pfdentry->wcb;
> @@ -367,9 +367,9 @@ fdset_event_dispatch(void *arg)
> pthread_mutex_unlock(&pfdset->fd_mutex);
>
> if (rcb && events[i].events & (EPOLLIN | EPOLLERR |
> EPOLLHUP))
> - rcb(fd, dat, &remove1);
> + rcb(fd, dat, &close1);
> if (wcb && events[i].events & (EPOLLOUT | EPOLLERR |
> EPOLLHUP))
> - wcb(fd, dat, &remove2);
> + wcb(fd, dat, &close2);
> pfdentry->busy = 0;
> /*
> * fdset_del needs to check busy flag.
> @@ -381,8 +381,10 @@ fdset_event_dispatch(void *arg)
> * fdentry not to be busy, so we can't call
> * fdset_del_locked().
> */
> - if (remove1 || remove2)
> + if (close1 || close2) {
> fdset_del(pfdset, fd);
> + close(fd);
> + }
> }
>
> if (pfdset->destroy)
> diff --git a/lib/vhost/fd_man.h b/lib/vhost/fd_man.h
> index 6398343a6a..eadcc6fb42 100644
> --- a/lib/vhost/fd_man.h
> +++ b/lib/vhost/fd_man.h
> @@ -12,7 +12,7 @@ struct fdset;
>
> #define MAX_FDS 1024
>
> -typedef void (*fd_cb)(int fd, void *dat, int *remove);
> +typedef void (*fd_cb)(int fd, void *dat, int *close);
>
> struct fdset *fdset_init(const char *name);
>
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index 433a42bf80..b27020ae8d 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -84,8 +84,8 @@ struct vhost_user {
>
> #define MAX_VIRTIO_BACKLOG 128
>
> -static void vhost_user_server_new_connection(int fd, void *data, int
> *remove);
> -static void vhost_user_read_cb(int fd, void *dat, int *remove);
> +static void vhost_user_server_new_connection(int fd, void *data, int *close);
> +static void vhost_user_read_cb(int fd, void *dat, int *close);
> static int create_unix_socket(struct vhost_user_socket *vsocket);
> static int vhost_user_start_client(struct vhost_user_socket *vsocket);
>
> @@ -290,7 +290,7 @@ vhost_user_add_connection(int fd, struct
> vhost_user_socket *vsocket)
>
> /* call back when there is new vhost-user connection from client */
> static void
> -vhost_user_server_new_connection(int fd, void *dat, int *remove __rte_unused)
> +vhost_user_server_new_connection(int fd, void *dat, int *close __rte_unused)
> {
> struct vhost_user_socket *vsocket = dat;
>
> @@ -303,7 +303,7 @@ vhost_user_server_new_connection(int fd, void *dat, int
> *remove __rte_unused)
> }
>
> static void
> -vhost_user_read_cb(int connfd, void *dat, int *remove)
> +vhost_user_read_cb(int connfd, void *dat, int *close)
> {
> struct vhost_user_connection *conn = dat;
> struct vhost_user_socket *vsocket = conn->vsocket;
> @@ -313,8 +313,7 @@ vhost_user_read_cb(int connfd, void *dat, int *remove)
> if (ret < 0) {
> struct virtio_net *dev = get_device(conn->vid);
>
> - close(connfd);
> - *remove = 1;
> + *close = 1;
>
> if (dev)
> vhost_destroy_device_notify(dev);
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index 8ba58555f9..9d39e71c68 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -117,7 +117,7 @@ static struct vhost_backend_ops vduse_backend_ops = {
> };
>
> static void
> -vduse_control_queue_event(int fd, void *arg, int *remove __rte_unused)
> +vduse_control_queue_event(int fd, void *arg, int *close __rte_unused)
> {
> struct virtio_net *dev = arg;
> uint64_t buf;
> @@ -350,7 +350,7 @@ vduse_device_stop(struct virtio_net *dev)
> }
>
> static void
> -vduse_events_handler(int fd, void *arg, int *remove __rte_unused)
> +vduse_events_handler(int fd, void *arg, int *close __rte_unused)
> {
> struct virtio_net *dev = arg;
> struct vduse_dev_request req;
> @@ -563,14 +563,13 @@ vduse_reconnect_log_check(struct virtio_net *dev,
> uint64_t features, uint32_t to
> }
>
> static void
> -vduse_reconnect_handler(int fd, void *arg, int *remove)
> +vduse_reconnect_handler(int fd __rte_unused, void *arg, int *close)
> {
> struct virtio_net *dev = arg;
>
> vduse_device_start(dev, true);
>
> - close(fd);
> - *remove = 1;
> + *close = 1;
> }
>
> static int
> --
> 2.48.1
>
Reviewed-by: Chenbo Xia <chen...@nvidia.com>
