From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 75824A054A; Tue, 25 Oct 2022 11:41:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 679ED42B6F; Tue, 25 Oct 2022 11:41:26 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 93DFF41143 for ; Tue, 25 Oct 2022 11:41:24 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 328F513CEC for ; Tue, 25 Oct 2022 11:41:24 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 3133C13F10; Tue, 25 Oct 2022 11:41:24 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.5 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A autolearn=disabled version=3.4.6 X-Spam-Score: -1.5 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 5C48313F8F; Tue, 25 Oct 2022 11:41:23 +0200 (CEST) Message-ID: <296a4668-72d1-4c3d-f454-30e1573e800b@lysator.liu.se> Date: Tue, 25 Oct 2022 11:41:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v3 4/4] trace: create new directory for each trace dump Content-Language: en-US To: David Marchand , dev@dpdk.org Cc: Jerin Jacob , Sunil Kumar Kori References: <20221013074928.3062458-1-david.marchand@redhat.com> <20221025090052.429232-1-david.marchand@redhat.com> <20221025090052.429232-5-david.marchand@redhat.com> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: <20221025090052.429232-5-david.marchand@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2022-10-25 11:00, David Marchand wrote: > Rather than always overwrite the content of the trace directory, create > new directories for each call to rte_trace_save(). > > In the unlikely event that multiple rte_trace_save() gets called many > times in a single second, try to suffix the name with a digit. > > Signed-off-by: David Marchand > --- > lib/eal/common/eal_common_trace.c | 4 ++ > lib/eal/common/eal_common_trace_utils.c | 89 +++++++++++-------------- > lib/eal/common/eal_trace.h | 1 + > 3 files changed, 45 insertions(+), 49 deletions(-) > > diff --git a/lib/eal/common/eal_common_trace.c b/lib/eal/common/eal_common_trace.c > index bb26af618a..7d411b336e 100644 > --- a/lib/eal/common/eal_common_trace.c > +++ b/lib/eal/common/eal_common_trace.c > @@ -92,6 +92,10 @@ eal_trace_fini(void) > trace_mem_free(); > trace_metadata_destroy(); > eal_trace_args_free(); > + free(trace.rootdir); > + trace.rootdir = NULL; > + free(trace.dir); > + trace.dir = NULL; > } > > bool > diff --git a/lib/eal/common/eal_common_trace_utils.c b/lib/eal/common/eal_common_trace_utils.c > index 8561a0e198..6256ef6703 100644 > --- a/lib/eal/common/eal_common_trace_utils.c > +++ b/lib/eal/common/eal_common_trace_utils.c > @@ -87,9 +87,10 @@ trace_uuid_generate(void) > } > > static int > -trace_session_name_generate(char **trace_dir) > +trace_dir_name_generate(char **trace_dir) > { > char date[sizeof("YYYY-mm-dd-AM-HH-MM-SS")]; > + struct trace *trace = trace_obj_get(); > struct tm *tm_result; > time_t tm; > > @@ -106,7 +107,8 @@ trace_session_name_generate(char **trace_dir) > goto fail; > } > > - if (asprintf(trace_dir, "%s-%s", eal_get_hugefile_prefix(), date) == -1) > + if (asprintf(trace_dir, "%s/%s-%s", trace->rootdir, > + eal_get_hugefile_prefix(), date) == -1) > goto fail; > > return 0; > @@ -115,21 +117,6 @@ trace_session_name_generate(char **trace_dir) > return -1; > } > > -static int > -trace_dir_update(const char *str) > -{ > - struct trace *trace = trace_obj_get(); > - char *dir; > - int rc; > - > - rc = asprintf(&dir, "%s%s", trace->dir != NULL ? trace->dir : "", str); > - if (rc != -1) { > - free(trace->dir); > - trace->dir = dir; > - } > - return rc; > -} > - > int > eal_trace_args_save(const char *val) > { > @@ -240,17 +227,16 @@ eal_trace_mode_args_save(const char *val) > int > eal_trace_dir_args_save(char const *val) > { > - char *dir_path; > - int rc; > + struct trace *trace = trace_obj_get(); > > - if (asprintf(&dir_path, "%s/", val) == -1) { > + free(trace->rootdir); > + trace->rootdir = strdup(val); > + if (trace->rootdir == NULL) { > trace_err("failed to copy directory: %s", strerror(errno)); > return -ENOMEM; > } > > - rc = trace_dir_update(dir_path); > - free(dir_path); > - return rc; > + return 0; > } > > int > @@ -293,7 +279,7 @@ trace_dir_default_path_get(char **dir_path) > } > > /* Append dpdk-traces to directory */ > - if (asprintf(dir_path, "%s/dpdk-traces/", home_dir) == -1) > + if (asprintf(dir_path, "%s/dpdk-traces", home_dir) == -1) > return -ENOMEM; > > return 0; > @@ -303,14 +289,11 @@ static int > trace_mkdir(void) > { > struct trace *trace = trace_obj_get(); > - static bool already_done; > - char *session; > + unsigned int try; > + char *trace_dir; > int rc; > > - if (already_done) > - return 0; > - > - if (trace->dir == NULL) { > + if (trace->rootdir == NULL) { > char *dir_path; > > rc = trace_dir_default_path_get(&dir_path); > @@ -318,38 +301,46 @@ trace_mkdir(void) > trace_err("fail to get default path"); > return rc; > } > - > - rc = trace_dir_update(dir_path); > - free(dir_path); > - if (rc < 0) > - return rc; > + trace->rootdir = dir_path; > } > > /* Create the path if it t exist, no "mkdir -p" available here */ > - rc = mkdir(trace->dir, 0700); > + rc = mkdir(trace->rootdir, 0700); > if (rc < 0 && errno != EEXIST) { > - trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno)); > + trace_err("mkdir %s failed [%s]", trace->rootdir, strerror(errno)); > rte_errno = errno; > return -rte_errno; > } > > - rc = trace_session_name_generate(&session); > - if (rc < 0) > - return rc; > - rc = trace_dir_update(session); > - free(session); > + rc = trace_dir_name_generate(&trace_dir); > if (rc < 0) > return rc; > - > - rc = mkdir(trace->dir, 0700); > - if (rc < 0) { > - trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno)); > - rte_errno = errno; > - return -rte_errno; > + rc = mkdir(trace_dir, 0700); > + if (rc == 0) > + goto out; > + > + /* 1000 tries are definitely enough :-) */ Do you need an upper bound at all? Is there a point in retrying for anything other than EEXIST? > + for (try = 1; try < 1000; try++) { > + char *trace_dir_suffix; > + > + if (asprintf(&trace_dir_suffix, "%s.%u", trace_dir, try) == -1) > + continue; > + rc = mkdir(trace_dir_suffix, 0700); > + if (rc == 0) { > + free(trace_dir); > + trace_dir = trace_dir_suffix; > + goto out; > + } > + free(trace_dir_suffix); > } > > + trace_err("mkdir %s failed [%s]", trace->dir, strerror(errno)); > + rte_errno = errno; > + return -rte_errno; > +out: > + free(trace->dir); > + trace->dir = trace_dir; > RTE_LOG(INFO, EAL, "Trace dir: %s\n", trace->dir); > - already_done = true; > return 0; > } > > diff --git a/lib/eal/common/eal_trace.h b/lib/eal/common/eal_trace.h > index d66bcfe198..8991088253 100644 > --- a/lib/eal/common/eal_trace.h > +++ b/lib/eal/common/eal_trace.h > @@ -48,6 +48,7 @@ struct trace_arg { > }; > > struct trace { > + char *rootdir; > char *dir; > int register_errno; > uint32_t status;