cleanup resources on shutdown
cover cleanup resources on shutdown
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no visible API or ABI changes here.
Stephen Hemminger (14):
eal: log: close on cleanup
eal: log: free dynamic state on cleanup
eal: alarm: close timerfd on eal cleanup
eal: cleanup threads
eal: intr: cleanup resources
eal: mp: end the multiprocess thread during cleanup
eal: interrupts close epoll fd on shutdown
eal: vfio: cleanup the mp sync handle
eal: close mem config on cleanup
tap: close netlink socket on device close
eal: cleanup plugins data
ethdev: raise priority of old driver warning
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
drivers/net/tap/rte_eth_tap.c | 7 ++++- lib/librte_eal/common/eal_common_log.c | 30 +++++++++++++++++- lib/librte_eal/common/eal_common_options.c | 12 +++++++ lib/librte_eal/common/eal_common_proc.c | 17 +++++++--- lib/librte_eal/common/eal_options.h | 1 + lib/librte_eal/common/eal_private.h | 30 ++++++++++++++++++ lib/librte_eal/common/hotplug_mp.c | 5 +++ lib/librte_eal/common/hotplug_mp.h | 6 ++++ lib/librte_eal/common/malloc_heap.c | 6 ++++ lib/librte_eal/common/malloc_heap.h | 3 ++ lib/librte_eal/common/malloc_mp.c | 12 +++++++ lib/librte_eal/common/malloc_mp.h | 3 ++ lib/librte_eal/linux/eal/eal.c | 28 +++++++++++++++++ lib/librte_eal/linux/eal/eal_alarm.c | 11 +++++++ lib/librte_eal/linux/eal/eal_interrupts.c | 35 ++++++++++++++++++--- lib/librte_eal/linux/eal/eal_log.c | 14 +++++++++ lib/librte_eal/linux/eal/eal_vfio.h | 1 + lib/librte_eal/linux/eal/eal_vfio_mp_sync.c | 8 +++++ lib/librte_ethdev/rte_ethdev.c | 2 +- 19 files changed, 218 insertions(+), 13 deletions(-)
Signature · 1 line
2.20.1
PATCH 1/14 eal: log: close on cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
--- lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++ lib/librte_eal/common/eal_private.h | 13 +++++++++++++ lib/librte_eal/linux/eal/eal.c | 1 + lib/librte_eal/linux/eal/eal_log.c | 14 ++++++++++++++ 4 files changed, 40 insertions(+)
lib/librte_eal/common/eal_common_log.c +12 -0
@@ -463,3 +463,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by envrionment-specific cleanup function. +*/ +void +eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/librte_eal/common/eal_private.h +13 -0
@@ -90,6 +90,12 @@ int rte_eal_memzone_init(void);*/ void eal_log_set_default (FILE *default_log); +/** +* Common log cleanup function (private to eal). +* Closes the default log stream. Called from rte_eal_cleanup(). +*/ +void eal_log_cleanup (void ); +/** * Fill configuration with number of physical and logical processors *
@@ -151,6 +157,13 @@ int rte_eal_timer_init(void);*/ int rte_eal_log_init (const char *id,int facility); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Save the log regexp for later */
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1327,6 +1327,7 @@ rte_eal_cleanup(void)rte_service_finalize ();rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config); +rte_eal_log_cleanup ();return 0 ; }
lib/librte_eal/linux/eal/eal_log.c +14 -0
@@ -37,8 +37,16 @@ console_log_write(__attribute__((unused)) void *c, const char *buf, size_t size)return ret; } +static int +console_log_close (__attribute__((unused))void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)return 0 ; } + +void +rte_eal_log_cleanup (void ) +{ +eal_log_cleanup (); +}
Signature · 1 line
2.20.1
PATCH 2/14 eal: log: free dynamic state on cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, free all the memory associated with dynamic log levels and types.
--- lib/librte_eal/common/eal_common_log.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_log.c +17 -1
@@ -14,6 +14,7 @@#include <rte_eal.h> #include <rte_log.h> #include <rte_per_lcore.h> +#include <rte_tailq.h> #include "eal_private.h"
@@ -54,7 +55,7 @@ struct log_cur_msg { };struct rte_log_dynamic_type { -const char *name; +char *name;uint32_t loglevel; };
@@ -470,8 +471,23 @@ eal_log_set_default(FILE *default_log)void eal_log_cleanup (void ) { +struct rte_eal_opt_loglevel *opt_ll, *tmp; +size_t i; +if (default_log_stream) {fclose (default_log_stream); default_log_stream =NULL ; } + + TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) { +free (opt_ll->pattern); +free (opt_ll); + } + +for (i =0 ; i < rte_logs.dynamic_types_len; i++) +free (rte_logs.dynamic_types[i].name); + + rte_logs.dynamic_types_len =0 ; +free (rte_logs.dynamic_types); + rte_logs.dynamic_types =NULL ; }
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 02/14] eal: log: free dynamic state on cleanup
Quoted 19 lines · Stephen Hemminger
When
rte_eal_cleanupis called, free all the memory associated with dynamic log levels and types.Fixes c1b5fa94a46f ("eal: support dynamic log types")Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/common/eal_common_log.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)lib/librte_eal/common/eal_common_log.c +0
-0@@ -470,8 +471,23 @@ eal_log_set_default(FILE *default_log)void eal_log_cleanup (void ) {
lib/librte_eal/common/eal_common_log.c eal_log_set_default(FILE *default_log) · line 483
Quoted 10 lines
lib/librte_eal/common/eal_common_log.c +6
-0+struct rte_eal_opt_loglevel *opt_ll, *tmp; +size_t i; +if (default_log_stream) {fclose (default_log_stream); default_log_stream =NULL ; } + + TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) { +free (opt_ll->pattern);
In regexp case, we have a leak on the regexp buffer. Fixed with:
+ if (opt_ll->pattern != NULL)
+ free(opt_ll->pattern);
+ else
+ regfree(&opt_ll->re_match);
Quoted 13 lines
lib/librte_eal/common/eal_common_log.c +9
-1+free (opt_ll); + } + +for (i =0 ; i < rte_logs.dynamic_types_len; i++) +free (rte_logs.dynamic_types[i].name); + + rte_logs.dynamic_types_len =0 ; +free (rte_logs.dynamic_types); + rte_logs.dynamic_types =NULL ; } --2.20.1
Signature · 1 line
David Marchand
PATCH 3/14 eal: alarm: close timerfd on eal cleanup
In reply to Stephen HemmingerCalling rte_eal_cleanup() should cause DPDK to cleanup all outstanding resources including file descriptors.
--- lib/librte_eal/common/eal_private.h | 7 +++++++ lib/librte_eal/linux/eal/eal.c | 1 + lib/librte_eal/linux/eal/eal_alarm.c | 11 +++++++++++ 3 files changed, 19 insertions(+)
lib/librte_eal/common/eal_private.h +7 -0
@@ -202,6 +202,13 @@ int rte_eal_intr_init(void);*/ int rte_eal_alarm_init (void ); +/** +* Cleanup alarm resources. +* +* This function is private to EAL. +*/ +void rte_eal_alarm_cleanup (void ); +/** * Function is to check if the kernel module(like, vfio, vfio_iommu_type1, * etc.) loaded.
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1325,6 +1325,7 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL );rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);rte_eal_log_cleanup ();
lib/librte_eal/linux/eal/eal_alarm.c +11 -0
@@ -6,6 +6,7 @@#include <signal.h> #include <errno.h> #include <string.h> +#include <unistd.h> #include <sys/queue.h> #include <sys/time.h> #include <sys/timerfd.h>
@@ -72,6 +73,16 @@ rte_eal_alarm_init(void)return -1 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static void eal_alarm_callback(void *arg __rte_unused) {
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 03/14] eal: alarm: close timerfd on eal cleanup
Quoted 10 lines · Stephen Hemminger
Calling
rte_eal_cleanup()should cause DPDK to cleanup all outstanding resources including file descriptors.Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/common/eal_private.h | 7 +++++++ lib/librte_eal/linux/eal/eal.c | 1 + lib/librte_eal/linux/eal/eal_alarm.c | 11 +++++++++++ 3 files changed, 19 insertions(+)
I won't merge this as my FreeBSD vm is broken but I suppose the bits for FreeBSD would be:
lib/librte_eal/freebsd/eal.c +1 -0
@@ -973,6 +973,7 @@ intrte_eal_cleanup (void) {rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
(quoted diff) +0 -0
b/lib/librte_eal/freebsd/eal_alarm.c
index c38b2e04f8..b2089d0b53 100644
lib/librte_eal/freebsd/eal_alarm.c +10 -0
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)return 0 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static inline int timespec_cmp (const struct timespec *now,const struct timespec *at) {
Signature · 1 line
David Marchand
PATCH 4/14 eal: cleanup threads
In reply to Stephen HemmingerWhen rte_eal_cleanup is called it should stop all the child threads and close the pipes between threads.
--- lib/librte_eal/linux/eal/eal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
lib/librte_eal/linux/eal/eal.c +13 -0
@@ -1319,11 +1319,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,int rte_eal_cleanup (void ) { +int i; +/* if we're in a primary process, we need to mark hugepages as freeable * so that finalization can release them back to the system. */ if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); + +RTE_LCORE_FOREACH_SLAVE (i) { +pthread_cancel (lcore_config[i].thread_id); +pthread_join (lcore_config[i].thread_id,NULL ); + +close (lcore_config[i].pipe_master2slave[0 ]); +close (lcore_config[i].pipe_master2slave[1 ]); +close (lcore_config[i].pipe_slave2master[0 ]); +close (lcore_config[i].pipe_slave2master[1 ]); + } +rte_service_finalize ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 04/14] eal: cleanup threads
Quoted 5 lines · Stephen Hemminger
When
rte_eal_cleanupis called it should stop all the child threads and close the pipes between threads.Signed-off-by Stephen Hemminger <[email protected]>---
PATCH 5/14 eal: intr: cleanup resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the interrupt thread and associated resources should be cleaned up.
--- lib/librte_eal/common/eal_private.h | 10 ++++++++++ lib/librte_eal/linux/eal/eal.c | 1 + lib/librte_eal/linux/eal/eal_interrupts.c | 9 +++++++++ 3 files changed, 20 insertions(+)
lib/librte_eal/common/eal_private.h +10 -0
@@ -191,6 +191,16 @@ int rte_eal_tailqs_init(void);*/ int rte_eal_intr_init (void ); +/** +* Cleanup interrupt handling. +* +* This function is private to EAL. +* +* @return +* 0 on success, negative on error +*/ +void rte_eal_intr_cleanup (void ); +/** * Init alarm mechanism. This is to allow a callback be called after * specific time.
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1338,6 +1338,7 @@ rte_eal_cleanup(void) }rte_service_finalize (); +rte_eal_intr_cleanup ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);
lib/librte_eal/linux/eal/eal_interrupts.c +9 -0
@@ -1137,6 +1137,15 @@ rte_eal_intr_init(void)return ret; } +void +rte_eal_intr_cleanup (void ) +{ +pthread_cancel (intr_thread); +pthread_join (intr_thread,NULL ); +close (intr_pipe.readfd); +close (intr_pipe.writefd); +} +static void eal_intr_proc_rxtx_intr (int fd,const struct rte_intr_handle *intr_handle) {
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
Quoted 49 lines · Stephen Hemminger
When
rte_eal_cleanupis called the interrupt thread and associated resources should be cleaned up.Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/common/eal_private.h | 10 ++++++++++ lib/librte_eal/linux/eal/eal.c | 1 + lib/librte_eal/linux/eal/eal_interrupts.c | 9 +++++++++ 3 files changed, 20 insertions(+)lib/librte_eal/common/eal_private.h +10
-0@@ -191,6 +191,16 @@ int rte_eal_tailqs_init(void);*/ int rte_eal_intr_init (void ); +/** +* Cleanup interrupt handling. +* +* This function is private to EAL. +* +* @return +* 0 on success, negative on error +*/ +void rte_eal_intr_cleanup (void ); +/** * Init alarm mechanism. This is to allow a callback be called after * specific time. lib/librte_eal/linux/eal/eal.c +1
-0@@ -1338,6 +1338,7 @@ rte_eal_cleanup(void) }rte_service_finalize (); +rte_eal_intr_cleanup ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);lib/librte_eal/linux/eal/eal_interrupts.c +0
-0
lib/librte_eal/linux/eal/eal_interrupts.c rte_eal_intr_init(void) · line 1146
Quoted 10 lines
lib/librte_eal/linux/eal/eal_interrupts.c +7
-0@@ -1137,6 +1137,15 @@ rte_eal_intr_init(void)return ret; } +void +rte_eal_intr_cleanup (void ) +{ +pthread_cancel (intr_thread); +pthread_join (intr_thread,NULL ); +close (intr_pipe.readfd); +close (intr_pipe.writefd);
What happens to the intr_sources callbacks?
I am unsure we can expect the application to clean this before the eal cleanup.
It would be worth a followup patch.
Quoted 8 lines
lib/librte_eal/linux/eal/eal_interrupts.c +2
-1+} +static void eal_intr_proc_rxtx_intr (int fd,const struct rte_intr_handle *intr_handle) { --2.20.1
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH 05/14] eal: intr: cleanup resources
Quoted 12 lines · On Sat, 25 Apr 2020 18:49:23 +0200 David Marchand <[email protected]>…
lib/librte_eal/linux/eal/eal_interrupts.c rte_eal_intr_init(void) · line 1146
Quoted 8 lines
lib/librte_eal/linux/eal/eal_interrupts.c +7
-0+void +rte_eal_intr_cleanup (void ) +{ +pthread_cancel (intr_thread); +pthread_join (intr_thread,NULL ); +close (intr_pipe.readfd); +close (intr_pipe.writefd);What happens to the
intr_sourcescallbacks?
I am unsure we can expect the application to clean this before the eal cleanup.
Quoted 1 line
It would be worth a followup patch.
The callbacks should be not run after cleanup.
The goal was to cleanup outstanding system resources (as reported by valgrind) on eal_cleanup
PATCH 6/14 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_proc.c +12 -5
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)struct mp_msg_internal msg;struct sockaddr_un sa; -while (1 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); - } +while (read_msg (&msg, &sa) >0 ) +process_msg (&msg, &sa);return NULL ; }
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 06/14] eal: mp: end the multiprocess thread during cleanup
Quoted 8 lines · Stephen Hemminger
When
rte_eal_cleanupis called, all control threads should exit. For the mp thread, this best handled by closing themp_socketand letting the thread see that.This also fixes potential problems where the
mp_socketgets another hard error, and the thread runs away repeating itself by reading the same error.
Quoted 5 lines
Fixes 85d6815fa6d0 ("eal: close multi-process socket during cleanup")Signed-off-by Stephen Hemminger <[email protected]>---
Signature · 2 lines
Thanks, Anatoly
PATCH 7/14 eal: interrupts close epoll fd on shutdown
In reply to Stephen HemmingerUse pthread callbacks to ensure that the epoll fd is closed when rte_eal_cleanup is called.
--- lib/librte_eal/linux/eal/eal_interrupts.c | 26 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)
lib/librte_eal/linux/eal/eal_interrupts.c +21 -5
@@ -1032,6 +1032,19 @@ eal_intr_handle_interrupts(int pfd, unsigned totalfds) } } + +/** +* Callback at end of intr_thread loop or if thread is cancelled +* that closes the epoll file descriptor +*/ +static void +eal_intr_thread_close (void *arg) +{ +int pfd = (int )(unsigned long )arg; + +close (pfd); +} +/** * It builds/rebuilds up the epoll file descriptor with all the * file descriptors being waited on. Then handles the interrupts.
@@ -1061,6 +1074,12 @@ eal_intr_thread_main(__rte_unused void *arg)if (pfd <0 )rte_panic ("Cannot create epoll instance\n" ); +/* close pfd on thread exit or cancel_pop +* see man page for restrictions on this macro. +*/ +pthread_cleanup_push (eal_intr_thread_close, + (void *)(unsigned long )pfd); + pipe_event.data.fd = intr_pipe.readfd;/** * add pipe fd into wait list, this pipe is used to
@@ -1100,11 +1119,8 @@ eal_intr_thread_main(__rte_unused void *arg)/* serve the interrupt */ eal_intr_handle_interrupts (pfd, numfds); -/** -* when we return, we need to rebuild the -* list of fds to monitor. -*/ -close (pfd); +/* close pfd and rebuild the list */ +pthread_cleanup_pop (1 ); } }
Signature · 1 line
2.20.1
PATCH 8/14 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/librte_eal/linux/eal/eal.c | 3 +++ lib/librte_eal/linux/eal/eal_vfio.h | 1 + lib/librte_eal/linux/eal/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 12 insertions(+)
lib/librte_eal/linux/eal/eal.c +3 -0
@@ -1338,6 +1338,9 @@ rte_eal_cleanup(void) }rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_eal_intr_cleanup ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();
lib/librte_eal/linux/eal/eal_vfio.h +1 -0
@@ -132,6 +132,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/librte_eal/linux/eal/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 08/14] eal: vfio: cleanup the mp sync handle
Quoted 8 lines · Stephen Hemminger
When
rte_eal_cleanupis called therte_mp_actionfor VFIO should be freed.Fixes edf73dd33072 ("ipc: handle unsupported IPC in action register")Signed-off-by Stephen Hemminger <[email protected]>---
Signature · 2 lines
Thanks, Anatoly
PATCH 9/14 eal: close mem config on cleanup
In reply to Stephen HemmingerResolves file descriptor left open after rte_eal_cleanup.
--- lib/librte_eal/linux/eal/eal.c | 6 ++++++ 1 file changed, 6 insertions(+)
lib/librte_eal/linux/eal/eal.c +6 -0
@@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);rte_eal_log_cleanup (); + +if (mem_cfg_fd !=-1 ) { +close (mem_cfg_fd); + mem_cfg_fd =-1 ; + } +return 0 ; }
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
Quoted 15 lines · Stephen Hemminger
Resolves file descriptor left open after
rte_eal_cleanup.Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/linux/eal/eal.c | 6 ++++++ 1 file changed, 6 insertions(+)lib/librte_eal/linux/eal/eal.c +0
-0@@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);
lib/librte_eal/linux/eal/eal.c rte_eal_cleanup(void) · line 1357
Quoted 10 lines
lib/librte_eal/linux/eal/eal.c +6
-0rte_eal_log_cleanup (); + +if (mem_cfg_fd !=-1 ) { +close (mem_cfg_fd); + mem_cfg_fd =-1 ; + } +return 0 ; }
For the patch,
However i think it's incomplete, as there are also memory-backing fbarrays that are still mapped. Also, secondary processes have their own shadow copies of the master page table located in the mem config, so those should be destroyed on cleanup too.
Signature · 2 lines
Thanks, Anatoly
Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
Quoted 29 lines · Anatoly
Quoted 15 lines · Stephen Hemminger
Resolves file descriptor left open after
rte_eal_cleanup.Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/linux/eal/eal.c | 6 ++++++ 1 file changed, 6 insertions(+)lib/librte_eal/linux/eal/eal.c +0
-0@@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);lib/librte_eal/linux/eal/eal.c rte_eal_cleanup(void) · line 1357
Quoted 10 lines
lib/librte_eal/linux/eal/eal.c +6
-0rte_eal_log_cleanup (); + +if (mem_cfg_fd !=-1 ) { +close (mem_cfg_fd); + mem_cfg_fd =-1 ; + } +return 0 ; }For the patch,
Quoted 7 lines
Acked-by Anatoly Burakov <[email protected]>However i think it's incomplete, as there are also memory-backing fbarrays that are still mapped. Also, secondary processes have their own shadow copies of the master page table located in the mem config, so those should be destroyed on cleanup too.
This patch set was targeting things in stages. It is not complete, some of the cleanups would be hard to do. Just getting the obvious things first.
Re: [dpdk-dev] [PATCH 09/14] eal: close mem config on cleanup
Quoted 34 lines · Stephen Hemminger
Quoted 31 lines · Anatoly
Quoted 15 lines · Stephen Hemminger
Resolves file descriptor left open after
rte_eal_cleanup.Signed-off-by Stephen Hemminger <[email protected]>--- lib/librte_eal/linux/eal/eal.c | 6 ++++++ 1 file changed, 6 insertions(+)lib/librte_eal/linux/eal/eal.c +0
-0@@ -1346,6 +1346,12 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();eal_cleanup_config (&internal_config);lib/librte_eal/linux/eal/eal.c rte_eal_cleanup(void) · line 1357
Quoted 10 lines
lib/librte_eal/linux/eal/eal.c +6
-0rte_eal_log_cleanup (); + +if (mem_cfg_fd !=-1 ) { +close (mem_cfg_fd); + mem_cfg_fd =-1 ; + } +return 0 ; }For the patch,
Acked-by Anatoly Burakov <[email protected]>
Quoted 9 lines
Quoted 5 lines
However i think it's incomplete, as there are also memory-backing fbarrays that are still mapped. Also, secondary processes have their own shadow copies of the master page table located in the mem config, so those should be destroyed on cleanup too.
This patch set was targeting things in stages. It is not complete, some of the cleanups would be hard to do. Just getting the obvious things first.
Fair enough. You still got my ack :)
Signature · 2 lines
Thanks, Anatoly
PATCH 10/14 tap: close netlink socket on device close
In reply to Stephen HemmingerThe netlink socket for flow creation was left open and never closed.
--- drivers/net/tap/rte_eth_tap.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/net/tap/rte_eth_tap.c +6 -1
@@ -1035,6 +1035,11 @@ tap_dev_close(struct rte_eth_dev *dev) &internals->remote_initial_flags); } +if (internals->nlsk_fd !=-1 ) { +close (internals->nlsk_fd); + internals->nlsk_fd =-1 ; + } +if (internals->ka_fd !=-1 ) {close (internals->ka_fd); internals->ka_fd =-1 ;
@@ -2406,7 +2411,7 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)TAP_LOG (DEBUG ,"Closing %s Ethernet device on numa %u" , tuntap_types[internals->type],rte_socket_id ()); -if (internals->nlsk_fd) { +if (internals->nlsk_fd !=-1 ) {tap_flow_flush (eth_dev,NULL );tap_flow_implicit_flush (internals,NULL );tap_nl_final (internals->nlsk_fd);
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 10/14] tap: close netlink socket on device close
Quoted 8 lines · Stephen Hemminger
The netlink socket for flow creation was left open and never closed.
Fixes bf7b7f437b49 ("net/tap: create netdevice during probing")Signed-off-by Stephen Hemminger <[email protected]>
Afaics, superseded by the "fixes for tap" series recently merged.
http://patchwork.dpdk.org/cover/68602/
Signature · 1 line
David Marchand
PATCH 11/14 eal: cleanup plugins data
In reply to Stephen HemmingerWhen rte_eal_cleanup is called walk through the list of shared objects loaded, and close them and free the data structure.
--- lib/librte_eal/common/eal_common_options.c | 12 ++++++++++++ lib/librte_eal/common/eal_options.h | 1 + lib/librte_eal/linux/eal/eal.c | 1 + 3 files changed, 14 insertions(+)
lib/librte_eal/common/eal_common_options.c +12 -0
@@ -308,6 +308,18 @@ eal_plugins_init(void)return 0 ; } +void +eal_plugins_cleanup (void ) +{ +struct shared_driver *solib, *tmp; + + TAILQ_FOREACH_SAFE(solib, &solib_list, next, tmp) { +if (solib->lib_handle) +dlclose (solib->lib_handle); +free (solib); + } +} +/* * Parse the coremask given as argument (hexadecimal string) and fill * the global configuration (core role and core count) with the parsed
lib/librte_eal/common/eal_options.h +1 -0
@@ -84,5 +84,6 @@ int eal_check_common_options(struct internal_config *internal_cfg);void eal_common_usage (void );enum rte_proc_type_t eal_proc_type_detect (void );int eal_plugins_init (void ); +void eal_plugins_cleanup (void );#endif /* EAL_OPTIONS_H */
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1344,6 +1344,7 @@ rte_eal_cleanup(void)rte_eal_intr_cleanup ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup (); +eal_plugins_cleanup ();eal_cleanup_config (&internal_config);rte_eal_log_cleanup ();
Signature · 1 line
2.20.1
PATCH 12/14 ethdev: raise priority of old driver warning
In reply to Stephen HemmingerThe priority of the message about drivers not using new (correct) behaviour on close was debug. And debug messages are typically surpressed and never seen. Raise the priority so that broken drivers are visible and hopefully get developers to fix.
--- lib/librte_ethdev/rte_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_ethdev/rte_ethdev.c +1 -1
@@ -1717,7 +1717,7 @@ rte_eth_dev_close(uint16_t port_id)rte_eth_dev_release_port (dev);return ; } -RTE_ETHDEV_LOG (DEBUG ,"Port closing is using an old behaviour.\n" +RTE_ETHDEV_LOG (NOTICE ,"Port closing is using an old behaviour.\n" "The driver %s should migrate to the new behaviour.\n" , dev->device->driver->name);/* old behaviour: only free queue arrays */
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 12/14] ethdev: raise priority of old driver warning
04/01/2020 02:33, Stephen Hemminger:
lib/librte_ethdev/rte_ethdev.c rte_eth_dev_close(uint16_t port_id) · line 1717
Quoted 12 lines
The priority of the message about drivers not using new (correct) behaviour on close was debug. And debug messages are typically surpressed and never seen. Raise the priority so that broken drivers are visible and hopefully get developers to fix.
Signed-off-by Stephen Hemminger <[email protected]>---
lib/librte_ethdev/rte_ethdev.c +1
-1@@ -1717,7 +1717,7 @@ rte_eth_dev_close(uint16_t port_id) - RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" + RTE_ETHDEV_LOG(NOTICE, "Port closing is using an old behaviour.\n"
PS: I did not notice this patch earlier.
Please, next time, Cc maintainers with this git-send-email option:
--cc-cmd devtools/get-maintainer.sh
PATCH 13/14 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/librte_eal/common/hotplug_mp.c | 5 +++++ lib/librte_eal/common/hotplug_mp.h | 6 ++++++ lib/librte_eal/linux/eal/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/librte_eal/common/hotplug_mp.c +5 -0
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/librte_eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1343,6 +1343,7 @@ rte_eal_cleanup(void)#endif rte_eal_intr_cleanup ();rte_eal_alarm_cleanup (); +eal_mp_dev_hotplug_cleanup ();rte_mp_channel_cleanup ();eal_plugins_cleanup ();eal_cleanup_config (&internal_config);
Signature · 1 line
2.20.1
PATCH 14/14 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/librte_eal/common/malloc_heap.c | 6 ++++++ lib/librte_eal/common/malloc_heap.h | 3 +++ lib/librte_eal/common/malloc_mp.c | 12 ++++++++++++ lib/librte_eal/common/malloc_mp.h | 3 +++ lib/librte_eal/linux/eal/eal.c | 1 + 5 files changed, 25 insertions(+)
lib/librte_eal/common/malloc_heap.c +6 -0
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/librte_eal/common/malloc_heap.h +3 -0
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); + #ifdef __cplusplus }#endif
lib/librte_eal/common/malloc_mp.c +12 -0
@@ -749,3 +749,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/librte_eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/librte_eal/linux/eal/eal.c +1 -0
@@ -1344,6 +1344,7 @@ rte_eal_cleanup(void)rte_eal_intr_cleanup ();rte_eal_alarm_cleanup ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_mp_channel_cleanup ();eal_plugins_cleanup ();eal_cleanup_config (&internal_config);
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH 14/14] eal: malloc: cleanup mp resources
Quoted 5 lines · Stephen Hemminger
The mp action resources in malloc should be cleaned up via
rte_eal_cleanup.Signed-off-by Stephen Hemminger <[email protected]>---
Signature · 2 lines
Thanks, Anatoly
Hello Stephen,
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
Quoted 6 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no visible API or ABI changes here.
Could you share what you did to run a dpdk application with valgrind?
I tried with testpmd and a 3.15 valgrind (fc30), but I get an init failure on the cpu flags.
$ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64 valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia ==10258== Memcheck, a memory error detector ==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==10258== Command: /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd -c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia ==10258== ERROR: This system does not support "RDSEED". Please check that RTE_MACHINE is set correctly. EAL: FATAL: unsupported cpu type. EAL: unsupported cpu type. EAL: Error - exiting with code: 1 Cause: Cannot init EAL: Operation not supported ==10258== ==10258== HEAP SUMMARY: ==10258== in use at exit: 1,388 bytes in 49 blocks ==10258== total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated ==10258== ==10258== LEAK SUMMARY: ==10258== definitely lost: 0 bytes in 0 blocks ==10258== indirectly lost: 0 bytes in 0 blocks ==10258== possibly lost: 0 bytes in 0 blocks ==10258== still reachable: 1,388 bytes in 49 blocks ==10258== suppressed: 0 bytes in 0 blocks ==10258== Rerun with --leak-check=full to see details of leaked memory ==10258== ==10258== For lists of detected and suppressed errors, rerun with: -s ==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Thanks.
--
David Marchand
Quoted 52 lines · On Wed, 5 Feb 2020 10:32:49 +0100 David Marchand <[email protected]>…
Hello Stephen,
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.Quoted 6 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no visible API or ABI changes here.
Could you share what you did to run a dpdk application with valgrind?
I tried with testpmd and a 3.15 valgrind (fc30), but I get an init failure on the cpu flags.
$ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64 valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd(quoted diff) +0
-3-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia==10258== Memcheck, a memory error detector
==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==10258== Command:/home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd(quoted diff) +0
-3-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia==10258==
ERROR: This system does not support "RDSEED".
Please check thatRTE_MACHINEis set correctly.
EAL: FATAL: unsupported cpu type.
EAL: unsupported cpu type.
EAL: Error - exiting with code: 1
Cause: Cannot init EAL: Operation not supported
==10258==
==10258== HEAP SUMMARY:
==10258== in use at exit: 1,388 bytes in 49 blocks
==10258== total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated
==10258==
==10258== LEAK SUMMARY:
==10258== definitely lost: 0 bytes in 0 blocks
Quoted 9 lines
==10258== indirectly lost: 0 bytes in 0 blocks
==10258== possibly lost: 0 bytes in 0 blocks
==10258== still reachable: 1,388 bytes in 49 blocks
==10258== suppressed: 0 bytes in 0 blocks
==10258== Rerun with --leak-check=full to see details of leaked memory ==10258==
==10258== For lists of detected and suppressed errors, rerun with: -s ==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
I am testing with valgrind on ARM.
It should be possible on x86 but you need to dial down the RTE_MACHINE choice to something valgrind understands.
Quoted 61 lines · Stephen Hemminger
Quoted 57 lines · On Wed, 5 Feb 2020 10:32:49 +0100 David Marchand <[email protected]>…
Hello Stephen,
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.Quoted 6 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no visible API or ABI changes here.
Could you share what you did to run a dpdk application with valgrind?
I tried with testpmd and a 3.15 valgrind (fc30), but I get an init failure on the cpu flags.
$ LD_LIBRARY_PATH=/home/dmarchan/builds/build-gcc-shared/install/usr/local/lib64 valgrind /home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd(quoted diff) +0
-3-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia==10258== Memcheck, a memory error detector
==10258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==10258== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==10258== Command:/home/dmarchan/builds/build-gcc-shared/install/usr/local/bin/dpdk-testpmd(quoted diff) +0
-3-c 3 --no-huge -m 20 -d librte_mempool_ring.so -d librte_pmd_null.so -w 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia==10258==
ERROR: This system does not support "RDSEED".
Please check thatRTE_MACHINEis set correctly.
EAL: FATAL: unsupported cpu type.
EAL: unsupported cpu type.
EAL: Error - exiting with code: 1
Cause: Cannot init EAL: Operation not supported
==10258==
==10258== HEAP SUMMARY:
==10258== in use at exit: 1,388 bytes in 49 blocks
==10258== total heap usage: 97 allocs, 48 frees, 89,426 bytes allocated
==10258==
==10258== LEAK SUMMARY:
==10258== definitely lost: 0 bytes in 0 blocks
==10258== indirectly lost: 0 bytes in 0 blocks
==10258== possibly lost: 0 bytes in 0 blocks
==10258== still reachable: 1,388 bytes in 49 blocks
==10258== suppressed: 0 bytes in 0 blocks
==10258== Rerun with --leak-check=full to see details of leaked memory
Quoted 9 lines
Quoted 4 lines
==10258==
==10258== For lists of detected and suppressed errors, rerun with: -s ==10258== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)I am testing with valgrind on ARM.
It should be possible on x86 but you need to dial down theRTE_MACHINEchoice to something valgrind understands.
Ok, so no black magic in valgrind :-)
Yeah I managed to run with the x86-default target we have in test-meson-builds.sh.
Thanks.
--
David Marchand
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
Quoted 4 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
I am too short on time to check and integrate this big series in rc2, and from now it would be too risky to take in 20.02.
Can you respin it for 20.05 with FreeBSD fixes too?
Thanks.
Signature · 1 line
David Marchand
Quoted 11 lines · On Thu, 6 Feb 2020 15:06:56 +0100 David Marchand <[email protected]>…
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
Quoted 8 lines
Quoted 4 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
I am too short on time to check and integrate this big series in rc2, and from now it would be too risky to take in 20.02.
Can you respin it for 20.05 with FreeBSD fixes too?
OK, but if this kind of patch can't be reviewed then the DPDK process is still broken.
I don't see how FreeBSD matters here. It can be leaky but that is ok. I split it out to get review, then you complain it is too big :-(
Quoted 9 lines · Stephen Hemminger
Recently started using valgrind with DPDK, and the results are not clean.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
Quoted 6 lines
This patch set is a start at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no visible API or ABI changes here.
I was about to push the series (except patch 10), but I hit a crash when passing an invalid option to test-null.sh.
Reproduced with:
Core was generated by `/home/dmarchan/builds/x86_64-native-linux-gcc+shared+kmods/app/testpmd -c 0x3 --log-level='.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0 Missing separate debuginfos, use: dnf debuginfo-install elfutils-libelf-0.178-7.fc30.x86_64 glibc-2.29-28.fc30.x86_64 jansson-2.12-2.fc30.x86_64 libgcc-9.2.1-1.fc30.x86_64 libpcap-1.9.1-1.fc30.x86_64 numactl-libs-2.0.12-2.fc30.x86_64 zlib-1.2.11-19.fc30.x86_64
(gdb) bt full
#0 0x00007fd5231dba64 in pthread_cancel () from /usr/lib64/libpthread.so.0 No symbol table info available.
#1 0x00007fd52320c586 in rte_eal_cleanup () at /home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
i = 1
#2 0x00007fd523215f5e in rte_exit (exit_code=exit_code@entry=1,
format=format@entry=0x47ada4 "Cannot init EAL: %s\n") at
/home/dmarchan/dpdk/lib/librte_eal/linux/eal_debug.c:83
ap = {{gp_offset = 24, fp_offset = 48, overflow_arg_area =0x7ffecdf7aa70, reg_save_area = 0x7ffecdf7a9a0}}
#3 0x000000000043535b in main (argc=21, argv=0x7ffecdf7abc8) at /home/dmarchan/dpdk/app/test-pmd/testpmd.c:3647
diag = -1
port_id = <optimized out>
count = <optimized out>
ret = <optimized out>
(gdb) f 1
#1 0x00007fd52320c586 in rte_eal_cleanup () at /home/dmarchan/dpdk/lib/librte_eal/linux/eal.c:1339
1339 pthread_cancel(lcore_config[i].thread_id);
(gdb) p lcore_config[1].thread_id
$1 = 0
rte_eal_cleanup() is called from rte_exit() by testpmd. But since rte_eal_init() failed at parsing, lcore_config[*].thread_id are invalid, and we crash on pthread_cancel.
I have no quick idea to fix this, series postponed to rc2.
Signature · 1 line
David Marchand
v2 cover eal: cleanup resources on shutdown
In reply to Stephen HemmingerStarted using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no new exposed API or ABI changes here.
v2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for later
Stephen Hemminger (8):
eal: log: close on cleanup
eal: log: free dynamic state on cleanup
eal: alarm: close file on cleanup
eal: cleanup threads
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
lib/librte_eal/common/eal_common_log.c | 33 ++++++++++++++++++++++++- lib/librte_eal/common/eal_common_proc.c | 17 +++++++++---- lib/librte_eal/common/eal_private.h | 20 +++++++++++++++ lib/librte_eal/common/hotplug_mp.c | 5 ++++ lib/librte_eal/common/hotplug_mp.h | 6 +++++ lib/librte_eal/common/malloc_heap.c | 6 +++++ lib/librte_eal/common/malloc_heap.h | 3 +++ lib/librte_eal/common/malloc_mp.c | 12 +++++++++ lib/librte_eal/common/malloc_mp.h | 3 +++ lib/librte_eal/freebsd/eal.c | 1 + lib/librte_eal/freebsd/eal_alarm.c | 10 ++++++++ lib/librte_eal/linux/eal.c | 20 +++++++++++++++ lib/librte_eal/linux/eal_alarm.c | 11 +++++++++ lib/librte_eal/linux/eal_log.c | 14 +++++++++++ lib/librte_eal/linux/eal_vfio.h | 1 + lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++ 16 files changed, 164 insertions(+), 6 deletions(-)
Signature · 1 line
2.20.1
PATCH v2 1/8 eal: log: close on cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
--- lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++ lib/librte_eal/common/eal_private.h | 13 +++++++++++++ lib/librte_eal/linux/eal.c | 1 + lib/librte_eal/linux/eal_log.c | 14 ++++++++++++++ 4 files changed, 40 insertions(+)
lib/librte_eal/common/eal_common_log.c +12 -0
@@ -479,3 +479,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by environment-specific cleanup function. +*/ +void +eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/librte_eal/common/eal_private.h +13 -0
@@ -89,6 +89,12 @@ int rte_eal_memzone_init(void);*/ void eal_log_set_default (FILE *default_log); +/** +* Common log cleanup function (private to eal). +* Closes the default log stream. Called from rte_eal_cleanup(). +*/ +void eal_log_cleanup (void ); +/** * Fill configuration with number of physical and logical processors *
@@ -150,6 +156,13 @@ int rte_eal_timer_init(void);*/ int rte_eal_log_init (const char *id,int facility); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Save the log regexp for later */
lib/librte_eal/linux/eal.c +1 -0
@@ -1337,6 +1337,7 @@ rte_eal_cleanup(void)rte_trace_save ();eal_trace_fini ();eal_cleanup_config (&internal_config); +rte_eal_log_cleanup ();return 0 ; }
lib/librte_eal/linux/eal_log.c +14 -0
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)return ret; } +static int +console_log_close (__attribute__((unused))void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)return 0 ; } + +void +rte_eal_log_cleanup (void ) +{ +eal_log_cleanup (); +}
Signature · 1 line
2.20.1
PATCH v2 2/8 eal: log: free dynamic state on cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, free all the memory associated with dynamic log levels and types.
--- lib/librte_eal/common/eal_common_log.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_log.c +20 -1
@@ -14,6 +14,7 @@#include <rte_eal.h> #include <rte_log.h> #include <rte_per_lcore.h> +#include <rte_tailq.h> #include "eal_private.h"
@@ -54,7 +55,7 @@ struct log_cur_msg { };struct rte_log_dynamic_type { -const char *name; +char *name;uint32_t loglevel; };
@@ -486,8 +487,26 @@ eal_log_set_default(FILE *default_log)void eal_log_cleanup (void ) { +struct rte_eal_opt_loglevel *opt_ll, *tmp; +size_t i; +if (default_log_stream) {fclose (default_log_stream); default_log_stream =NULL ; } + + TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) { +if (opt_ll->pattern !=NULL ) +free (opt_ll->pattern); +else +regfree (&opt_ll->re_match); +free (opt_ll); + } + +for (i =0 ; i < rte_logs.dynamic_types_len; i++) +free (rte_logs.dynamic_types[i].name); + + rte_logs.dynamic_types_len =0 ; +free (rte_logs.dynamic_types); + rte_logs.dynamic_types =NULL ; }
Signature · 1 line
2.20.1
PATCH v2 3/8 eal: alarm: close file on cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, free all the memory associated with dynamic log levels and types.
--- lib/librte_eal/common/eal_private.h | 7 +++++++ lib/librte_eal/freebsd/eal.c | 1 + lib/librte_eal/freebsd/eal_alarm.c | 10 ++++++++++ lib/librte_eal/linux/eal.c | 1 + lib/librte_eal/linux/eal_alarm.c | 11 +++++++++++ 5 files changed, 30 insertions(+)
lib/librte_eal/common/eal_private.h +7 -0
@@ -201,6 +201,13 @@ int rte_eal_intr_init(void);*/ int rte_eal_alarm_init (void ); +/** +* Cleanup alarm resources. +* +* This function is private to EAL. +*/ +void rte_eal_alarm_cleanup (void ); +/** * Function is to check if the kernel module(like, vfio, vfio_iommu_type1, * etc.) loaded.
lib/librte_eal/freebsd/eal.c +1 -0
@@ -973,6 +973,7 @@ intrte_eal_cleanup (void) {rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/librte_eal/freebsd/eal_alarm.c +10 -0
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)return 0 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static inline int timespec_cmp (const struct timespec *now,const struct timespec *at) {
lib/librte_eal/linux/eal.c +1 -0
@@ -1333,6 +1333,7 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL );rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/librte_eal/linux/eal_alarm.c +11 -0
@@ -6,6 +6,7 @@#include <signal.h> #include <errno.h> #include <string.h> +#include <unistd.h> #include <sys/queue.h> #include <sys/time.h> #include <sys/timerfd.h>
@@ -74,6 +75,16 @@ rte_eal_alarm_init(void)return -1 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static void eal_alarm_callback(void *arg __rte_unused) {
Signature · 1 line
2.20.1
PATCH v2 4/8 eal: cleanup threads
In reply to Stephen HemmingerWhen rte_eal_cleanup is called it should stop all the child threads and close the pipes between threads.
--- lib/librte_eal/linux/eal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
lib/librte_eal/linux/eal.c +13 -0
@@ -1327,11 +1327,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,int rte_eal_cleanup (void ) { +int i; +/* if we're in a primary process, we need to mark hugepages as freeable * so that finalization can release them back to the system. */ if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); + +RTE_LCORE_FOREACH_SLAVE (i) { +pthread_cancel (lcore_config[i].thread_id); +pthread_join (lcore_config[i].thread_id,NULL ); + +close (lcore_config[i].pipe_master2slave[0 ]); +close (lcore_config[i].pipe_master2slave[1 ]); +close (lcore_config[i].pipe_slave2master[0 ]); +close (lcore_config[i].pipe_slave2master[1 ]); + } +rte_service_finalize ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();
Signature · 1 line
2.20.1
PATCH v2 5/8 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_proc.c +12 -5
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)struct mp_msg_internal msg;struct sockaddr_un sa; -while (1 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); - } +while (read_msg (&msg, &sa) >0 ) +process_msg (&msg, &sa);return NULL ; }
Signature · 1 line
2.20.1
PATCH v2 6/8 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/librte_eal/linux/eal.c | 3 +++ lib/librte_eal/linux/eal_vfio.h | 1 + lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 12 insertions(+)
lib/librte_eal/linux/eal.c +3 -0
@@ -1346,6 +1346,9 @@ rte_eal_cleanup(void) }rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();
lib/librte_eal/linux/eal_vfio.h +1 -0
@@ -132,6 +132,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/librte_eal/linux/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.20.1
PATCH v2 7/8 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/librte_eal/common/hotplug_mp.c | 5 +++++ lib/librte_eal/common/hotplug_mp.h | 6 ++++++ lib/librte_eal/linux/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/librte_eal/common/hotplug_mp.c +5 -0
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/librte_eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/librte_eal/linux/eal.c +1 -0
@@ -1350,6 +1350,7 @@ rte_eal_cleanup(void)vfio_mp_sync_cleanup ();#endif rte_eal_alarm_cleanup (); +eal_mp_dev_hotplug_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.20.1
PATCH v2 8/8 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/librte_eal/common/malloc_heap.c | 6 ++++++ lib/librte_eal/common/malloc_heap.h | 3 +++ lib/librte_eal/common/malloc_mp.c | 12 ++++++++++++ lib/librte_eal/common/malloc_mp.h | 3 +++ lib/librte_eal/linux/eal.c | 1 + 5 files changed, 25 insertions(+)
lib/librte_eal/common/malloc_heap.c +6 -0
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/librte_eal/common/malloc_heap.h +3 -0
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); + #ifdef __cplusplus }#endif
lib/librte_eal/common/malloc_mp.c +12 -0
@@ -749,3 +749,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/librte_eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/librte_eal/linux/eal.c +1 -0
@@ -1351,6 +1351,7 @@ rte_eal_cleanup(void)#endif rte_eal_alarm_cleanup ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.20.1
v3 cover eal: cleanup resources on shutdown
In reply to Stephen HemmingerStarted using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no new exposed API or ABI changes here.
v3
- fix a couple of minor checkpatch complaints
v2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for later
Stephen Hemminger (8):
eal: log: close on cleanup
eal: log: free dynamic state on cleanup
eal: alarm: close file on cleanup
eal: cleanup threads
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
lib/librte_eal/common/eal_common_log.c | 33 ++++++++++++++++++++++++- lib/librte_eal/common/eal_common_proc.c | 17 +++++++++---- lib/librte_eal/common/eal_private.h | 20 +++++++++++++++ lib/librte_eal/common/hotplug_mp.c | 5 ++++ lib/librte_eal/common/hotplug_mp.h | 6 +++++ lib/librte_eal/common/malloc_heap.c | 6 +++++ lib/librte_eal/common/malloc_heap.h | 3 +++ lib/librte_eal/common/malloc_mp.c | 12 +++++++++ lib/librte_eal/common/malloc_mp.h | 3 +++ lib/librte_eal/freebsd/eal.c | 1 + lib/librte_eal/freebsd/eal_alarm.c | 10 ++++++++ lib/librte_eal/linux/eal.c | 20 +++++++++++++++ lib/librte_eal/linux/eal_alarm.c | 11 +++++++++ lib/librte_eal/linux/eal_log.c | 14 +++++++++++ lib/librte_eal/linux/eal_vfio.h | 1 + lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++ 16 files changed, 164 insertions(+), 6 deletions(-)
Signature · 1 line
2.20.1
PATCH v3 1/8 eal: log: close on cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
--- lib/librte_eal/common/eal_common_log.c | 12 ++++++++++++ lib/librte_eal/common/eal_private.h | 13 +++++++++++++ lib/librte_eal/linux/eal.c | 1 + lib/librte_eal/linux/eal_log.c | 14 ++++++++++++++ 4 files changed, 40 insertions(+)
lib/librte_eal/common/eal_common_log.c +12 -0
@@ -479,3 +479,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by environment-specific cleanup function. +*/ +void +eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/librte_eal/common/eal_private.h +13 -0
@@ -89,6 +89,12 @@ int rte_eal_memzone_init(void);*/ void eal_log_set_default (FILE *default_log); +/** +* Common log cleanup function (private to eal). +* Closes the default log stream. Called from rte_eal_cleanup(). +*/ +void eal_log_cleanup (void ); +/** * Fill configuration with number of physical and logical processors *
@@ -150,6 +156,13 @@ int rte_eal_timer_init(void);*/ int rte_eal_log_init (const char *id,int facility); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Save the log regexp for later */
lib/librte_eal/linux/eal.c +1 -0
@@ -1337,6 +1337,7 @@ rte_eal_cleanup(void)rte_trace_save ();eal_trace_fini ();eal_cleanup_config (&internal_config); +rte_eal_log_cleanup ();return 0 ; }
lib/librte_eal/linux/eal_log.c +14 -0
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)return ret; } +static int +console_log_close(__rte_unused void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
@@ -60,3 +68,9 @@ rte_eal_log_init(const char *id, int facility)return 0 ; } + +void +rte_eal_log_cleanup (void ) +{ +eal_log_cleanup (); +}
Signature · 1 line
2.20.1
PATCH v3 2/8 eal: log: free dynamic state on cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, free all the memory associated with dynamic log levels and types.
--- lib/librte_eal/common/eal_common_log.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_log.c +20 -1
@@ -14,6 +14,7 @@#include <rte_eal.h> #include <rte_log.h> #include <rte_per_lcore.h> +#include <rte_tailq.h> #include "eal_private.h"
@@ -54,7 +55,7 @@ struct log_cur_msg { };struct rte_log_dynamic_type { -const char *name; +char *name;uint32_t loglevel; };
@@ -486,8 +487,26 @@ eal_log_set_default(FILE *default_log)void eal_log_cleanup (void ) { +struct rte_eal_opt_loglevel *opt_ll, *tmp; +size_t i; +if (default_log_stream) {fclose (default_log_stream); default_log_stream =NULL ; } + + TAILQ_FOREACH_SAFE(opt_ll, &opt_loglevel_list, next, tmp) { +if (opt_ll->pattern !=NULL ) +free (opt_ll->pattern); +else +regfree (&opt_ll->re_match); +free (opt_ll); + } + +for (i =0 ; i < rte_logs.dynamic_types_len; i++) +free (rte_logs.dynamic_types[i].name); + + rte_logs.dynamic_types_len =0 ; +free (rte_logs.dynamic_types); + rte_logs.dynamic_types =NULL ; }
Signature · 1 line
2.20.1
PATCH v3 3/8 eal: alarm: close file on cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, free all the memory associated with dynamic log levels and types.
--- lib/librte_eal/common/eal_private.h | 7 +++++++ lib/librte_eal/freebsd/eal.c | 1 + lib/librte_eal/freebsd/eal_alarm.c | 10 ++++++++++ lib/librte_eal/linux/eal.c | 1 + lib/librte_eal/linux/eal_alarm.c | 11 +++++++++++ 5 files changed, 30 insertions(+)
lib/librte_eal/common/eal_private.h +7 -0
@@ -201,6 +201,13 @@ int rte_eal_intr_init(void);*/ int rte_eal_alarm_init (void ); +/** +* Cleanup alarm resources. +* +* This function is private to EAL. +*/ +void rte_eal_alarm_cleanup (void ); +/** * Function is to check if the kernel module(like, vfio, vfio_iommu_type1, * etc.) loaded.
lib/librte_eal/freebsd/eal.c +1 -0
@@ -973,6 +973,7 @@ intrte_eal_cleanup (void) {rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/librte_eal/freebsd/eal_alarm.c +10 -0
@@ -61,6 +61,16 @@ rte_eal_alarm_init(void)return 0 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static inline int timespec_cmp (const struct timespec *now,const struct timespec *at) {
lib/librte_eal/linux/eal.c +1 -0
@@ -1333,6 +1333,7 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL );rte_service_finalize (); +rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/librte_eal/linux/eal_alarm.c +11 -0
@@ -6,6 +6,7 @@#include <signal.h> #include <errno.h> #include <string.h> +#include <unistd.h> #include <sys/queue.h> #include <sys/time.h> #include <sys/timerfd.h>
@@ -74,6 +75,16 @@ rte_eal_alarm_init(void)return -1 ; } +void +rte_eal_alarm_cleanup (void ) +{ +if (intr_handle.fd ==-1 ) +return ; + +close (intr_handle.fd); + intr_handle.fd =-1 ; +} +static void eal_alarm_callback(void *arg __rte_unused) {
Signature · 1 line
2.20.1
PATCH v3 4/8 eal: cleanup threads
In reply to Stephen HemmingerWhen rte_eal_cleanup is called it should stop all the child threads and close the pipes between threads.
--- lib/librte_eal/linux/eal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
lib/librte_eal/linux/eal.c +13 -0
@@ -1327,11 +1327,24 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,int rte_eal_cleanup (void ) { +int i; +/* if we're in a primary process, we need to mark hugepages as freeable * so that finalization can release them back to the system. */ if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); + +RTE_LCORE_FOREACH_SLAVE (i) { +pthread_cancel (lcore_config[i].thread_id); +pthread_join (lcore_config[i].thread_id,NULL ); + +close (lcore_config[i].pipe_master2slave[0 ]); +close (lcore_config[i].pipe_master2slave[1 ]); +close (lcore_config[i].pipe_slave2master[0 ]); +close (lcore_config[i].pipe_slave2master[1 ]); + } +rte_service_finalize ();rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();
Signature · 1 line
2.20.1
PATCH v3 5/8 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/librte_eal/common/eal_common_proc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_proc.c +12 -5
@@ -276,8 +276,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -305,7 +314,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -376,10 +385,8 @@ mp_handle(void *arg __rte_unused)struct mp_msg_internal msg;struct sockaddr_un sa; -while (1 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); - } +while (read_msg (&msg, &sa) >0 ) +process_msg (&msg, &sa);return NULL ; }
Signature · 1 line
2.20.1
PATCH v3 6/8 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/librte_eal/linux/eal.c | 3 +++ lib/librte_eal/linux/eal_vfio.h | 1 + lib/librte_eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 12 insertions(+)
lib/librte_eal/linux/eal.c +3 -0
@@ -1346,6 +1346,9 @@ rte_eal_cleanup(void) }rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_eal_alarm_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();
lib/librte_eal/linux/eal_vfio.h +1 -0
@@ -132,6 +132,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/librte_eal/linux/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.20.1
PATCH v3 7/8 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/librte_eal/common/hotplug_mp.c | 5 +++++ lib/librte_eal/common/hotplug_mp.h | 6 ++++++ lib/librte_eal/linux/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/librte_eal/common/hotplug_mp.c +5 -0
@@ -463,3 +463,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/librte_eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/librte_eal/linux/eal.c +1 -0
@@ -1350,6 +1350,7 @@ rte_eal_cleanup(void)vfio_mp_sync_cleanup ();#endif rte_eal_alarm_cleanup (); +eal_mp_dev_hotplug_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.20.1
PATCH v3 8/8 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/librte_eal/common/malloc_heap.c | 6 ++++++ lib/librte_eal/common/malloc_heap.h | 3 +++ lib/librte_eal/common/malloc_mp.c | 12 ++++++++++++ lib/librte_eal/common/malloc_mp.h | 3 +++ lib/librte_eal/linux/eal.c | 1 + 5 files changed, 25 insertions(+)
lib/librte_eal/common/malloc_heap.c +6 -0
@@ -1362,3 +1362,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/librte_eal/common/malloc_heap.h +3 -0
@@ -100,6 +100,9 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); + #ifdef __cplusplus }#endif
lib/librte_eal/common/malloc_mp.c +12 -0
@@ -749,3 +749,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/librte_eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/librte_eal/linux/eal.c +1 -0
@@ -1351,6 +1351,7 @@ rte_eal_cleanup(void)#endif rte_eal_alarm_cleanup ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_mp_channel_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.20.1
Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
In reply to Stephen HemmingerQuoted 17 lines · Stephen Hemminger
Started using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no new exposed API or ABI changes here.
Quoted 7 lines
v3
- fix a couple of minor checkpatch complaintsv2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for later
Same segfault as v1.
$ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop ./build/app/dpdk-testpmd: unrecognized option '--plop' EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodes
Usage: ./build/app/dpdk-testpmd [options]
(snip)
EAL: FATAL: Invalid 'command line' arguments.
EAL: Invalid 'command line' arguments.
EAL: Error - exiting with code: 1
Cause: Cannot init EAL: Invalid argument./devtools/test-null.sh: line 32: 23134 Broken pipe (
sleep 1 && echo stop )
23135 Segmentation fault (core dumped) | $testpmd -c
$coremask --no-huge -m 20 $libs -w 0:0.0 --vdev net_null1 --vdev net_null2 $eal_options -- --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
That's a pity this patchset is not concluded.
Please Stephen, could you respin with a fix?
03/05/2020 19:21, David Marchand:
Quoted 51 lines
Quoted 17 lines · Stephen Hemminger
Started using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no new exposed API or ABI changes here.
Quoted 7 lines
v3
- fix a couple of minor checkpatch complaintsv2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for laterSame segfault as v1.
$ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop ./build/app/dpdk-testpmd: unrecognized option '--plop' EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodesUsage: ./build/app/dpdk-testpmd [options]
(snip)
EAL: FATAL: Invalid 'command line' arguments.
EAL: Invalid 'command line' arguments.
EAL: Error - exiting with code: 1
Cause: Cannot init EAL: Invalid argument./devtools/test-null.sh: line 32: 23134 Broken pipe (
sleep 1 && echo stop )
23135 Segmentation fault (core dumped) | $testpmd -c
$coremask --no-huge -m 20 $libs -w 0:0.0 --vdevnet_null1--vdevnet_null2$eal_options -- --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
Re: [dpdk-dev] [PATCH v3 0/8] eal: cleanup resources on shutdown
Ping
20/10/2020 00:24, Thomas Monjalon:
Quoted 62 lines
That's a pity this patchset is not concluded.
Please Stephen, could you respin with a fix?03/05/2020 19:21, David Marchand:
Quoted 51 lines
Quoted 17 lines · Stephen Hemminger
Started using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (
rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There are no new exposed API or ABI changes here.
Quoted 7 lines
v3
- fix a couple of minor checkpatch complaintsv2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for laterSame segfault as v1.
$ ./devtools/test-null.sh ./build/app/dpdk-testpmd 0x3 --plop ./build/app/dpdk-testpmd: unrecognized option '--plop' EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodesUsage: ./build/app/dpdk-testpmd [options]
(snip)
EAL: FATAL: Invalid 'command line' arguments.
EAL: Invalid 'command line' arguments.
EAL: Error - exiting with code: 1
Cause: Cannot init EAL: Invalid argument./devtools/test-null.sh: line 32: 23134 Broken pipe (
sleep 1 && echo stop )
23135 Segmentation fault (core dumped) | $testpmd -c
$coremask --no-huge -m 20 $libs -w 0:0.0 --vdevnet_null1--vdevnet_null2$eal_options -- --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
v4 cover cleanup more stuff on shutdown
In reply to Stephen HemmingerStarted using valgrind with DPDK, and there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There should be no new exposed API or ABI changes here.
v4
- rebase to 20.11-rc
- drop one patch (alarm cleanup is implemented)
- drop patch that ends worker threads on cleanup.
the test is calling rte_exit/eal_cleanup in a forked process. (could argue this is a test bug)!
v3
- fix a couple of minor checkpatch complaints
v2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for later
Stephen Hemminger (5):
eal: close log in eal_cleanup
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- lib/eal/common/eal_private.h | 7 +++++++ lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 7 +++++++ lib/eal/linux/eal_log.c | 8 ++++++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 13 files changed, 96 insertions(+), 3 deletions(-)
Signature · 1 line
2.30.2
PATCH v4 1/5 eal: close log in eal_cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
This helps reduce false reports from tools like ASAN and valgrind that track memory leaks.
--- lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_private.h | 7 +++++++ lib/eal/linux/eal.c | 1 + lib/eal/linux/eal_log.c | 8 ++++++++ 4 files changed, 29 insertions(+)
lib/eal/common/eal_common_log.c +13 -0
@@ -18,6 +18,7 @@#include <rte_per_lcore.h> #include "eal_log.h" +#include "eal_private.h" struct rte_log_dynamic_type {const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by eal_cleanup +*/ +void +rte_eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/eal/common/eal_private.h +7 -0
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);*/ int rte_eal_intr_init (void ); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Init alarm mechanism. This is to allow a callback be called after * specific time.
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_trace_save ();eal_trace_fini ();eal_cleanup_config (internal_conf); +rte_eal_log_cleanup ();return 0 ; }
lib/eal/linux/eal_log.c +8 -0
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)return ret; } +static int +console_log_close(__rte_unused void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
Signature · 1 line
2.30.2
PATCH v4 2/5 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
lib/eal/common/eal_common_proc.c +17 -3
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)struct sockaddr_un sa;while (mp_fd >=0 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); +int ret; + + ret =read_msg (&msg, &sa); +if (ret <=0 ) +break ; + +process_msg (&msg, &sa); }return NULL ;
Signature · 1 line
2.30.2
PATCH v4 3/5 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/eal/linux/eal.c | 4 ++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 13 insertions(+)
lib/eal/linux/eal.c +4 -0
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); +rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();
lib/eal/linux/eal_vfio.h +1 -0
@@ -133,6 +133,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/eal/linux/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.30.2
PATCH v4 4/5 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/linux/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/eal/common/hotplug_mp.c +5 -0
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach (); +eal_mp_dev_hotplug_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.30.2
PATCH v4 5/5 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 1 + 5 files changed, 25 insertions(+)
lib/eal/common/malloc_heap.c +6 -0
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/eal/common/malloc_heap.h +3 -0
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); +#endif /* MALLOC_HEAP_H_ */
lib/eal/common/malloc_mp.c +12 -0
@@ -806,3 +806,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/eal/linux/eal.c +1 -0
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.30.2
v5 cover cleanup DPDK resources via eal_cleanup
In reply to Stephen HemmingerWhen testing using ASAN or valgrind with DPDK; there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus internal DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There should be no new exposed API or ABI changes here.
v5
- add stub for windows build in rte_malloc cleanup
v4
- rebase to 20.11-rc
- drop one patch (alarm cleanup is implemented)
- drop patch that ends worker threads on cleanup.
the test is calling rte_exit/eal_cleanup in a forked process. (could argue this is a test bug)!
v3
- fix a couple of minor checkpatch complaints
v2
- rebase after 20.05 file renames
- incorporate review comment feedback
- hold off some of the more involved patches for later
Stephen Hemminger (5):
eal: close log in eal_cleanup
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- lib/eal/common/eal_private.h | 7 +++++++ lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 7 +++++++ lib/eal/linux/eal_log.c | 8 ++++++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ lib/eal/windows/eal_mp.c | 7 +++++++ 14 files changed, 103 insertions(+), 3 deletions(-)
Signature · 1 line
2.30.2
PATCH v5 1/5 eal: close log in eal_cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
This helps reduce false reports from tools like ASAN and valgrind that track memory leaks.
--- lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_private.h | 7 +++++++ lib/eal/linux/eal.c | 1 + lib/eal/linux/eal_log.c | 8 ++++++++ 4 files changed, 29 insertions(+)
lib/eal/common/eal_common_log.c +13 -0
@@ -18,6 +18,7 @@#include <rte_per_lcore.h> #include "eal_log.h" +#include "eal_private.h" struct rte_log_dynamic_type {const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by eal_cleanup +*/ +void +rte_eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/eal/common/eal_private.h +7 -0
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);*/ int rte_eal_intr_init (void ); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Init alarm mechanism. This is to allow a callback be called after * specific time.
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_trace_save ();eal_trace_fini ();eal_cleanup_config (internal_conf); +rte_eal_log_cleanup ();return 0 ; }
lib/eal/linux/eal_log.c +8 -0
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)return ret; } +static int +console_log_close(__rte_unused void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
Signature · 1 line
2.30.2
PATCH v5 2/5 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
lib/eal/common/eal_common_proc.c +17 -3
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)struct sockaddr_un sa;while (mp_fd >=0 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); +int ret; + + ret =read_msg (&msg, &sa); +if (ret <=0 ) +break ; + +process_msg (&msg, &sa); }return NULL ;
Signature · 1 line
2.30.2
PATCH v5 3/5 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/eal/linux/eal.c | 4 ++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 13 insertions(+)
lib/eal/linux/eal.c +4 -0
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); +rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();
lib/eal/linux/eal_vfio.h +1 -0
@@ -133,6 +133,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/eal/linux/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.30.2
PATCH v5 4/5 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/linux/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/eal/common/hotplug_mp.c +5 -0
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach (); +eal_mp_dev_hotplug_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.30.2
PATCH v5 5/5 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 1 + lib/eal/windows/eal_mp.c | 7 +++++++ 6 files changed, 32 insertions(+)
lib/eal/common/malloc_heap.c +6 -0
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/eal/common/malloc_heap.h +3 -0
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); +#endif /* MALLOC_HEAP_H_ */
lib/eal/common/malloc_mp.c +12 -0
@@ -806,3 +806,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/eal/linux/eal.c +1 -0
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/eal/windows/eal_mp.c +7 -0
@@ -87,6 +87,13 @@ register_mp_requests(void)return 0 ; } +void +register_mp_requests (void ) +{ +/* Non-stub function succeeds if multi-process is not supported. */ +EAL_LOG_STUB (); +} +int request_to_primary (struct malloc_mp_req *req) {
Signature · 1 line
2.30.2
v6 cover cleanup more resources on eal_cleanup
In reply to Stephen HemmingerWhen testing using ASAN or valgrind with DPDK; there are lots of leftover memory and file descriptors. This makes it hard to find application leaks versus internal DPDK leaks.
The DPDK has a function that applications can use to tell it to cleanup resources on shutdown (rte_eal_cleanup). But the current coverage of that API is spotty. Many internal parts of DPDK leave files and allocated memory behind.
This patch set is a first step at getting the sub-parts of DPDK to cleanup after themselves. These are the easier ones, the harder and more critical ones are in the drivers and the memory subsystem.
There should be no new exposed API or ABI changes here.
v6 - fix windows stub
v5 - add stub for windows build in rte_malloc cleanup v4
- rebase to 20.11-rc
- drop one patch (alarm cleanup is implemented)
- drop patch that ends worker threads on cleanup.
the test is calling rte_exit/eal_cleanup in a forked process. (could argue this is a test bug)!
Stephen Hemminger (5):
eal: close log in eal_cleanup
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- lib/eal/common/eal_private.h | 7 +++++++ lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 7 +++++++ lib/eal/linux/eal_log.c | 8 ++++++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ lib/eal/windows/eal_mp.c | 7 +++++++ 14 files changed, 103 insertions(+), 3 deletions(-)
Signature · 1 line
2.30.2
PATCH v6 1/5 eal: close log in eal_cleanup
When application calls rte_eal_cleanup on shutdown, the DPDK log should be closed and cleaned up.
This helps reduce false reports from tools like ASAN and valgrind that track memory leaks.
--- lib/eal/common/eal_common_log.c | 13 +++++++++++++ lib/eal/common/eal_private.h | 7 +++++++ lib/eal/linux/eal.c | 1 + lib/eal/linux/eal_log.c | 8 ++++++++ 4 files changed, 29 insertions(+)
lib/eal/common/eal_common_log.c +13 -0
@@ -18,6 +18,7 @@#include <rte_per_lcore.h> #include "eal_log.h" +#include "eal_private.h" struct rte_log_dynamic_type {const char *name;
@@ -535,3 +536,15 @@ eal_log_set_default(FILE *default_log) "Debug dataplane logs available - lower performance\n");#endif } + +/* +* Called by eal_cleanup +*/ +void +rte_eal_log_cleanup (void ) +{ +if (default_log_stream) { +fclose (default_log_stream); + default_log_stream =NULL ; + } +}
lib/eal/common/eal_private.h +7 -0
@@ -152,6 +152,13 @@ int rte_eal_tailqs_init(void);*/ int rte_eal_intr_init (void ); +/** +* Close the default log stream +* +* This function is private to EAL. +*/ +void rte_eal_log_cleanup (void ); +/** * Init alarm mechanism. This is to allow a callback be called after * specific time.
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_trace_save ();eal_trace_fini ();eal_cleanup_config (internal_conf); +rte_eal_log_cleanup ();return 0 ; }
lib/eal/linux/eal_log.c +8 -0
@@ -37,8 +37,16 @@ console_log_write(__rte_unused void *c, const char *buf, size_t size)return ret; } +static int +console_log_close(__rte_unused void *c) +{ +closelog (); +return 0 ; +} +static cookie_io_functions_t console_log_func = { .write = console_log_write, + .close = console_log_close, };/*
Signature · 1 line
2.30.2
PATCH v6 2/5 eal: mp: end the multiprocess thread during cleanup
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, all control threads should exit. For the mp thread, this best handled by closing the mp_socket and letting the thread see that.
This also fixes potential problems where the mp_socket gets another hard error, and the thread runs away repeating itself by reading the same error.
--- lib/eal/common/eal_common_proc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
lib/eal/common/eal_common_proc.c +17 -3
@@ -282,8 +282,17 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) msgh.msg_control = control; msgh.msg_controllen =sizeof (control); +retry: msglen =recvmsg (mp_fd, &msgh,0 ); + +/* zero length message means socket was closed */ +if (msglen ==0 ) +return 0 ; +if (msglen <0 ) { +if (errno ==EINTR ) + goto retry; +RTE_LOG (ERR ,EAL ,"recvmsg failed, %s\n" ,strerror (errno));return -1 ; }
@@ -311,7 +320,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)RTE_LOG (ERR ,EAL ,"invalid received data length\n" );return -1 ; } -return 0 ; +return msglen; } static void
@@ -385,8 +394,13 @@ mp_handle(void *arg __rte_unused)struct sockaddr_un sa;while (mp_fd >=0 ) { -if (read_msg (&msg, &sa) ==0 ) -process_msg (&msg, &sa); +int ret; + + ret =read_msg (&msg, &sa); +if (ret <=0 ) +break ; + +process_msg (&msg, &sa); }return NULL ;
Signature · 1 line
2.30.2
PATCH v6 3/5 eal: vfio: cleanup the mp sync handle
In reply to Stephen HemmingerWhen rte_eal_cleanup is called the rte_mp_action for VFIO should be freed.
--- lib/eal/linux/eal.c | 4 ++++ lib/eal/linux/eal_vfio.h | 1 + lib/eal/linux/eal_vfio_mp_sync.c | 8 ++++++++ 3 files changed, 13 insertions(+)
lib/eal/linux/eal.c +4 -0
@@ -1362,7 +1362,11 @@ rte_eal_cleanup(void)if (rte_eal_process_type () ==RTE_PROC_PRIMARY )rte_memseg_walk (mark_freeable,NULL ); +rte_service_finalize (); +#ifdef VFIO_PRESENT +vfio_mp_sync_cleanup (); +#endif rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();
lib/eal/linux/eal_vfio.h +1 -0
@@ -133,6 +133,7 @@ int vfio_has_supported_extensions(int vfio_container_fd);int vfio_mp_sync_setup (void ); +void vfio_mp_sync_cleanup (void );#define EAL_VFIO_MP "eal_vfio_mp_sync"
lib/eal/linux/eal_vfio_mp_sync.c +8 -0
@@ -120,4 +120,12 @@ vfio_mp_sync_setup(void) return 0; } +void +vfio_mp_sync_cleanup (void ) +{ +if (rte_eal_process_type () !=RTE_PROC_PRIMARY ) +return ; + +rte_mp_action_unregister (EAL_VFIO_MP ); +}#endif
Signature · 1 line
2.30.2
PATCH v6 4/5 eal: hotplug: cleanup multiprocess resources
In reply to Stephen HemmingerWhen rte_eal_cleanup is called, hotplug should unregister the resources associated with the multi-process server.
--- lib/eal/common/hotplug_mp.c | 5 +++++ lib/eal/common/hotplug_mp.h | 6 ++++++ lib/eal/linux/eal.c | 1 + 3 files changed, 12 insertions(+)
lib/eal/common/hotplug_mp.c +5 -0
@@ -462,3 +462,8 @@ int eal_mp_dev_hotplug_init(void)return 0 ; } + +void eal_mp_dev_hotplug_cleanup (void ) +{ +rte_mp_action_unregister (EAL_DEV_MP_ACTION_REQUEST ); +}
lib/eal/common/hotplug_mp.h +6 -0
@@ -37,6 +37,12 @@ struct eal_dev_mp_req {int eal_mp_dev_hotplug_init (void ); +/** +* Unregister all mp action callbacks for hotplug. +*/ +void +eal_mp_dev_hotplug_cleanup (void ); +/** * This is a synchronous wrapper for secondary process send * request to primary process, this is invoked when an attach
lib/eal/linux/eal.c +1 -0
@@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)rte_mp_channel_cleanup ();/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach (); +eal_mp_dev_hotplug_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
Signature · 1 line
2.30.2
PATCH v6 5/5 eal: malloc: cleanup mp resources
In reply to Stephen HemmingerThe mp action resources in malloc should be cleaned up via rte_eal_cleanup.
--- lib/eal/common/malloc_heap.c | 6 ++++++ lib/eal/common/malloc_heap.h | 3 +++ lib/eal/common/malloc_mp.c | 12 ++++++++++++ lib/eal/common/malloc_mp.h | 3 +++ lib/eal/linux/eal.c | 1 + lib/eal/windows/eal_mp.c | 7 +++++++ 6 files changed, 32 insertions(+)
lib/eal/common/malloc_heap.c +6 -0
@@ -1403,3 +1403,9 @@ rte_eal_malloc_heap_init(void)/* add all IOVA-contiguous areas to the heap */ return rte_memseg_contig_walk (malloc_add_seg,NULL ); } + +void +rte_eal_malloc_heap_cleanup (void ) +{ +unregister_mp_requests (); +}
lib/eal/common/malloc_heap.h +3 -0
@@ -85,4 +85,7 @@ malloc_socket_to_heap_id(unsigned int socket_id);int rte_eal_malloc_heap_init (void ); +void +rte_eal_malloc_heap_cleanup (void ); +#endif /* MALLOC_HEAP_H_ */
lib/eal/common/malloc_mp.c +12 -0
@@ -806,3 +806,15 @@ register_mp_requests(void) }return 0 ; } + +void +unregister_mp_requests (void ) +{ +if (rte_eal_process_type () ==RTE_PROC_PRIMARY ) { +rte_mp_action_unregister (MP_ACTION_REQUEST ); + }else { +rte_mp_action_unregister (MP_ACTION_SYNC ); +rte_mp_action_unregister (MP_ACTION_ROLLBACK ); +rte_mp_action_unregister (MP_ACTION_RESPONSE ); + } +}
lib/eal/common/malloc_mp.h +3 -0
@@ -63,6 +63,9 @@ struct malloc_mp_req {int register_mp_requests (void ); +void +unregister_mp_requests (void ); +int request_to_primary (struct malloc_mp_req *req);
lib/eal/linux/eal.c +1 -0
@@ -1371,6 +1371,7 @@ rte_eal_cleanup(void)/* after this point, any DPDK pointers will become dangling */ rte_eal_memory_detach ();eal_mp_dev_hotplug_cleanup (); +rte_eal_malloc_heap_cleanup ();rte_eal_alarm_cleanup ();rte_trace_save ();eal_trace_fini ();
lib/eal/windows/eal_mp.c +7 -0
@@ -87,6 +87,13 @@ register_mp_requests(void)return 0 ; } +void +unregister_mp_requests (void ) +{ +/* Non-stub function succeeds if multi-process is not supported. */ +EAL_LOG_STUB (); +} +int request_to_primary (struct malloc_mp_req *req) {
Signature · 1 line
2.30.2
Thomas Monjalon
AppliedRe: [PATCH v6 0/5] cleanup more resources on eal_cleanup
In reply to Stephen Hemminger13/11/2021 18:22, Stephen Hemminger:
Quoted 6 lines
Stephen Hemminger (5):
eal: close log ineal_cleanup
eal: mp: end the multiprocess thread during cleanup
eal: vfio: cleanup the mp sync handle
eal: hotplug: cleanup multiprocess resources
eal: malloc: cleanup mp resources
Applied, thanks.