Re: [dpdk-stable] patch 'eal: clean up unused files on initialization' has been queued to LTS release 18.11.1

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

FYI,

There's an issue with this patch on FreeBSD that I'm still investigating. I'm hoping to have a fix by tomorrow EOD.

Thanks,
Anatoly


> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Friday, January 4, 2019 1:24 PM
> To: Burakov, Anatoly <anatoly.burakov@intel.com>
> Cc: Varghese, Vipin <vipin.varghese@intel.com>; dpdk stable
> <stable@dpdk.org>
> Subject: patch 'eal: clean up unused files on initialization' has been queued to
> LTS release 18.11.1
> 
> Hi,
> 
> FYI, your patch has been queued to LTS release 18.11.1
> 
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 01/11/19. So please shout if
> anyone has objections.
> 
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for
> rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
> 
> Thanks.
> 
> Kevin Traynor
> 
> ---
> From 8c95205c36c6872e2a96a70bd0044d91cbe1792a Mon Sep 17 00:00:00
> 2001
> From: Anatoly Burakov <anatoly.burakov@intel.com>
> Date: Tue, 13 Nov 2018 15:54:44 +0000
> Subject: [PATCH] eal: clean up unused files on initialization
> 
> [ upstream commit 0a529578f162df8b16e4eb7423e55570f3d13c97 ]
> 
> When creating process data structures, EAL will create many files in EAL
> runtime directory. Because we allow multiple secondary processes to run,
> each secondary process gets their own unique file. With many secondary
> processes running and exiting on the system, runtime directory will, over
> time, create enormous amounts of sockets, fbarray files and other stuff that
> just sits there unused because the process that allocated it has died a long
> time ago. This may lead to exhaustion of disk (or RAM) space in the runtime
> directory.
> 
> Fix this by removing every unlocked file at initialization that matches either
> socket or fbarray naming convention. We cannot be sure of any other files,
> so we'll leave them alone. Also, remove similar code from mp socket code.
> 
> We do it at the end of init, rather than at the beginning, because secondary
> process will use primary process' data structures even if the primary itself has
> died, and we don't want to remove those before we lock them.
> 
> Bugzilla ID: 106
> 
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c         | 100 ++++++++++++++++++++++++
>  lib/librte_eal/common/eal_common_proc.c |  30 -------
>  lib/librte_eal/common/eal_filesystem.h  |   3 +
>  lib/librte_eal/linuxapp/eal/eal.c       |  99 +++++++++++++++++++++++
>  4 files changed, 202 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index b8152a75c..41ddb5a22 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -4,4 +4,6 @@
>   */
> 
> +#include <dirent.h>
> +#include <fnmatch.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -142,4 +144,90 @@ eal_create_runtime_dir(void)  }
> 
> +int
> +eal_clean_runtime_dir(void)
> +{
> +	DIR *dir;
> +	struct dirent *dirent;
> +	int dir_fd, fd, lck_result;
> +	static const char * const filters[] = {
> +		"fbarray_*",
> +		"mp_socket_*"
> +	};
> +
> +	/* open directory */
> +	dir = opendir(runtime_dir);
> +	if (!dir) {
> +		RTE_LOG(ERR, EAL, "Unable to open runtime directory
> %s\n",
> +				runtime_dir);
> +		goto error;
> +	}
> +	dir_fd = dirfd(dir);
> +
> +	/* lock the directory before doing anything, to avoid races */
> +	if (flock(dir_fd, LOCK_EX) < 0) {
> +		RTE_LOG(ERR, EAL, "Unable to lock runtime directory %s\n",
> +			runtime_dir);
> +		goto error;
> +	}
> +
> +	dirent = readdir(dir);
> +	if (!dirent) {
> +		RTE_LOG(ERR, EAL, "Unable to read runtime directory %s\n",
> +				runtime_dir);
> +		goto error;
> +	}
> +
> +	while (dirent != NULL) {
> +		unsigned int f_idx;
> +		bool skip = true;
> +
> +		/* skip files that don't match the patterns */
> +		for (f_idx = 0; f_idx < RTE_DIM(filters); f_idx++) {
> +			const char *filter = filters[f_idx];
> +
> +			if (fnmatch(filter, dirent->d_name, 0) == 0) {
> +				skip = false;
> +				break;
> +			}
> +		}
> +		if (skip) {
> +			dirent = readdir(dir);
> +			continue;
> +		}
> +
> +		/* try and lock the file */
> +		fd = openat(dir_fd, dirent->d_name, O_RDONLY);
> +
> +		/* skip to next file */
> +		if (fd == -1) {
> +			dirent = readdir(dir);
> +			continue;
> +		}
> +
> +		/* non-blocking lock */
> +		lck_result = flock(fd, LOCK_EX | LOCK_NB);
> +
> +		/* if lock succeeds, remove the file */
> +		if (lck_result != -1)
> +			unlinkat(dir_fd, dirent->d_name, 0);
> +		close(fd);
> +		dirent = readdir(dir);
> +	}
> +
> +	/* closedir closes dir_fd and drops the lock */
> +	closedir(dir);
> +	return 0;
> +
> +error:
> +	if (dir)
> +		closedir(dir);
> +
> +	RTE_LOG(ERR, EAL, "Error while clearing runtime dir: %s\n",
> +		strerror(errno));
> +
> +	return -1;
> +}
> +
> +
>  const char *
>  rte_eal_get_runtime_dir(void)
> @@ -808,4 +896,16 @@ rte_eal_init(int argc, char **argv)
>  	}
> 
> +	/*
> +	 * Clean up unused files in runtime directory. We do this at the end of
> +	 * init and not at the beginning because we want to clean stuff up
> +	 * whether we are primary or secondary process, but we cannot
> remove
> +	 * primary process' files because secondary should be able to run
> even
> +	 * if primary process is dead.
> +	 */
> +	if (eal_clean_runtime_dir() < 0) {
> +		rte_eal_init_alert("Cannot clear runtime directory\n");
> +		return -1;
> +	}
> +
>  	rte_eal_mcfg_complete();
> 
> diff --git a/lib/librte_eal/common/eal_common_proc.c
> b/lib/librte_eal/common/eal_common_proc.c
> index 1c3f09aad..6b876590a 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -543,27 +543,4 @@ open_socket_fd(void)  }
> 
> -static int
> -unlink_sockets(const char *filter)
> -{
> -	int dir_fd;
> -	DIR *mp_dir;
> -	struct dirent *ent;
> -
> -	mp_dir = opendir(mp_dir_path);
> -	if (!mp_dir) {
> -		RTE_LOG(ERR, EAL, "Unable to open directory %s\n",
> mp_dir_path);
> -		return -1;
> -	}
> -	dir_fd = dirfd(mp_dir);
> -
> -	while ((ent = readdir(mp_dir))) {
> -		if (fnmatch(filter, ent->d_name, 0) == 0)
> -			unlinkat(dir_fd, ent->d_name, 0);
> -	}
> -
> -	closedir(mp_dir);
> -	return 0;
> -}
> -
>  int
>  rte_mp_channel_init(void)
> @@ -604,11 +581,4 @@ rte_mp_channel_init(void)
>  	}
> 
> -	if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
> -			unlink_sockets(mp_filter)) {
> -		RTE_LOG(ERR, EAL, "failed to unlink mp sockets\n");
> -		close(dir_fd);
> -		return -1;
> -	}
> -
>  	if (open_socket_fd() < 0) {
>  		close(dir_fd);
> diff --git a/lib/librte_eal/common/eal_filesystem.h
> b/lib/librte_eal/common/eal_filesystem.h
> index 6e0331fdb..64a028db7 100644
> --- a/lib/librte_eal/common/eal_filesystem.h
> +++ b/lib/librte_eal/common/eal_filesystem.h
> @@ -26,4 +26,7 @@ int
>  eal_create_runtime_dir(void);
> 
> +int
> +eal_clean_runtime_dir(void);
> +
>  #define RUNTIME_CONFIG_FNAME "config"
>  static inline const char *
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 361744d40..d252c8591 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -14,5 +14,7 @@
>  #include <getopt.h>
>  #include <sys/file.h>
> +#include <dirent.h>
>  #include <fcntl.h>
> +#include <fnmatch.h>
>  #include <stddef.h>
>  #include <errno.h>
> @@ -150,4 +152,89 @@ eal_create_runtime_dir(void)  }
> 
> +int
> +eal_clean_runtime_dir(void)
> +{
> +	DIR *dir;
> +	struct dirent *dirent;
> +	int dir_fd, fd, lck_result;
> +	static const char * const filters[] = {
> +		"fbarray_*",
> +		"mp_socket_*"
> +	};
> +
> +	/* open directory */
> +	dir = opendir(runtime_dir);
> +	if (!dir) {
> +		RTE_LOG(ERR, EAL, "Unable to open runtime directory
> %s\n",
> +				runtime_dir);
> +		goto error;
> +	}
> +	dir_fd = dirfd(dir);
> +
> +	/* lock the directory before doing anything, to avoid races */
> +	if (flock(dir_fd, LOCK_EX) < 0) {
> +		RTE_LOG(ERR, EAL, "Unable to lock runtime directory %s\n",
> +			runtime_dir);
> +		goto error;
> +	}
> +
> +	dirent = readdir(dir);
> +	if (!dirent) {
> +		RTE_LOG(ERR, EAL, "Unable to read runtime directory %s\n",
> +				runtime_dir);
> +		goto error;
> +	}
> +
> +	while (dirent != NULL) {
> +		unsigned int f_idx;
> +		bool skip = true;
> +
> +		/* skip files that don't match the patterns */
> +		for (f_idx = 0; f_idx < RTE_DIM(filters); f_idx++) {
> +			const char *filter = filters[f_idx];
> +
> +			if (fnmatch(filter, dirent->d_name, 0) == 0) {
> +				skip = false;
> +				break;
> +			}
> +		}
> +		if (skip) {
> +			dirent = readdir(dir);
> +			continue;
> +		}
> +
> +		/* try and lock the file */
> +		fd = openat(dir_fd, dirent->d_name, O_RDONLY);
> +
> +		/* skip to next file */
> +		if (fd == -1) {
> +			dirent = readdir(dir);
> +			continue;
> +		}
> +
> +		/* non-blocking lock */
> +		lck_result = flock(fd, LOCK_EX | LOCK_NB);
> +
> +		/* if lock succeeds, remove the file */
> +		if (lck_result != -1)
> +			unlinkat(dir_fd, dirent->d_name, 0);
> +		close(fd);
> +		dirent = readdir(dir);
> +	}
> +
> +	/* closedir closes dir_fd and drops the lock */
> +	closedir(dir);
> +	return 0;
> +
> +error:
> +	if (dir)
> +		closedir(dir);
> +
> +	RTE_LOG(ERR, EAL, "Error while clearing runtime dir: %s\n",
> +		strerror(errno));
> +
> +	return -1;
> +}
> +
>  const char *
>  rte_eal_get_runtime_dir(void)
> @@ -1097,4 +1184,16 @@ rte_eal_init(int argc, char **argv)
>  	}
> 
> +	/*
> +	 * Clean up unused files in runtime directory. We do this at the end of
> +	 * init and not at the beginning because we want to clean stuff up
> +	 * whether we are primary or secondary process, but we cannot
> remove
> +	 * primary process' files because secondary should be able to run
> even
> +	 * if primary process is dead.
> +	 */
> +	if (eal_clean_runtime_dir() < 0) {
> +		rte_eal_init_alert("Cannot clear runtime directory\n");
> +		return -1;
> +	}
> +
>  	rte_eal_mcfg_complete();
> 
> --
> 2.19.0
> 
> ---
>   Diff of the applied patch vs upstream commit (please double-check if non-
> empty:
> ---
> --- -	2019-01-04 13:23:07.672255523 +0000
> +++ 0009-eal-clean-up-unused-files-on-initialization.patch	2019-01-04
> 13:23:07.000000000 +0000
> @@ -1,8 +1,10 @@
> -From 0a529578f162df8b16e4eb7423e55570f3d13c97 Mon Sep 17 00:00:00
> 2001
> +From 8c95205c36c6872e2a96a70bd0044d91cbe1792a Mon Sep 17 00:00:00
> 2001
>  From: Anatoly Burakov <anatoly.burakov@intel.com>
>  Date: Tue, 13 Nov 2018 15:54:44 +0000
>  Subject: [PATCH] eal: clean up unused files on initialization
> 
> +[ upstream commit 0a529578f162df8b16e4eb7423e55570f3d13c97 ]
> +
>  When creating process data structures, EAL will create many files  in EAL
> runtime directory. Because we allow multiple secondary  processes to run,
> each secondary process gets their own unique @@ -24,7 +26,6 @@  before
> we lock them.
> 
>  Bugzilla ID: 106
> -Cc: stable@dpdk.org
> 
>  Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>  Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>