From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 1A3B89E7 for ; Mon, 8 Jun 2015 16:00:23 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP; 08 Jun 2015 07:00:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,573,1427785200"; d="scan'208";a="742996523" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by orsmga002.jf.intel.com with ESMTP; 08 Jun 2015 07:00:01 -0700 Received: from orsmsx156.amr.corp.intel.com (10.22.240.22) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 8 Jun 2015 06:59:55 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by ORSMSX156.amr.corp.intel.com (10.22.240.22) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 8 Jun 2015 06:59:55 -0700 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.213]) by fmsmsx115.amr.corp.intel.com ([169.254.4.5]) with mapi id 14.03.0224.002; Mon, 8 Jun 2015 06:59:54 -0700 From: "Wiles, Keith" To: "Wiles, Keith" , "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH] eal:Fix log messages always being printed from rte_eal_cpu_init Thread-Index: AQHQodun4TZdk2mRjUSHjUXcXZbRI52ivReAgAAHgAA= Date: Mon, 8 Jun 2015 13:59:53 +0000 Message-ID: References: <1433635446-78275-1-git-send-email-keith.wiles@intel.com> <20150608110940.GD3996@bricha3-MOBL3> In-Reply-To: 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: <1EBD6F9172325045A18DB1AAEF7A175E@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 14:00:24 -0000 On 6/8/15, 8:33 AM, "Wiles, Keith" wrote: > > >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. Well it looks like a fair number of the functions in eal.c are common between both versions. I could try to move them all or just leave the code as is. What do you want here? >> >>Regards, >>/Bruce >