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

[Burakov, Anatoly]

Hi Kevin,

FYI this patch fixes the issue with the clean up patch: http://patches.dpdk.org/patch/49577/

Thanks,
Anatoly

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor at redhat.com]
> Sent: Tuesday, January 8, 2019 6:09 PM
> To: Burakov, Anatoly <anatoly.burakov at intel.com>
> Cc: Varghese, Vipin <vipin.varghese at intel.com>; dpdk stable
> <stable at dpdk.org>
> Subject: Re: patch 'eal: clean up unused files on initialization' has been
> queued to LTS release 18.11.1
> 
> On 01/08/2019 04:53 PM, Burakov, Anatoly wrote:
> > 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. In that case, I'll hold off applying until the fix is also ready for
> backport also.
> 
> Kevin.
> 
> > Thanks,
> > Anatoly
> >
> >
> >> -----Original Message-----
> >> From: Kevin Traynor [mailto:ktraynor at redhat.com]
> >> Sent: Friday, January 4, 2019 1:24 PM
> >> To: Burakov, Anatoly <anatoly.burakov at intel.com>
> >> Cc: Varghese, Vipin <vipin.varghese at intel.com>; dpdk stable
> >> <stable at 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 at 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 at intel.com>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov at 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 <sip:07672255523> +0000
> >> +++ 0009-eal-clean-up-unused-files-on-initialization.patch	2019-01-04
> >> 13:23:07.000000000 <sip:07000000000> +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 at 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 at dpdk.org
> >>
> >>  Reported-by: Vipin Varghese <vipin.varghese at intel.com>
> >>  Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>