From: "Wiles, Keith" <keith.wiles@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] eal:Fix log messages always being printed from rte_eal_cpu_init
Date: Mon, 8 Jun 2015 13:33:03 +0000 [thread overview]
Message-ID: <D19B024A.21CD4%keith.wiles@intel.com> (raw)
In-Reply-To: <20150608110940.GD3996@bricha3-MOBL3>
On 6/8/15, 6:09 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
>On Sat, Jun 06, 2015 at 07:04:05PM -0500, Keith Wiles wrote:
>> The RTE_LOG(DEBUG, ...) messages in rte_eal_cpu_init() are printed
>> even when the log level on the command line was set to INFO or lower.
>>
>> The problem is the rte_eal_cpu_init() routine was called before
>> the command line args are scanned. Setting --log-level=7 now
>> correctly does not print the messages from the rte_eal_cpu_init()
>>routine.
>>
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>
>This seems a good idea - make it easy to reduce the verbosity on startup
>if
>so desired. Some comments below.
>
>> ---
>> lib/librte_eal/bsdapp/eal/eal.c | 43
>>++++++++++++++++++++++++++++++++++-----
>> lib/librte_eal/linuxapp/eal/eal.c | 43
>>++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 76 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal.c
>>b/lib/librte_eal/bsdapp/eal/eal.c
>> index 43e8a47..ca10f2c 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal.c
>> @@ -306,6 +306,38 @@ eal_get_hugepage_mem_size(void)
>> return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
>> }
>>
>> +/* Parse the arguments for --log-level only */
>> +static void
>> +eal_log_level_parse(int argc, char **argv)
>> +{
>> + int opt;
>> + char **argvopt;
>> + int option_index;
>> +
>> + argvopt = argv;
>> +
>> + eal_reset_internal_config(&internal_config);
>> +
>> + while ((opt = getopt_long(argc, argvopt, eal_short_options,
>> + eal_long_options, &option_index)) != EOF) {
>> +
>> + int ret;
>> +
>> + /* getopt is not happy, stop right now */
>> + if (opt == '?')
>> + break;
>> +
>> + ret = (opt == OPT_LOG_LEVEL_NUM)?
>> + eal_parse_common_option(opt, optarg, &internal_config) : 0;
>> +
>> + /* common parser is not happy */
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + optind = 0; /* reset getopt lib */
>> +}
>> +
>> /* Parse the argument given in the command line of the application */
>> static int
>> eal_parse_args(int argc, char **argv)
>> @@ -317,8 +349,6 @@ eal_parse_args(int argc, char **argv)
>>
>> argvopt = argv;
>>
>> - eal_reset_internal_config(&internal_config);
>> -
>> while ((opt = getopt_long(argc, argvopt, eal_short_options,
>> eal_long_options, &option_index)) != EOF) {
>>
>> @@ -447,6 +477,12 @@ rte_eal_init(int argc, char **argv)
>> if (rte_eal_log_early_init() < 0)
>> rte_panic("Cannot init early logs\n");
>>
>> + eal_log_level_parse(argc, argv);
>> +
>> + /* set log level as early as possible */
>> + rte_set_log_level(internal_config.log_level);
>> +
>> + RTE_LOG(INFO, EAL, "DPDK Version %s\n", rte_version());
>
>There is already the -v option to the EAL to print the DPDK version. Just
>add
>that flag to any command, as it has no other effects. I don't think we
>need to
>increase the verbosity of startup by always printing it.
OK will remove, but it is one of the things you always need to know when
someone submits the startup messages. This way you do not have to put it
in the email or ask them to tell you.
>
>> if (rte_eal_cpu_init() < 0)
>> rte_panic("Cannot detect lcores\n");
>>
>> @@ -454,9 +490,6 @@ rte_eal_init(int argc, char **argv)
>> if (fctret < 0)
>> exit(1);
>>
>> - /* set log level as early as possible */
>> - rte_set_log_level(internal_config.log_level);
>> -
>> if (internal_config.no_hugetlbfs == 0 &&
>> internal_config.process_type != RTE_PROC_SECONDARY &&
>> eal_hugepage_info_init() < 0)
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>>b/lib/librte_eal/linuxapp/eal/eal.c
>> index bd770cf..090ec99 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -499,6 +499,38 @@ eal_get_hugepage_mem_size(void)
>> return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
>> }
>>
>> +/* Parse the arguments for --log-level only */
>> +static void
>> +eal_log_level_parse(int argc, char **argv)
>> +{
>> + int opt;
>> + char **argvopt;
>> + int option_index;
>> +
>> + argvopt = argv;
>> +
>> + eal_reset_internal_config(&internal_config);
>> +
>> + while ((opt = getopt_long(argc, argvopt, eal_short_options,
>> + eal_long_options, &option_index)) != EOF) {
>> +
>> + int ret;
>> +
>> + /* getopt is not happy, stop right now */
>> + if (opt == '?')
>> + break;
>> +
>> + ret = (opt == OPT_LOG_LEVEL_NUM)?
>> + eal_parse_common_option(opt, optarg, &internal_config) : 0;
>> +
>> + /* common parser is not happy */
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + optind = 0; /* reset getopt lib */
>> +}
>> +
>
>This function looks duplicated for linux and bsd. Can we move it to one
>of the
>common files instead?
I looked at moving this function to the common location, but the problem
is all of the routines of this type are all static functions. I did not
want to add yet another global function to the mix. I will look at it and
see what I can do.
>
>Regards,
>/Bruce
next prev parent reply other threads:[~2015-06-08 13:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-07 0:04 Keith Wiles
2015-06-07 0:04 ` [dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define Keith Wiles
2015-08-02 17:15 ` Thomas Monjalon
2015-08-02 19:10 ` Wiles, Keith
2015-08-02 19:15 ` Wiles, Keith
2015-08-02 20:44 ` Thomas Monjalon
2015-08-02 20:58 ` Wiles, Keith
2015-08-02 21:22 ` Thomas Monjalon
2015-08-02 21:40 ` [dpdk-dev] [PATCH v2] log:Change magic number on RTE_LOG_LEVEL to an enum name Keith Wiles
2015-08-03 3:13 ` Stephen Hemminger
2015-06-08 11:09 ` [dpdk-dev] [PATCH] eal:Fix log messages always being printed from rte_eal_cpu_init Bruce Richardson
2015-06-08 13:33 ` Wiles, Keith [this message]
2015-06-08 13:59 ` Wiles, Keith
2015-06-08 21:55 ` [dpdk-dev] [PATCH v2] " Keith Wiles
2015-06-19 9:54 ` David Marchand
2015-06-22 20:04 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D19B024A.21CD4%keith.wiles@intel.com \
--to=keith.wiles@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).