When calling rte_eal_cleanup, the mp channel cleanup routine only sets mp_fd to -1 leaving the rte_mp_handle control thread running. This control thread can spew warnings on reading on an invalid fd. To handle this situation, sets mp_fd to -1 to signal the control thread it should exit, but since this thread might be sleeping on the socket, cancel the thread too. Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup") Cc: stable@dpdk.org Signed-off-by: David Marchand <david.marchand@redhat.com> --- lib/eal/common/eal_common_proc.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c index dc4a2efa82..2c46c21421 100644 --- a/lib/eal/common/eal_common_proc.c +++ b/lib/eal/common/eal_common_proc.c @@ -35,6 +35,7 @@ #include "eal_internal_cfg.h" static int mp_fd = -1; +static pthread_t mp_handle_tid; static char mp_filter[PATH_MAX]; /* Filter for secondary process sockets */ static char mp_dir_path[PATH_MAX]; /* The directory path for all mp sockets */ static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER; @@ -383,7 +384,7 @@ mp_handle(void *arg __rte_unused) struct mp_msg_internal msg; struct sockaddr_un sa; - while (1) { + while (mp_fd >= 0) { if (read_msg(&msg, &sa) == 0) process_msg(&msg, &sa); } @@ -566,25 +567,11 @@ open_socket_fd(void) return mp_fd; } -static void -close_socket_fd(void) -{ - char path[PATH_MAX]; - - if (mp_fd < 0) - return; - - close(mp_fd); - create_socket_path(peer_name, path, sizeof(path)); - unlink(path); -} - int rte_mp_channel_init(void) { char path[PATH_MAX]; int dir_fd; - pthread_t mp_handle_tid; const struct internal_config *internal_conf = eal_get_internal_configuration(); @@ -645,7 +632,19 @@ rte_mp_channel_init(void) void rte_mp_channel_cleanup(void) { - close_socket_fd(); + char path[PATH_MAX]; + int fd; + + if (mp_fd < 0) + return; + + fd = mp_fd; + mp_fd = -1; + pthread_cancel(mp_handle_tid); + pthread_join(mp_handle_tid, NULL); + close(fd); + create_socket_path(peer_name, path, sizeof(path)); + unlink(path); } /** -- 2.23.0
On Mon, Jun 14, 2021 at 11:13 AM David Marchand <david.marchand@redhat.com> wrote: > > When calling rte_eal_cleanup, the mp channel cleanup routine only sets > mp_fd to -1 leaving the rte_mp_handle control thread running. > This control thread can spew warnings on reading on an invalid fd. > > To handle this situation, sets mp_fd to -1 to signal the control thread > it should exit, but since this thread might be sleeping on the socket, > cancel the thread too. > > Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup") > Cc: stable@dpdk.org > Reported-by: Owen Hilyard <ohilyard@iol.unh.edu> > Signed-off-by: David Marchand <david.marchand@redhat.com> Anatoly, review please. -- David Marchand
When calling rte_eal_cleanup, the mp channel cleanup routine only sets mp_fd to -1 leaving the rte_mp_handle control thread running. This control thread can spew warnings on reading on an invalid fd. This is especially noticed with ASAN enabled. To handle this situation, set mp_fd to -1 to signal the control thread it should exit, but since this thread might be sleeping on the socket, cancel the thread too. Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup") Cc: stable@dpdk.org Reported-by: Owen Hilyard <ohilyard@iol.unh.edu> Signed-off-by: David Marchand <david.marchand@redhat.com> --- Changes since v1: - no functional change, but left close_socket_fd() helper to keep symmetry with rte_mp_channel_init()/open_socket_fd(), --- lib/eal/common/eal_common_proc.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c index dc4a2efa82..ebd0f6673b 100644 --- a/lib/eal/common/eal_common_proc.c +++ b/lib/eal/common/eal_common_proc.c @@ -35,6 +35,7 @@ #include "eal_internal_cfg.h" static int mp_fd = -1; +static pthread_t mp_handle_tid; static char mp_filter[PATH_MAX]; /* Filter for secondary process sockets */ static char mp_dir_path[PATH_MAX]; /* The directory path for all mp sockets */ static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER; @@ -383,7 +384,7 @@ mp_handle(void *arg __rte_unused) struct mp_msg_internal msg; struct sockaddr_un sa; - while (1) { + while (mp_fd >= 0) { if (read_msg(&msg, &sa) == 0) process_msg(&msg, &sa); } @@ -567,14 +568,11 @@ open_socket_fd(void) } static void -close_socket_fd(void) +close_socket_fd(int fd) { char path[PATH_MAX]; - if (mp_fd < 0) - return; - - close(mp_fd); + close(fd); create_socket_path(peer_name, path, sizeof(path)); unlink(path); } @@ -584,7 +582,6 @@ rte_mp_channel_init(void) { char path[PATH_MAX]; int dir_fd; - pthread_t mp_handle_tid; const struct internal_config *internal_conf = eal_get_internal_configuration(); @@ -645,7 +642,16 @@ rte_mp_channel_init(void) void rte_mp_channel_cleanup(void) { - close_socket_fd(); + int fd; + + if (mp_fd < 0) + return; + + fd = mp_fd; + mp_fd = -1; + pthread_cancel(mp_handle_tid); + pthread_join(mp_handle_tid, NULL); + close_socket_fd(fd); } /** -- 2.23.0
On 7/7/21 1:02 PM, David Marchand wrote:
> When calling rte_eal_cleanup, the mp channel cleanup routine only sets
> mp_fd to -1 leaving the rte_mp_handle control thread running.
> This control thread can spew warnings on reading on an invalid fd.
> This is especially noticed with ASAN enabled.
>
> To handle this situation, set mp_fd to -1 to signal the control thread
> it should exit, but since this thread might be sleeping on the socket,
> cancel the thread too.
>
> Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
> Cc: stable@dpdk.org
>
> Reported-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - no functional change, but left close_socket_fd() helper to keep
> symmetry with rte_mp_channel_init()/open_socket_fd(),
>
> ---
> lib/eal/common/eal_common_proc.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
On Thu, Jul 8, 2021 at 9:23 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 7/7/21 1:02 PM, David Marchand wrote:
> > When calling rte_eal_cleanup, the mp channel cleanup routine only sets
> > mp_fd to -1 leaving the rte_mp_handle control thread running.
> > This control thread can spew warnings on reading on an invalid fd.
> > This is especially noticed with ASAN enabled.
> >
> > To handle this situation, set mp_fd to -1 to signal the control thread
> > it should exit, but since this thread might be sleeping on the socket,
> > cancel the thread too.
> >
> > Fixes: 85d6815fa6d0 ("eal: close multi-process socket during cleanup")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Applied, thanks.
--
David Marchand