From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id ED524CF9 for ; Mon, 8 Jun 2015 15:33:29 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 08 Jun 2015 06:33:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,573,1427785200"; d="scan'208";a="584046017" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by orsmga003.jf.intel.com with ESMTP; 08 Jun 2015 06:33:05 -0700 Received: from orsmsx155.amr.corp.intel.com (10.22.240.21) by ORSMSX105.amr.corp.intel.com (10.22.225.132) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 8 Jun 2015 06:33:04 -0700 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by ORSMSX155.amr.corp.intel.com (10.22.240.21) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 8 Jun 2015 06:33:04 -0700 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.213]) by FMSMSX109.amr.corp.intel.com ([169.254.15.155]) with mapi id 14.03.0224.002; Mon, 8 Jun 2015 06:33:04 -0700 From: "Wiles, Keith" To: "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH] eal:Fix log messages always being printed from rte_eal_cpu_init Thread-Index: AQHQodun4TZdk2mRjUSHjUXcXZbRI52ivReA Date: Mon, 8 Jun 2015 13:33:03 +0000 Message-ID: References: <1433635446-78275-1-git-send-email-keith.wiles@intel.com> <20150608110940.GD3996@bricha3-MOBL3> In-Reply-To: <20150608110940.GD3996@bricha3-MOBL3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.56.243] Content-Type: text/plain; charset="us-ascii" Content-ID: <0B8AD6D484943F41ABD5D1F969F6C3DF@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] eal:Fix log messages always being printed from rte_eal_cpu_init 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: Mon, 08 Jun 2015 13:33:30 -0000 On 6/8/15, 6:09 AM, "Richardson, Bruce" 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. >>=20 >> The problem is the rte_eal_cpu_init() routine was called before >> the command line args are scanned. Setting --log-level=3D7 now >> correctly does not print the messages from the rte_eal_cpu_init() >>routine. >>=20 >> Signed-off-by: Keith Wiles > >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(-) >>=20 >> 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; >> } >> =20 >> +/* 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 =3D argv; >> + >> + eal_reset_internal_config(&internal_config); >> + >> + while ((opt =3D getopt_long(argc, argvopt, eal_short_options, >> + eal_long_options, &option_index)) !=3D EOF) { >> + >> + int ret; >> + >> + /* getopt is not happy, stop right now */ >> + if (opt =3D=3D '?') >> + break; >> + >> + ret =3D (opt =3D=3D OPT_LOG_LEVEL_NUM)? >> + eal_parse_common_option(opt, optarg, &internal_config) : 0; >> + >> + /* common parser is not happy */ >> + if (ret < 0) >> + break; >> + } >> + >> + optind =3D 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) >> =20 >> argvopt =3D argv; >> =20 >> - eal_reset_internal_config(&internal_config); >> - >> while ((opt =3D getopt_long(argc, argvopt, eal_short_options, >> eal_long_options, &option_index)) !=3D EOF) { >> =20 >> @@ -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"); >> =20 >> + 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"); >> =20 >> @@ -454,9 +490,6 @@ rte_eal_init(int argc, char **argv) >> if (fctret < 0) >> exit(1); >> =20 >> - /* set log level as early as possible */ >> - rte_set_log_level(internal_config.log_level); >> - >> if (internal_config.no_hugetlbfs =3D=3D 0 && >> internal_config.process_type !=3D 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; >> } >> =20 >> +/* 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 =3D argv; >> + >> + eal_reset_internal_config(&internal_config); >> + >> + while ((opt =3D getopt_long(argc, argvopt, eal_short_options, >> + eal_long_options, &option_index)) !=3D EOF) { >> + >> + int ret; >> + >> + /* getopt is not happy, stop right now */ >> + if (opt =3D=3D '?') >> + break; >> + >> + ret =3D (opt =3D=3D OPT_LOG_LEVEL_NUM)? >> + eal_parse_common_option(opt, optarg, &internal_config) : 0; >> + >> + /* common parser is not happy */ >> + if (ret < 0) >> + break; >> + } >> + >> + optind =3D 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