From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.cs.Stanford.EDU (smtp1.cs.stanford.edu [171.64.64.25]) by dpdk.org (Postfix) with ESMTP id D52A158EC for ; Thu, 13 Oct 2016 18:21:12 +0200 (CEST) Received: from mail-io0-f174.google.com ([209.85.223.174]:32864) by smtp1.cs.Stanford.EDU with esmtpsa (TLSv1.2:AES128-GCM-SHA256:128) (Exim 4.84_2) (envelope-from ) id 1buiko-0005qF-PW for dev@dpdk.org; Thu, 13 Oct 2016 09:21:12 -0700 Received: by mail-io0-f174.google.com with SMTP id q192so91394643iod.0 for ; Thu, 13 Oct 2016 09:21:10 -0700 (PDT) X-Gm-Message-State: AA6/9RmHOCecteRW2F5z6J5RydOlFw8lVw5HVznreIaOVDA5i8qVN44+14hN/cnECkwnMS1op7uyVNS8Wf1mBQ== X-Received: by 10.107.171.194 with SMTP id u185mr7301560ioe.102.1476375670397; Thu, 13 Oct 2016 09:21:10 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.98.67 with HTTP; Thu, 13 Oct 2016 09:20:29 -0700 (PDT) In-Reply-To: References: <20161012212917.10760-1-ouster@cs.stanford.edu> <2601191342CEEE43887BDE71AB9772583F0C1256@irsmsx105.ger.corp.intel.com> From: John Ousterhout Date: Thu, 13 Oct 2016 09:20:29 -0700 X-Gmail-Original-Message-ID: Message-ID: To: "Van Haaren, Harry" Cc: "Ananyev, Konstantin" , "thomas.monjalon@6wind.com" , "dev@dpdk.org" X-Spam-Score: -1.0 X-Spam-Level: X-Spam-Checker-Version: SpamAssassin on smtp1.cs.Stanford.EDU X-Scan-Signature: 6e4c5576ee092703973ea487f2e68838 Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over rte_config file X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Oct 2016 16:21:13 -0000 Hi Harry, But, given the existence of the --file-prefix option, isn't it already unsafe for Collectd to check only for .rte_config? If it's important for other programs to be able to find the config files, it seems to me that a more robust mechanism is needed than the current one. -John- On Thu, Oct 13, 2016 at 9:07 AM, Van Haaren, Harry < harry.van.haaren@intel.com> wrote: > Hi John, > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John Ousterhout > > Subject: Re: [dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over > rte_config file > > > > > For example, it took me several hours > > to figure out why the problem was occurring and then to hunt down the > > --file-prefix solution. Is there some reason why it would be a bad idea > to > > fix this in DPDK? > > Right now, DPDK will by default always use a consistent .rte_config > location, which allows other applications to monitor that. For example, > Collectd is able to monitor a DPDK primary process by checking if the > .rte_config file exists at its default location[1]. > > If we change the default behavior of DPDK, other projects can no longer > rely on that file being created, and these monitoring projects must be > updated in sync with DPDK to avoid breakage if versions mismatch. > > Although I see your use-case, I think using the --file-prefix as > Konstantin suggests is a better solution in this case. > > Regards, -Harry > > [1] https://github.com/collectd/collectd/blob/master/src/dpdkstat.c#L60 > > > > On Thu, Oct 13, 2016 at 3:53 AM, Ananyev, Konstantin < > > konstantin.ananyev@intel.com> wrote: > > > > > > > > Hi John, > > > > > > > Before this patch, DPDK used the file ~/.rte_config as a lock to > detect > > > > potential interference between multiple DPDK applications running on > the > > > > same machine. However, if a single user ran DPDK applications > > > concurrently > > > > on several different machines, and if the user's home directory was > > > shared > > > > between the machines via NFS, DPDK would incorrectly detect conflicts > > > > for all but the first application and abort them. This patch fixes > the > > > > problem by incorporating the machine name into the config file name > > > (e.g., > > > > ~/.rte_hostname_config). > > > > > > > > Signed-off-by: John Ousterhout > > > > --- > > > > doc/guides/prog_guide/multi_proc_support.rst | 11 +++++++---- > > > > lib/librte_eal/common/eal_common_proc.c | 8 ++------ > > > > lib/librte_eal/common/eal_filesystem.h | 15 +++++++++++++-- > > > > 3 files changed, 22 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/doc/guides/prog_guide/multi_proc_support.rst > > > b/doc/guides/prog_guide/multi_proc_support.rst > > > > index badd102..a54fa1c 100644 > > > > --- a/doc/guides/prog_guide/multi_proc_support.rst > > > > +++ b/doc/guides/prog_guide/multi_proc_support.rst > > > > @@ -129,10 +129,13 @@ Support for this usage scenario is provided > using > > > the ``--file-prefix`` paramete > > > > > > > > By default, the EAL creates hugepage files on each hugetlbfs > filesystem > > > using the rtemap_X filename, > > > > where X is in the range 0 to the maximum number of hugepages -1. > > > > -Similarly, it creates shared configuration files, memory mapped in > each > > > process, using the /var/run/.rte_config filename, > > > > -when run as root (or $HOME/.rte_config when run as a non-root user; > > > > -if filesystem and device permissions are set up to allow this). > > > > -The rte part of the filenames of each of the above is configurable > > > using the file-prefix parameter. > > > > +Similarly, it creates shared configuration files, memory mapped in > each > > > process. > > > > +When run as root, the name of the configuration file will be > > > > +/var/run/.rte_*host*_config, where *host* is the name of the > machine. > > > > +When run as a non-root user, the the name of the configuration file > > > will be > > > > +$HOME/.rte_*host*_config (if filesystem and device permissions are > set > > > up to allow this). > > > > +If the ``--file-prefix`` parameter has been specified, its value > will > > > be used > > > > +in place of "rte" in the file names. > > > > > > I am not sure that we need to handle all such cases inside EAL. > > > User can easily overcome that problem by just adding something like: > > > --file-prefix=`uname -n` > > > to his command-line. > > > Konstantin > > > > > > > > > > > In addition to specifying the file-prefix parameter, > > > > any DPDK applications that are to be run side-by-side must > explicitly > > > limit their memory use. > > > > diff --git a/lib/librte_eal/common/eal_common_proc.c > > > b/lib/librte_eal/common/eal_common_proc.c > > > > index 12e0fca..517aa0c 100644 > > > > --- a/lib/librte_eal/common/eal_common_proc.c > > > > +++ b/lib/librte_eal/common/eal_common_proc.c > > > > @@ -45,12 +45,8 @@ rte_eal_primary_proc_alive(const char > > > *config_file_path) > > > > > > > > if (config_file_path) > > > > config_fd = open(config_file_path, O_RDONLY); > > > > - else { > > > > - char default_path[PATH_MAX+1]; > > > > - snprintf(default_path, PATH_MAX, RUNTIME_CONFIG_FMT, > > > > - default_config_dir, "rte"); > > > > - config_fd = open(default_path, O_RDONLY); > > > > - } > > > > + else > > > > + config_fd = open(eal_runtime_config_path(), O_RDONLY); > > > > if (config_fd < 0) > > > > return 0; > > > > > > > > diff --git a/lib/librte_eal/common/eal_filesystem.h > > > b/lib/librte_eal/common/eal_filesystem.h > > > > index fdb4a70..4929aa3 100644 > > > > --- a/lib/librte_eal/common/eal_filesystem.h > > > > +++ b/lib/librte_eal/common/eal_filesystem.h > > > > @@ -41,7 +41,7 @@ > > > > #define EAL_FILESYSTEM_H > > > > > > > > /** Path of rte config file. */ > > > > -#define RUNTIME_CONFIG_FMT "%s/.%s_config" > > > > +#define RUNTIME_CONFIG_FMT "%s/.%s_%s_config" > > > > > > > > #include > > > > #include > > > > @@ -59,11 +59,22 @@ eal_runtime_config_path(void) > > > > static char buffer[PATH_MAX]; /* static so auto-zeroed */ > > > > const char *directory = default_config_dir; > > > > const char *home_dir = getenv("HOME"); > > > > + static char nameBuffer[1000]; > > > > + int result; > > > > > > > > if (getuid() != 0 && home_dir != NULL) > > > > directory = home_dir; > > > > + > > > > + /* > > > > + * Include the name of the host in the config file path. > Otherwise, > > > > + * if DPDK applications run on different hosts but share a home > > > > + * directory (e.g. via NFS), they will choose the same config > > > > + * file and conflict unnecessarily. > > > > + */ > > > > + result = gethostname(nameBuffer, sizeof(nameBuffer)-1); > > > > snprintf(buffer, sizeof(buffer) - 1, RUNTIME_CONFIG_FMT, > directory, > > > > - internal_config.hugefile_prefix); > > > > + internal_config.hugefile_prefix, > > > > + (result == 0) ? nameBuffer : "unknown-host"); > > > > return buffer; > > > > } > > > > > > > > -- > > > > 2.8.3 > > > > > > >