Re: [PATCH v3 1/6] util: Simplify epoll_del() interface to take epollfd directly

[Stefano Brivio <sbrivio@redhat.com>]

On Thu,  9 Oct 2025 15:04:04 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Change epoll_del() to accept the epoll file descriptor directly instead
> of the full context structure. This simplifies the interface and aligns
> with the threading refactoring by reducing dependency on the context
> structure for basic epoll operations as we will manage an epollfd per
> thread.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  icmp.c       | 2 +-
>  tap.c        | 2 +-
>  tcp.c        | 6 +++---
>  tcp_splice.c | 4 ++--
>  udp_flow.c   | 4 ++--
>  util.c       | 6 +++---
>  util.h       | 2 +-
>  vhost_user.c | 6 +++---
>  8 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/icmp.c b/icmp.c
> index 6dffafb0bf54..bd3108a21675 100644
> --- a/icmp.c
> +++ b/icmp.c
> @@ -151,7 +151,7 @@ unexpected:
>  static void icmp_ping_close(const struct ctx *c,
>  			    const struct icmp_ping_flow *pingf)
>  {
> -	epoll_del(c, pingf->sock);
> +	epoll_del(c->epollfd, pingf->sock);
>  	close(pingf->sock);
>  	flow_hash_remove(c, FLOW_SIDX(pingf, INISIDE));
>  }
> diff --git a/tap.c b/tap.c
> index 95d309bd1938..134c37a72979 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1142,7 +1142,7 @@ void tap_sock_reset(struct ctx *c)
>  	}
>  
>  	/* Close the connected socket, wait for a new connection */
> -	epoll_del(c, c->fd_tap);
> +	epoll_del(c->epollfd, c->fd_tap);
>  	close(c->fd_tap);
>  	c->fd_tap = -1;
>  	if (c->mode == MODE_VU)
> diff --git a/tcp.c b/tcp.c
> index 91aa33016f36..9100c67e57e0 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -511,9 +511,9 @@ static int tcp_epoll_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
>  
>  	if (conn->events == CLOSED) {
>  		if (conn->in_epoll)
> -			epoll_del(c, conn->sock);
> +			epoll_del(c->epollfd, conn->sock);
>  		if (conn->timer != -1)
> -			epoll_del(c, conn->timer);
> +			epoll_del(c->epollfd, conn->timer);
>  		return 0;
>  	}
>  
> @@ -3456,7 +3456,7 @@ int tcp_flow_migrate_source_ext(const struct ctx *c,
>  	if (c->migrate_no_linger)
>  		close(s);
>  	else
> -		epoll_del(c, s);
> +		epoll_del(c->epollfd, s);
>  
>  	/* Adjustments unrelated to FIN segments: sequence numbers we dumped are
>  	 * based on the end of the queues.
> diff --git a/tcp_splice.c b/tcp_splice.c
> index 26cb63064583..666ee62b738f 100644
> --- a/tcp_splice.c
> +++ b/tcp_splice.c
> @@ -204,8 +204,8 @@ static void conn_flag_do(const struct ctx *c, struct tcp_splice_conn *conn,
>  	}
>  
>  	if (flag == CLOSING) {
> -		epoll_del(c, conn->s[0]);
> -		epoll_del(c, conn->s[1]);
> +		epoll_del(c->epollfd, conn->s[0]);
> +		epoll_del(c->epollfd, conn->s[1]);
>  	}
>  }
>  
> diff --git a/udp_flow.c b/udp_flow.c
> index cef3fb588bbe..84973f807167 100644
> --- a/udp_flow.c
> +++ b/udp_flow.c
> @@ -51,7 +51,7 @@ void udp_flow_close(const struct ctx *c, struct udp_flow *uflow)
>  	flow_foreach_sidei(sidei) {
>  		flow_hash_remove(c, FLOW_SIDX(uflow, sidei));
>  		if (uflow->s[sidei] >= 0) {
> -			epoll_del(c, uflow->s[sidei]);
> +			epoll_del(c->epollfd, uflow->s[sidei]);
>  			close(uflow->s[sidei]);
>  			uflow->s[sidei] = -1;
>  		}
> @@ -88,7 +88,7 @@ static int udp_flow_sock(const struct ctx *c,
>  	if (flowside_connect(c, s, pif, side) < 0) {
>  		int rc = -errno;
>  
> -		epoll_del(c, s);
> +		epoll_del(c->epollfd, s);
>  		close(s);
>  
>  		flow_dbg_perror(uflow, "Couldn't connect flow socket");
> diff --git a/util.c b/util.c
> index c492f904b3fc..88a91b1100f5 100644
> --- a/util.c
> +++ b/util.c
> @@ -996,12 +996,12 @@ void raw_random(void *buf, size_t buflen)
>  
>  /**
>   * epoll_del() - Remove a file descriptor from our passt epoll
> - * @c:		Execution context
> + * @epollfd:	epoll file descriptor to add to

Nit: "to delete [the descriptor] from"? It's fixed in 2/6 but still it
looks confusing in this patch.

-- 
Stefano